REPO_POLICIES.md mandates a 30s test timeout that is too tight to be safe #101

New Issue

clawbot · 2026-08-09T16:13:06+02:00

clawbot commented

2026-08-09 16:13:06 +02:00

REPO_POLICIES.md:192 says "Add a 30-second timeout", and the canonical
recipe at :212-214 uses -timeout 30s. Measurement in vaultik says that
value is too tight, so this is a proposed amendment to the org-canonical
policy text rather than a vaultik change.

Filing here because vaultik is where the evidence was gathered.
REPO_POLICIES.md is org-canonical and not editable by this repo, so
this needs sneak and an upstream edit.

Evidence

Measured on vaultik's suite (go test -race, per-package timeout):

measurement	value
slowest package, warm host	`internal/database` ~6.4s
slowest package, cold containerised	`internal/database` 8.1s
slowest package, ordinary cold run (independent reviewer)	`internal/database` 10.2s

Against the 10.2s observation, a 30s timeout is 2.9x headroom. That is
not a safety margin on a loaded or throttled CI runner — it is a flake
waiting for a slow day, and the failure mode is a timeout that looks like
a real defect and sends a correct change back for rework.

Note the 10.2s figure came from a reviewer's independent cold run and is
higher than the 8.1s the implementing agent measured. The true worst
case is not well characterised, which is itself an argument for generous
headroom.

The framing that matters

A -timeout is a hang backstop, not a performance budget. Its job is
to convert a deadlocked test into a stack dump instead of a wedged CI job.
It should therefore sit far above the slowest legitimate runtime, not
snugly above it. Nothing is gained by failing at 30s that is not gained by
failing at 120s, and the tight value costs real flakes.

One correction worth carrying upstream, since it is a plausible-sounding
claim that could justify the tight value: cold-cache compilation is not
charged against -timeout. The flag reaches the test binary as
-test.timeout and its clock starts inside testing.M.Run, after
compilation and linking. Verified twice independently in vaultik — a run
with an empty GOCACHE spent ~46s compiling and then reported
per-package durations within noise of warm. A shell timeout 30 go test ./... does include compilation, but that is a different mechanism.

The trade the policy should decide deliberately

script/test reruns verbosely on failure, so a hung package costs the
timeout twice. At 120s that is 120 + 120 = 240s, which pushes a
hang-case Docker build to roughly 6m30s — over the same policy's
5-minute build limit. So the policy currently contains two constraints
that a hang cannot satisfy simultaneously.

Options:

Raise the mandated timeout (vaultik has adopted 120s) and raise or
qualify the 5-minute build limit for the hang case.
Raise the timeout and drop the verbose rerun on timeout specifically,
so a hang costs the budget once.
Keep 30s and accept flakes on slow runners.

Recommendation: option 1 or 2. The hang case is rare and already
pathological; optimising the policy so a hang fits inside the build
limit at the cost of routine flakes is the wrong direction.

Definition of done

A decision recorded on the canonical REPO_POLICIES.md timeout text.
If amended, the canonical recipe at :212-214 updated to match, and
the interaction with the 5-minute build limit resolved rather than left
contradictory.
Consuming repos re-vendored, with vaultik's local 120s either matching
the new canonical value or its divergence documented.

Until then, vaultik ships -timeout 120s as a documented, deliberate
divergence from the canonical text, recorded in script/test's comment.

`REPO_POLICIES.md:192` says "Add a 30-second timeout", and the canonical recipe at `:212-214` uses `-timeout 30s`. Measurement in vaultik says that value is too tight, so this is a proposed amendment to the org-canonical policy text rather than a vaultik change. Filing here because vaultik is where the evidence was gathered. `REPO_POLICIES.md` is org-canonical and not editable by this repo, so this needs `sneak` and an upstream edit. ## Evidence Measured on vaultik's suite (`go test -race`, per-package timeout): | measurement | value | | --- | --- | | slowest package, warm host | `internal/database` ~6.4s | | slowest package, cold containerised | `internal/database` **8.1s** | | slowest package, ordinary cold run (independent reviewer) | `internal/database` **10.2s** | Against the 10.2s observation, a 30s timeout is **2.9x headroom**. That is not a safety margin on a loaded or throttled CI runner — it is a flake waiting for a slow day, and the failure mode is a timeout that looks like a real defect and sends a correct change back for rework. Note the 10.2s figure came from a reviewer's independent cold run and is *higher* than the 8.1s the implementing agent measured. The true worst case is not well characterised, which is itself an argument for generous headroom. ## The framing that matters A `-timeout` is a **hang backstop, not a performance budget**. Its job is to convert a deadlocked test into a stack dump instead of a wedged CI job. It should therefore sit far above the slowest legitimate runtime, not snugly above it. Nothing is gained by failing at 30s that is not gained by failing at 120s, and the tight value costs real flakes. One correction worth carrying upstream, since it is a plausible-sounding claim that could justify the tight value: **cold-cache compilation is not charged against `-timeout`.** The flag reaches the test binary as `-test.timeout` and its clock starts inside `testing.M.Run`, after compilation and linking. Verified twice independently in vaultik — a run with an empty `GOCACHE` spent ~46s compiling and then reported per-package durations within noise of warm. A shell `timeout 30 go test ./...` *does* include compilation, but that is a different mechanism. ## The trade the policy should decide deliberately `script/test` reruns verbosely on failure, so a hung package costs the timeout **twice**. At 120s that is 120 + 120 = 240s, which pushes a hang-case Docker build to roughly 6m30s — over the same policy's 5-minute build limit. So the policy currently contains two constraints that a hang cannot satisfy simultaneously. Options: 1. Raise the mandated timeout (vaultik has adopted **120s**) and raise or qualify the 5-minute build limit for the hang case. 2. Raise the timeout and drop the verbose rerun on timeout specifically, so a hang costs the budget once. 3. Keep 30s and accept flakes on slow runners. Recommendation: **option 1 or 2.** The hang case is rare and already pathological; optimising the policy so a *hang* fits inside the build limit at the cost of routine flakes is the wrong direction. ## Definition of done 1. A decision recorded on the canonical `REPO_POLICIES.md` timeout text. 2. If amended, the canonical recipe at `:212-214` updated to match, and the interaction with the 5-minute build limit resolved rather than left contradictory. 3. Consuming repos re-vendored, with vaultik's local 120s either matching the new canonical value or its divergence documented. Until then, vaultik ships `-timeout 120s` as a **documented, deliberate divergence** from the canonical text, recorded in `script/test`'s comment.

sneak was assigned by clawbot

2026-08-09 16:13:10 +02:00

clawbot referenced this issue

2026-08-09 16:13:27 +02:00

Make the test gate unfakeable and stop test-integration lying (closes #93) #98

clawbot referenced this issue from a commit

2026-08-09 16:18:26 +02:00

Make the test gate unfakeable and stop test-integration lying (closes #93)

clawbot referenced this issue

2026-08-09 16:21:32 +02:00

Make the test gate unfakeable and stop test-integration lying (closes #93) #98

clawbot referenced this issue