Skip to content

flags-stamp suite: an absent cc65 must be loud, not green (#177) - #186

Merged
JC-000 merged 2 commits into
fix/174-dry-run-non-destructivefrom
fix/177-flags-stamp-skip-loud
Sep 6, 2026
Merged

flags-stamp suite: an absent cc65 must be loud, not green (#177)#186
JC-000 merged 2 commits into
fix/174-dry-run-non-destructivefrom
fix/177-flags-stamp-skip-loud

Conversation

@JC-000

@JC-000 JC-000 commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Closes #177. The eighth site in #178's survey.

Merge order is load-bearing

#183  (this PR's base — issue #174)
  ->  #180  (#178a, brings tools/_skip_policy.py)
  ->  #186  (this PR)

This branch imports tools/_skip_policy.py and no longer carries a copy.
An earlier revision vendored it, on the reasoning that an identical both-added
file merges cleanly in either order. #180 then amended the module and
git merge-tree reported an add/add conflict in both directions, so that claim
is retracted and the copy is dropped instead. No sha is quoted here on
purpose
: the module has moved three times while this PR was open, which is
itself the argument for importing rather than vendoring. The authority is
origin/test/178a-skip-policy-rig-half, whatever it currently points at.

Dropping the copy costs nothing — this branch uses only require(),
cannot_run() and EXIT_CANNOT_RUN, whose contracts have not changed through
any of those revisions.

Verified against that branch's current head: the merge is conflict-free
(including pytest.ini, which ends up with all three new testpaths lines),
the merged tree's module is byte-identical to the one #180 provides — so it
comes from exactly one place — and on that tree
python3 tools/test_flags_stamp_skip_is_loud.py is PASSED: 0 failure(s) (4 executed) with bare pytest at 92 passed. Until #180 lands, this
branch alone is 1 error and cannot import; every measurement below was taken
on the merged tree.

The defect

Eight tests, each shelling out to make, each opening with

    missing = _toolchain_missing()
    if missing:
        print(f"SKIP: {missing} not on PATH")
        return

A pytest test that returns None without asserting is a pass, and the
module is pinned in pytest.ini testpaths — so on a machine without cc65
bare pytest at the repo root goes green while asserting the
build/flags.stamp invariant it never checked. The SKIP: lines are not even
visible: pytest captures stdout on a passing test and discards it.

Red / green

New tools/test_flags_stamp_skip_is_loud.py 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 happens to be installed.

RED, subject module at the pre-fix revision:

=== involuntary skip must be loud (#177) ===
  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)

with the subject's own output captured in the failure message — the exact shape
#177 reports (SKIP: ca65 not on PATH / ok × 8, then PASSED: 0 failure(s)).

GREEN, after the fix: PASSED: 0 failure(s) (4 executed), and identically
against #180's amended module.

Both channels after the fix, toolchain stripped:

$ python3 tools/test_build_flags_stamp.py ; echo $?
============================================================
COULD NOT RUN: ca65 not on PATH -- every case here shells out to `make`
  0 of 9 checks executed
  this run certifies NOTHING about build/flags.stamp — …
  set C64_ALLOW_SKIP=1 to accept an unverified run (exit 0 instead of 2)
  exit 2 = could not run (1 would mean a check failed)
============================================================
2

$ pytest -q tools/test_build_flags_stamp.py
9 failed in 0.11s

$ C64_ALLOW_SKIP=1 pytest -q tools/test_build_flags_stamp.py
SKIPPED [9] tools/_skip_policy.py: COULD NOT RUN: ca65 not on PATH … --
  this run certifies NOTHING about build/flags.stamp … [C64_ALLOW_SKIP=1 set]
9 skipped in 0.02s

With cc65 present nothing changes: bare pytest at the repo root is
92 passed on the merged tree described above.

Shape of the fix

lane before after
pytest bare return → pass require() raises SkipPolicyError; the reason string carries the vacuity warning, because under -ra the reason is the only surviving channel
standalone PASSED: 0 failure(s), exit 0 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 none 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 (asserted)

main() hoists the toolchain check out of the loop and calls cannot_run()
directly, rather than letting eight require() calls report eight identical
failures for one missing binary. Review confirmed this is also why the
standalone lane's behaviour is unaffected by the _skip_policy.py revision:
it never reaches require().

test_the_strip_actually_strips guards the guard — a PATH-stripping bug
would make every other assertion run against a machine that can build, and
they would all pass having tested nothing: the same vacuity, one level up.

Known follow-up, not a blocker

_toolchain_missing() checks only ca65/ld65. An uninitialised submodule
(no libs/nistcurves working tree) is a different failure: farm.make()
asserts and the suite reports eight loud failures rather than a named
COULD NOT RUN. Loud is the correct half; the naming is not. Worth a separate
pass, and it does not affect what this PR pins — #177's own repro is verbatim a
PATH strip producing seven silent passes.

🤖 Generated with Claude Code

@JC-000
JC-000 force-pushed the fix/177-flags-stamp-skip-loud branch from b8db5a8 to 3a2024a Compare September 5, 2026 23:28
JC-000 added a commit that referenced this pull request Sep 5, 2026
#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>
JC-000 and others added 2 commits September 5, 2026 18:41
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>
@JC-000
JC-000 force-pushed the fix/177-flags-stamp-skip-loud branch from 3a2024a to 3619583 Compare September 5, 2026 23:41
@JC-000
JC-000 merged commit 94183fa into fix/174-dry-run-non-destructive Sep 6, 2026
JC-000 added a commit that referenced this pull request Sep 6, 2026
recover: land #186 on master (it merged into #183's branch, not master)
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