What
tests/scripts/test_cpu_x86_llamacpp_floor.py::test_a_contended_leg_is_discarded_and_never_summarised
is not hermetic: its verdict depends on the real load average of the box running
it, not on the harness behaviour it means to pin.
Observed during an unrelated records-only run of scripts/agent-preflight.sh --staged
(branch row/model-matrix-recipe-backfill, diff touched only .agents/model-matrix.md,
.agents/roadmap_v1.md, docs/FEATURES.md, scripts/check-agent-record.py):
FAIL test_cpu_x86_llamacpp_floor
F.........
FAIL: test_a_contended_leg_is_discarded_and_never_summarised
File "tests/scripts/test_cpu_x86_llamacpp_floor.py", line 163
self.assertEqual(got.returncode, 2, got.stdout + got.stderr)
AssertionError: 4 != 2 : ours rep=1 START load=63.07 38.78 24.40 builders=0
ours rep=1 END exit=0 load=63.07 38.78 24.40 builders=0 foreign=0%
ours rep=1 DISCARDED (exit=0 builders_after=0 foreign=0%)
pair rep=1 RETRY
The same test, same tree, at load average: 21.91 a few minutes later:
Ran 10 tests in 18.353s
OK
Why it matters
The test drives the harness with FOREIGN_MAX=-1 to force every leg to be
discarded, and asserts the harness gives up with exit 2 (GIVING_UP). At high
real load the harness instead exits 4 (NO_QUIET_WINDOW) — the quiet-window
gate fires before the contention-discard path the test exists to cover. So under
load the assertion is not weakened, it is measuring a different code path
entirely, and the intended guarantee goes untested while the suite reports a
failure that looks like a code defect in whatever change happens to be in flight.
That is the expensive failure mode: an infrastructure-dependent test presents as a
verdict on the author's diff. In this instance the diff was eight markdown rows.
Scope
Make the test pin the discard path deterministically rather than racing the box —
e.g. neutralise the quiet-window gate for this case the way the case already
neutralises the foreign-CPU threshold (QUIET_BUSY / WAIT_TIMEOUT are already
harness-injectable, and the sibling
test_no_quiet_window_stops_instead_of_averaging_through_it covers exit 4 on its
own). Do not relax the exit-code assertion; the guarantee it names is the right
one, it is simply not reached under load.
This changes a test's semantics, so it takes the normal row / spec / fresh-review
path rather than an in-flow fix.
What
tests/scripts/test_cpu_x86_llamacpp_floor.py::test_a_contended_leg_is_discarded_and_never_summarisedis not hermetic: its verdict depends on the real load average of the box running
it, not on the harness behaviour it means to pin.
Observed during an unrelated records-only run of
scripts/agent-preflight.sh --staged(branch
row/model-matrix-recipe-backfill, diff touched only.agents/model-matrix.md,.agents/roadmap_v1.md,docs/FEATURES.md,scripts/check-agent-record.py):The same test, same tree, at
load average: 21.91a few minutes later:Why it matters
The test drives the harness with
FOREIGN_MAX=-1to force every leg to bediscarded, and asserts the harness gives up with exit 2 (
GIVING_UP). At highreal load the harness instead exits 4 (
NO_QUIET_WINDOW) — the quiet-windowgate fires before the contention-discard path the test exists to cover. So under
load the assertion is not weakened, it is measuring a different code path
entirely, and the intended guarantee goes untested while the suite reports a
failure that looks like a code defect in whatever change happens to be in flight.
That is the expensive failure mode: an infrastructure-dependent test presents as a
verdict on the author's diff. In this instance the diff was eight markdown rows.
Scope
Make the test pin the discard path deterministically rather than racing the box —
e.g. neutralise the quiet-window gate for this case the way the case already
neutralises the foreign-CPU threshold (
QUIET_BUSY/WAIT_TIMEOUTare alreadyharness-injectable, and the sibling
test_no_quiet_window_stops_instead_of_averaging_through_itcovers exit 4 on itsown). Do not relax the exit-code assertion; the guarantee it names is the right
one, it is simply not reached under load.
This changes a test's semantics, so it takes the normal row / spec / fresh-review
path rather than an in-flow fix.