Repository navigation
Description
Symptom
tools/test_build_flags_stamp.py has seven tests. On a machine without ca65
or ld65 on PATH, all seven report as passes — not skips — and the module
is pinned in pytest.ini testpaths, so bare pytest at the repo root goes
green having verified nothing about the build-flag invariant.
Measured, with the toolchain removed from PATH:
$ env PATH="/tmp/nobin:/usr/bin:/bin" pytest tools/test_build_flags_stamp.py -q
....... [100%]
7 passed in 0.01s
Seven dots. 0.01 s is the only tell — and the SKIP: lines are not even
visible, because pytest captures stdout on a passing test and discards it.
The standalone runner is no better:
$ env PATH="/tmp/nobin:/usr/bin:/bin" python3 tools/test_build_flags_stamp.py
=== build/flags.stamp ===
SKIP: ca65 not on PATH
ok test_backend_flip_removes_the_other_backends_prg
SKIP: ca65 not on PATH
ok test_flag_change_without_clean_matches_a_clean_build
SKIP: ca65 not on PATH
ok test_https_host_change_is_still_incremental
SKIP: ca65 not on PATH
ok test_profile_flip_without_clean_matches_a_clean_build
SKIP: ca65 not on PATH
ok test_stamp_records_the_whole_command_line
SKIP: ca65 not on PATH
ok test_unchanged_flags_rebuild_nothing
SKIP: ca65 not on PATH
ok test_vic_blank_flip_without_clean_matches_a_clean_build
PASSED: 0 failure(s)
EXIT=0
PASSED: 0 failure(s) for a run in which zero assertions executed.
Mechanism
_toolchain_missing() (tools/test_build_flags_stamp.py:77-82) returns the name
of the first absent tool. Each of the seven tests opens with the same four lines
— a print and a bare return:
missing = _toolchain_missing()
if missing:
print(f"SKIP: {missing} not on PATH")
returnSites, one per test:
| test | def | guard |
|---|---|---|
test_flag_change_without_clean_matches_a_clean_build |
:133 | :139-142 |
test_profile_flip_without_clean_matches_a_clean_build |
:165 | :172-175 |
test_vic_blank_flip_without_clean_matches_a_clean_build |
:191 | :193-196 |
test_backend_flip_removes_the_other_backends_prg |
:212 | :225-228 |
test_unchanged_flags_rebuild_nothing |
:244 | :246-249 |
test_https_host_change_is_still_incremental |
:266 | :274-277 |
test_stamp_records_the_whole_command_line |
:300 | :307-310 |
A pytest test function that returns None without asserting is a pass, so
the involuntary skip is laundered into green on both channels. The standalone
main() (:332-347) has the same hole from the other side: it treats "no
exception raised" as ok, so a test that ran nothing is indistinguishable from
one that ran everything.
This is precisely the class #158 closed (audit commit 7497e48) and #157
reintroduced (#165, fixed by PR #172): an involuntary skip is a failure; an
explicit skip is allowed but must never be silent. A missing toolchain is
involuntary.
Why testpaths makes this worse than an ordinary silent skip
pytest.ini pins this module in testpaths, and
tools/test_pytest_boundary.py enforces that pinning in both directions — so it
cannot simply be dropped from the set. Bare pytest at the root is the one
command a newcomer or a CI container is most likely to run, and a container
without cc65 is the most likely place for the toolchain to be absent. The
suite's green there is not merely uninformative; it actively asserts that the
build/flags.stamp invariant holds.
That invariant is load-bearing. It is what closes CLAUDE.md's two documented
silent-failure modes (a mixed link, and no link at all), and PR #168 leans on it
to argue the cardinal make clean rule is now "an enforced invariant rather than
a discipline". If the enforcement can evaporate on a PATH accident, that
argument is weaker than it reads.
Cross-link: this file is also the blocker in #174
Whoever fixes either issue touches this file.
test_backend_flip_removes_the_other_backends_prg
(tools/test_build_flags_stamp.py:212-241) deliberately runs make -n BACKEND=ip65 and asserts that build/c64-https.prg and build/tls13.o are
already gone. Its docstring is explicit:
The invalidation happens during parse, so it is visible here with
make -n:
no recipe runs, yet the stale PRG and objects must already be gone.
That makes the destructive dry run reported in #174 a currently-tested
property, not an oversight, so any guard added for #174 breaks this test.
Correction (2026-08-31): the test's stated reason for using -n is stale.
The docstring at :221-223 justifies it as exercising the ip65 flip without
linking ip65, "because ca65 resolves .incbin against the current directory".
That rule was retired by #116 and is one of the passages PR #168 deletes from
CLAUDE.md. src/net/ip65/ip65_blob.s:61 is a bare .incbin "ip65-c64.bin" with
no ../, resolved through Makefile:74
(CA65FLAGS += --bin-include-dir $(abspath $(IP65_BUILD))), and
Makefile:68-73 spells out the intent: absolute roots, no relative operand left
to escape through, "a missing blob now fails the assemble loudly instead of
quietly resolving somewhere else."
So the Farm can almost certainly link ip65, and #174's fix has an option this
issue originally foreclosed. FARM_LINKS
(tools/test_build_flags_stamp.py:67) symlinks ip65-build, so
$(abspath $(IP65_BUILD)) evaluated with make's CWD in the farm resolves
through that symlink to the real blob. Verified directly, without make: ca65
assembling src/net/ip65/ip65_blob.s inside a mkdtemp tree with the same
seven symlinks exits 0 and emits a 10,862 B object, embedding the real 6,951 B
ip65-build/ip65-c64.bin. The .lib archives CLAUDE.md warns about are needed
to build the blob (Makefile:826-829), not to consume it, and the blob is a
prebuilt artifact that is already present.
Not proven here: a full make BACKEND=ip65 link in the farm. That needs
make, which #174 makes unsafe to run casually, so it was not attempted. Every
input it would require — cfg/c64-https-ip65.cfg, src/net/ip65/, the blob —
is symlinked and present, so the remaining risk is low but real. Whoever fixes
#174 should try it: if a real ip65 link works in the farm, the test can observe
the invalidation with a genuine build instead of depending on -n being
destructive, and the conflict between #174 and this issue dissolves.
Fixing this issue (E) touches all seven guards; fixing #174 touches this one
test. Sequencing them together avoids two conflicting rewrites of the same file.
Worth recording for both: the suite's Farm
(tools/test_build_flags_stamp.py:84-96) is a tempfile.mkdtemp tree of
symlinks to src, cfg, tools, libs, ip65, ip65-build and Makefile
(FARM_LINKS, :67), with no build/ symlink — so the suite never writes
to the real build/, however destructive its make invocations are. That same
ip65/ip65-build pair is what makes the ip65 link available to it.
Fix
Adopt the shape PR #172 landed for tools/test_uci_data_acc.py:
- Raise, do not return. An absent toolchain becomes an
Unavailable-style
exception that pytest records as a failure and standalonemain()turns into
exit 2 ("could not run"), distinct from exit 1 ("a check failed"). - Keep a voluntary opt-out, and name it. An env var — the siblings use
C64_NET_TESTS_OPTIONAL=1andC64_UCI_TESTS_OPTIONAL=1, so
C64_BUILD_TESTS_OPTIONAL=1fits — converts it to an announced, exit-0 skip. - Put the warning in the pytest skip reason. Per test: an involuntary skip in test_uci_data_acc.py is a failure (#165) #172: under pytest the
reason string is the only channel that survives (-rais pinned in
pytest.iniaddopts); module stdout is swallowed. A reason of just
"ca65 not on PATH" reads as a bare7 skippedand loses the point. - Make
main()count executions, not exceptions.PASSED: 0 failure(s)
must be unreachable when zero tests executed.
Filed alongside the wider survey issue for this defect class; found during a
supervised agent session, 2026-08-31.
Activity
flags-stamp suite: an absent cc65 must be loud, not green (#177)
Take tools/_skip_policy.py from #178a instead of vendoring it
flags-stamp suite: an absent cc65 must be loud, not green (#177)
Take tools/_skip_policy.py from #178a instead of vendoring it
Merge pull request #186 from JC-000/fix/177-flags-stamp-skip-loud