fix(conformance): no hardcoded checkout path, and no silent substitute - #20
Merged
Conversation
This is how docs/conformance.md went stale, so it is not tidying. Three sites defaulted to an absolute path on one developer's machine when PTY_NODE_CHECKOUT was unset. The path is a machine specific that CLAUDE.md forbids in a shipped artifact, but the silence is what did the damage. The test did not skip. It compared against whatever was at that path, which on one host is a checkout seven commits behind, and reported a confident failure about a completion entry that was not missing. CI never saw it, because the path does not exist on a runner, so there the filter returned None and the test skipped. It only misbehaved where that path happened to exist. The generator was worse. With no checkout it fell back to that path, and node_suites falls back again to the compiled-in SUITES list, so running it with the variable unset wrote a checked-in document describing a Node tree nobody named, looking exactly as authoritative as a correct one. That is how docs/conformance.md came to say 0.12.0+500eab2 while node-ref said 86dcc5e — and a stale map is what a stale comparison is built on. The two cases get different answers on purpose. A TEST skips: a missing reference means the comparison cannot be made, and that is not a failure. A GENERATOR errors: it writes a file that outlives the run and carries no memory of what it was made from. completions.rs was also the only test doing this. parity_shapes.rs and parity_fixtures.rs already consult node_checkout() and skip. This was a divergence from a local convention, not a missing one, so the fix copies them. Controlled in both directions, because a test that always skips is worse than one that compares against a stale checkout: at least the stale one fails sometimes. With the variable set the test RUNS and passes, 9 of 9. Unset, it skips and names the variable it needs. The generator refuses when unset, refuses with a clear reason when pointed somewhere without tests/, and gets past that check when pointed at the pinned checkout. No /home/ or /Users/ path remains anywhere outside target.
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
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.
This is how
docs/conformance.mdwent stale, so it is not tidying.Three sites defaulted to an absolute path on one developer's machine when
PTY_NODE_CHECKOUTwas unset. The path is a machine specific CLAUDE.md forbids in a shipped artifact — but the silence is what did the damage.What it actually caused
The test did not skip. It compared against whatever was at that path, which on one host is a checkout seven commits behind, and reported a confident failure about a completion entry that was not missing at all. CI never saw it: the path does not exist on a runner, so there the filter returns
Noneand the test skips. It only misbehaved where that path happened to exist — which is why it survived.The generator was worse. With no checkout it fell back to that path, and
node_suitesfalls back again to the compiled-inSUITESlist, so running it with the variable unset wrote a checked-in document describing a Node tree nobody named, looking exactly as authoritative as a correct one.That is the mechanism by which
docs/conformance.mdcame to say0.12.0+500eab2whilenode-refsaid86dcc5e. A stale map is what a stale comparison is built on, and a stale comparison is what produced eight false divergences earlier this week.Two cases, two different answers, on purpose
completions.rswas also the only test doing this.parity_shapes.rsandparity_fixtures.rsalready consultnode_checkout()and skip. So this was a divergence from an established local convention rather than a missing one, and the fix copies the neighbours rather than inventing a pattern.Controlled in both directions
A test that always skips would be worse than one comparing against a stale checkout — at least the stale one fails sometimes. So the skip path being obviously right is not a reason to skip the check:
no Node checkout with completions/ (set PTY_NODE_CHECKOUT).node-ref.tests/: exits 1 with that reason.grepfor/home/and/Users/outsidetarget: 0 remaining.One thing this surfaces but does not fix
Pointed at the pinned checkout, the generator now stops at a pre-existing guard:
Node suite kill-report.test.ts has no classification. Five Node suites postdate the map —kill-report,output-activity,proc-table,process-groups,spawn-already-published. TheSUITEStable is untouched by this diff; classifying them is the separate work of regenerating the map, and it is exactly what the old silent fallback was hiding.