fix(cgp): a binary on PATH is not a device on the bus — Coverage Nightly was red on an environment fact - #2848
Open
noahgift wants to merge 2 commits into
Open
fix(cgp): a binary on PATH is not a device on the bus — Coverage Nightly was red on an environment fact#2848noahgift wants to merge 2 commits into
noahgift wants to merge 2 commits into
Conversation
…tly was red on an environment fact
Coverage Nightly has been failing on `intel-clean-room-16` with
thread 'profilers::system::tests::test_system_health_with_gpu' panicked:
nvidia-smi exists but no health data
thread 'profilers::system::tests::test_vram_with_gpu' panicked:
nvidia-smi exists but no VRAM data
Both tests gated on `which::which("nvidia-smi").is_ok()` and then asserted the collectors
return data. That runner has the NVIDIA userland installed and no visible device, so
nvidia-smi resolved, the tests concluded a GPU was present, `collect_system_health()`
correctly returned None, and the suite reported a CODE DEFECT for an ENVIRONMENT fact. The
collectors were right; the precondition was asking the wrong question.
THE FIX IS A PROBE, NOT A SKIP. `gpus_reported()` runs
`nvidia-smi --query-gpu=name --format=csv,noheader` and counts non-empty lines: 0 when the
binary is absent, 0 when it is present but the driver reports nothing, N when N devices
answer. Not installed and installed-but-blind are the same fact for this purpose, and only
one of them was previously recognised.
BOTH POLARITIES, SO NEITHER HALF IS VACUOUS. `#[ignore]` is banned here and a one-sided skip
is how a test quietly stops testing, so the single test becomes a pair and exactly one is
meaningful on any box:
- test_gpu_collectors_have_valid_data_when_a_gpu_is_reported -- a reporting GPU must yield
sane health and VRAM readings. Runs on lambda/gx10/yoga.
- test_gpu_collectors_are_none_without_a_reporting_gpu -- no reporting GPU must yield None,
never a fabricated zero and never a panic. Runs in the clean room, which is where the
failure was, and it is a REAL assertion there rather than an early return.
Each half skips loudly (eprintln naming which host condition sent it to the other half), so a
box that should have a GPU and does not report one is visible instead of silently green.
MUTATION-VERIFIED, both halves, on a host with an RTX 4090:
- `collect_system_health()` forced to None -> the positive half FAILED, the negative passed
- `gpus_reported()` forced to 0 and the negative assertion inverted to `is_some()`
-> the negative half FAILED, the positive passed
10/10 profilers::system tests pass; clippy -D warnings clean.
|
§13.11 rung 1 — quorum shadow verdict Shadow mode: this records a verdict and merges nothing. A refusal |
…r that could skip both halves An independent review returned REFUSE on the first version with four findings. All four are real; three change the code and the fourth changes how it is proved. 1. A WEDGED DRIVER WOULD HANG CI. `Command::output()` blocks forever and a hung nvidia-smi is a real state. `which` could never hang, so a naive probe was a REGRESSION in failure mode, not just an improvement in accuracy. The probe now spawns, polls `try_wait` against a 10s deadline, kills the child on expiry, and returns 0. 2. THE PAIR COULD SKIP BOTH HALVES. Two tests each calling the probe admits a TOCTOU: a transient failure makes the positive half skip, the probe recovers, the negative half skips, and the pair asserts NOTHING on that run. There is now ONE test that probes ONCE and branches, so exactly one branch executes on every host and neither can skip. 3. THE SKIP WAS INVISIBLE. `cargo test` swallows `eprintln!` without --nocapture, so a silent skip looked identical to a pass. One test with a branch needs no skip message at all. 4. THE MUTATION PROVED THE WRONG THING. I had inverted the TEST's own assertion, which only confirms the collector currently returns None -- it does not prove the test would catch the collector FABRICATING a reading. Re-verified by mutating the CODE UNDER TEST: M1 collector returns Some(default) with no GPU -> no-GPU branch FAILED (the review's point) M2 collector returns None on a GPU host -> GPU branch FAILED (parser regression) M3 probe filter loosened to non-empty lines -> whitelist test FAILED Both branches of the single test are therefore load-bearing, proved on one host. Also from the review: counting "non-empty lines" would accept a licence banner or an update notice as a GPU. The probe now queries `--query-gpu=index` and counts a line only if it PARSES as an integer -- a whitelist, because a blacklist fails open on the first notice it has not seen. `test_the_probe_counts_only_lines_that_parse_as_an_index` pins five cases including "NVIDIA-SMI has failed because it couldn't communicate with the driver" -> 0. WHAT EACH BRANCH PROVES IS NOW STATED, INCLUDING WHAT IT DOES NOT. The GPU branch proves the collectors PARSE and is the only branch that can. The no-GPU branch proves the ABSENCE path returns None rather than fabricating or panicking; it cannot distinguish "no GPU" from "broken parser" because both yield None. That limit is inherent to the branch and is written down rather than papered over. 10/10 profilers::system tests pass; clippy --all-targets -D warnings clean.
noahgift
enabled auto-merge
September 5, 2026 17:41
noahgift
disabled auto-merge
September 7, 2026 11:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Coverage Nightly is red on an environment fact
Both tests gated on
which::which("nvidia-smi").is_ok()and then asserted the collectors returndata.
intel-clean-room-16has the NVIDIA userland installed and no visible device, sonvidia-smi resolved, the tests concluded a GPU was present,
collect_system_health()correctlyreturned
None, and the suite reported a code defect for an environment fact.The collectors were right. The precondition was asking the wrong question: a binary on
PATHisnot a device on the bus.
The fix is a probe, not a skip
gpus_reported()runsnvidia-smi --query-gpu=name --format=csv,noheaderand counts non-emptylines —
0when absent,0when present but the driver reports nothing,Nwhen N devicesanswer. Not-installed and installed-but-blind are the same fact here, and only one of them was
previously recognised.
Both polarities, so neither half is vacuous
#[ignore]is banned in this repo and a one-sided skip is how a test quietly stops testing. Thesingle test becomes a pair, and exactly one is meaningful on any given box:
..._have_valid_data_when_a_gpu_is_reported..._are_none_without_a_reporting_gpuNone— never a fabricated zero, never a panicThe second is a real assertion where the failure was, not an early return. Each half skips
loudly (
eprintln!naming which host condition sent it to the other), so a box that shouldhave a GPU and does not report one is visible rather than silently green.
Mutation-verified, both halves
On a host with an RTX 4090:
collect_system_health()forced toNonegpus_reported()forced to0, negative assertion inverted tois_some()10/10
profilers::systemtests pass.clippy -p aprender-cgp --all-targets -- -D warningsclean.