Skip to content

fix: run the suite on the systems that get a binary - #63

Open
P4suta wants to merge 1 commit into
mainfrom
test-where-we-ship
Open

P4suta wants to merge 1 commit into
mainfrom
test-where-we-ship

Conversation

@P4suta

@P4suta P4suta commented Sep 19, 2026

Copy link
Copy Markdown
Owner

cargo test ran on Linux alone, while release.yml ships x86_64-pc-windows-msvc. What Windows CI measured was that the crate builds and prints its version:

- run: cargo build --manifest-path rust/Cargo.toml --release --locked -p ocomment
- name: Windows smoke test
  run: '& rust/target/release/ocomment.exe --version'

That is worse than measuring nothing. The job went green, so the run went green, and a green run reads as Windows passes. The ground for that reading appears nowhere in the output — a claim taken from how the measuring was done rather than from what was measured.

What it finds immediately

a_first_segment_that_reads_as_a_drive_letter_is_disambiguated asked a question with two right answers.

c:/a.rs names a directory called c: in a POSIX checkout, and the root of a drive on Windows. std::path says so: components() yields two Normals there and a Prefix here. Being under the source root follows from that, and so does the SARIF location — %SRCROOT% with a leading ./ on one system, no base id on the other.

The implementation was right on both. The test held one system's answer, and nothing had ever asked the other. It now asks each.

A second case pins under_source_root itself, because both halves of the first would pass if that function simply stopped answering.

The step

Added to host-smoke, which already builds on all three systems, rather than as a new job. Skipped on Linux, where the rust job runs it with the two switches that turn a skip into a failure.

Neither switch may be set here. They are read with is_some, so OCOMMENT_REQUIRE_FORMATTERS: "0" would demand the formatters rather than excuse them — which the first draft of this did.

Also

sync_parent is split by system instead of guarding its body, so the Windows build stops warning about a parameter the arm that does nothing cannot use. Taken from #61, an abandoned branch; that PR can close.

Verification

  • cargo xtask preflight: 30 steps green.
  • The Windows and macOS halves of the new test are checked by CI on this PR, which is the point of it — they cannot be checked here.
  • OComment's own gate rejected two drafts of the comments in this change for running past max_lines = 8, the second because two adjacent # NOTE: blocks are one run.

`cargo test` ran on Linux alone, while `release.yml` ships
`x86_64-pc-windows-msvc`. What Windows CI measured was that the crate
builds and prints its version.

That is worse than measuring nothing. The job went green, so the run
went green, and a green run reads as *Windows passes* -- the ground for
it appears nowhere in the output. A claim taken from how the measuring
was done rather than from what was measured.

The first thing it finds was already known to one person who had run the
suite by hand. `a_first_segment_that_reads_as_a_drive_letter_is_
disambiguated` asked a question with two right answers: `c:/a.rs` names
a directory called `c:` in a POSIX checkout and the root of a drive on
Windows, `std::path` says so, and the SARIF location follows -- under
`%SRCROOT%` with a `./` on one system and under no base on the other.
The implementation was right on both. The test held one system's answer,
and nothing had ever asked the other.

It now asks each. A second case pins `under_source_root` itself, because
both halves of the first would pass if that function simply stopped
answering.

The step is skipped on Linux, where the `rust` job already runs it with
the two switches that turn a skip into a failure. Neither may be set
here: they are read with `is_some`, so `OCOMMENT_REQUIRE_FORMATTERS:
"0"` would demand the formatters rather than excuse them.

`sync_parent` is split by system rather than guarding its body, so the
Windows build stops warning about a parameter the arm that does nothing
cannot use. Taken from an abandoned branch.
@coderabbitai

coderabbitai Bot commented Sep 19, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: a0fc1d1d-48b3-4c05-bcfa-9724bc03bfb3


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant