script/test omitted -count=1, so Go's test result cache could satisfy
the gate outright. A cached package prints `ok <pkg> (cached)`, and
that is an `ok` line: two back-to-back `make test` runs on the parent
commit produced the same 14 `ok` lines, the second in 0.42s with every
line marked (cached), having executed no test at all. The evidence
signal this repo leans on was therefore forgeable. This sits one level
below the Docker layer cache #85 addressed: CHECK_EPOCH forces the
`RUN make test` step to re-execute, but a GOCACHE baked into an earlier
image layer would survive into the re-executed step, so the step can
re-run and still do no work.
Add -count=1 unconditionally rather than only in the containerised
path. The pre-commit hook runs this same script, and a gate that is
honest only in CI is dishonest exactly where people rely on it most.
test-coverage gets the same flag -- a coverage profile assembled from
cached results describes a run that did not happen -- and script/check
inherits it by calling script/test.
Delete make test-integration rather than making it real (option (b) of
issue #69). No file in the repo carries a build tag, so -tags=integration
selected nothing extra and the target was an exact duplicate of make
test, while internal/vaultik/integration_test.go ran unconditionally on
every make test -- the opposite of what a reader of the Makefile would
conclude. Tagging a subset was rejected because the entire suite runs in
well under a minute, so gating would save seconds in exchange for a
build-tag scheme and a second CI path that must be kept wired up; in a
repo that has now found five distinct ways for a gate to report an
unearned green, a mechanism whose failure mode is "some tests silently
stopped running" is a bad trade. Deleting it also means nothing can
drop out of CI coverage, since nothing is conditional. The Makefile and
README now say plainly that make test runs everything.
Raise -timeout from 30s to 120s, after measuring rather than assuming.
The standing claim that cold-cache compilation is charged against
-timeout is false: -timeout reaches the test binary as -test.timeout
and its clock starts inside testing.M.Run, after compilation and
linking. A containerised run with an empty GOCACHE spent 46s compiling
and still reported per-package durations within noise of a warm host
run. The real exposure was margin, not compilation: against the slowest
package's worst observed time, 10.2s for internal/database on a cold
run, 30s left only 2.9x, thin for a loaded CI runner, and each fresh
measurement of that package has come in above the last. -timeout is a
hang backstop, not a performance budget, so it should sit far above the
slowest legitimate runtime; 120s leaves about 12x while still bounding
a hung package, including the verbose rerun, to a few minutes.
120s is a deliberate divergence from REPO_POLICIES.md:192, which
mandates a 30-second timeout, and from that file's canonical Go recipe
at :212-214. REPO_POLICIES.md is org-canonical and not editable from
this repo, so the divergence and its reasoning are recorded in
script/test's comment and in TODO.md, and issue #101 proposes amending
the policy text upstream. That issue also carries the trade this
surfaces: the verbose rerun makes a hung package pay the timeout twice,
which at 120s puts a hang-case Docker build over the same policy's
five-minute build limit.
Both invocations in script/test now share one run_tests function so the
quiet run and the verbose rerun cannot drift apart in flags.
Measurements and the full verification are recorded once, on the pull
request, and deliberately not restated here or in TODO.md.
