recover: land #186 on master (it merged into #183's branch, not master) - #196
Merged
Conversation
tools/test_build_flags_stamp.py has eight tests, every one of which shells
out to `make`. Each opened with
missing = _toolchain_missing()
if missing:
print(f"SKIP: {missing} not on PATH")
return
and a pytest test function that returns None without asserting is a PASS.
The module is pinned in pytest.ini `testpaths`, so on a machine without
cc65 -- a CI container, most likely -- bare `pytest` at the repo root
reported `7 passed in 0.01s` having verified nothing. The SKIP lines were
not even visible: pytest captures stdout on a passing test and discards it.
The standalone runner had the same hole from the other side, printing
`PASSED: 0 failure(s)` for a run in which zero assertions executed.
The invariant being laundered is build/flags.stamp itself -- what closes
CLAUDE.md's two documented silent-failure modes. #158 adopted the rule (an
involuntary skip is a failure; a voluntary skip is allowed but must never be
silent), #157 reintroduced it, PR #172 closed it again.
Both channels now:
pytest lane _require_toolchain() calls _skip_policy.require(), which
raises SkipPolicyError carrying the vacuity warning
INSIDE the reason string -- under `-ra` the reason is the
only channel that survives.
standalone lane main() hoists the check and returns cannot_run() = exit
2, distinct from 1 ("a check ran and failed"), plus a
ran+failed==0 guard so PASSED can never be printed by a
run that executed nothing.
opt-out C64_ALLOW_SKIP=1 buys exit 0 / a real pytest skip and
still prints the warning; exactly "1", so
C64_ALLOW_SKIP=0 does NOT open the hatch.
tools/_skip_policy.py is vendored from the #178 Part A work, byte-identical
to the copy on branch test/178a-skip-policy-rig-half (c3cf8a4), so this PR
is green standing alone rather than importing a module that is not on master
yet. An identical both-added file merges without conflict in either order.
That branch also adds tools/test_skip_policy.py (unit tests for the module)
and wires the rig lane; none of that is duplicated here.
New tools/test_flags_stamp_skip_is_loud.py is the red-green. It re-runs
the subject module in a subprocess whose PATH has had every directory
containing ca65/ld65 removed -- computed from the real PATH, not a
hardcoded /usr/bin:/bin, so it strips the toolchain wherever it is
installed. test_the_strip_actually_strips guards the guard: a
PATH-stripping bug would otherwise make every assertion vacuous, one level
up from the defect being fixed.
Measured, with the subject module at the pre-fix revision:
FAIL test_opt_out_is_explicit_and_still_warns
FAIL test_pytest_run_without_toolchain_is_not_green
FAIL test_standalone_run_without_toolchain_exits_cannot_run
ok test_the_strip_actually_strips
FAILED: 3 failure(s) (4 executed)
and after:
PASSED: 0 failure(s) (4 executed)
Bare `pytest` at the repo root: 60 passed (was 56; +4 from the new module).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#180 (#178a) amended the module after this branch copied it, so the two copies are no longer identical (theirs c359a9b1..., the vendored one befcf309...) and `git merge-tree` reports an add/add conflict with five hunks in both directions. The "merges cleanly in either order" claim this branch shipped with is now false, and the PR body is corrected to match. Dropping the copy is free: this branch uses only require(), cannot_run() and EXIT_CANNOT_RUN, whose contracts did not change, and all five probes in tools/test_flags_stamp_skip_is_loud.py behave identically against either version of the module. Merge order is therefore load-bearing and stated in the PR body: #183 (this branch's base) -> #180 (brings _skip_policy.py) -> #186. Also migrates the guard in test_an_unrelated_option_does_not_suppress_invalidation, added to the base branch after this one was written, to _require_toolchain(). It was the one remaining site still spelling the silent-skip prologue by hand, which is the whole point of #177. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nto recover/186-onto-master
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.
Why this exists
PR #186 shows as MERGED, but its content never reached
master.It was stacked on #183, so its base branch was
fix/174-dry-run-non-destructiverather than
master. #183 merged tomasterfirst; #186 then merged intofix/174-dry-run-non-destructive, which was already merged and is now a deadend.
tools/test_flags_stamp_skip_is_loud.pyis absent frommaster, andissue #177 is still genuinely open.
This PR merges #186's branch onto current
masterso the work lands. Nocommits were rewritten and nothing was cherry-picked; #186's two commits are
carried as-is.
Why merging the stacked branch directly would have been wrong
fix/174-dry-run-non-destructivepredates #180, so a naive merge of thatbranch reverts #180's three files. This PR merges #186's own branch
(
fix/177-flags-stamp-skip-loud), which three-way merges correctly.Verification
tools/_skip_policy.pyafter the merge is byte-identical tomaster's(test: an involuntary skip is a failure — tools/_skip_policy.py and the rig lane (#178a) #180's fixed version, with
VoluntarySkip/sys.modules.get). flags-stamp suite: an absent cc65 must be loud, not green (#177) #186'svendored copy was already dropped in
3619583, so exactly one copy exists.tools/test_skip_policy.pyandtools/test_rig_skip_contract.pyboth survive intact.
masterin identical conditions (freshworktree, no
build/): identical, 12 on both, so the merge introducesno new failure. Passing count goes 76 → 80, exactly flags-stamp suite: an absent cc65 must be loud, not green (#177) #186's four checks.
build/present,masteris 88 passed.Closes #177.
🤖 Generated with Claude Code