script/test omits -count=1, so Go's test cache can satisfy the gate without running tests #93
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: sneak/vaultik#93
Reference in New Issue
Block a user
Surfaced by the PR #92 review, which corrected a claim I had been relying
on. This is a fifth distinct way this repo's gate could report a green
it did not earn — the first four being #78, #80, #85, and #88.
The gap
script/testruns:No
-count=1. Go's own test cache is therefore live, and a cachedpackage prints:
That line counts as an
okline. So the evidence signal this repo hasbeen leaning on — "14
oklines means the suite really ran" — can besatisfied by a run in which no test executed at all.
This is one level below the Docker layer cache (#85). Fixing the layer
cache guarantees the
RUN make teststep re-executes; it does notguarantee
go testinside that step does any work, becauseGOCACHEbaked into earlier image layers survives into the re-executed step.
Why it has not bitten yet
Every verification recorded during #85, #91, and #92 explicitly counted
(cached)occurrences and found zero, so no landed verdict rests on acached test run. That was luck plus discipline, not a property of the
tooling — and relying on every future agent to count
(cached)by hand isthe same brittle defence that #88's VOID rule currently depends on.
Also correcting the record
PR #92 argued that differing per-package durations prove real execution
because "a replayed layer reproduces its output byte for byte". That
mechanism is wrong. Under
BUILDKIT_PROGRESS=plaina replayed layerprints
CACHEDand no stdout at all, so zerooklines already rules outlayer replay; durations add nothing there.
Where durations genuinely help is exactly this issue — distinguishing a
real
ok pkg 5.8sfrom a cachedok pkg (cached). The reviewer alsofound the primitive is not reliable per-package:
internal/pidlockmeasured 1.016s on two independently-executed runs, identical to the
millisecond. Applied to the whole vector it is informative; applied to a
single package it is not.
Definition of done
script/testcannot be satisfied by Go's test cache. Preferred:add
-count=1. If that is judged too costly for the local inner loop,the alternative is to keep the cache locally but force
-count=1inthe containerised/CI path — state which was chosen and why.
make testtwice back to back and confirm thesecond run shows zero
(cached)markers and real per-packagedurations. Paste both runs.
-count=1disables test-result caching, so warmlocal
make testwill get slower. Report before/after wall time sothe trade-off is explicit rather than discovered later.
script/check,test-coverage, ortest-integration(see #69 — that target iscurrently a no-op), and fix consistently or say why not.
script/cibuildexits 0, verified as a genuine run.Note
Consider this alongside #69, which reviews the
-timeout 30sper-packagebudget. Both concern
script/test, and-count=1makes every run a coldtest run, which interacts directly with that timeout — doing them
together avoids landing a timeout value tuned against cached runs.
Plan
Implementing this together with #69 as a single PR, since both change
script/testand the timeout in #69 has to be chosen against theuncached behaviour this issue introduces.
Defect reproduced first
Before changing anything, on
mainat3f9c2e5, two back-to-backmake testruns:oklines(cached)markersRun 2 produced the full 14-
okevidence signal in 0.42 seconds whileexecuting no test whatsoever. Exactly as described.
What I will do
-count=1toscript/test(DoD item 1, the preferred option) —unconditionally, not only in the containerised path. Rationale to be
confirmed against the measured cost below, but the local gate is the
one the pre-commit hook runs, so leaving it forgeable locally keeps
the hole where it is most likely to be hit.
script/checkcallsscript/test, so it inherits thefix;
test-coverageis a raw inlinego testand gets-count=1too;
test-integrationis resolved by #69.run rather than warm host numbers.
One claim I intend to check rather than inherit
The comment on #69 states that "compilation of a large
-racebuild ona cold cache is charged against that same 30-second budget". I do not
believe that is how
go test -timeoutworks: the flag is passed to thebuilt test binary as
-test.timeoutand its clock starts insidetesting.M.Run, after compilation and linking are already finished. Thesibling-repo failure cited was
timeout 30 go test ./...— a shelltimeoutwrapping the whole invocation, which genuinely does includecompilation. That is a different mechanism from
go test -timeout.I will settle it empirically: a cold-cache containerised run should
report per-package durations close to the warm host durations if
compilation is excluded, and much larger ones if it is included. The
timeout value will be chosen from whatever that shows, and the PR will
state the result either way.