Rewrite script/test to the canonical pattern (30s timeout, -race -cover, conditional verbose rerun) #67

New Issue

clawbot · 2026-08-09T03:40:00+02:00

clawbot commented

2026-08-09 03:40:00 +02:00

Context

script/test:20 is a single line that violates the test policy three ways:

go test -v --timeout 10s ./...

Timeout is 10s, policy requires 30s. This is the flaky-test problem:
the gpg tests generate real RSA-2048 keys via a gpg subprocess, and on a
slow or entropy-starved machine (CI, a container, a loaded laptop) that
keygen races the 10s budget for the entire ./... run. The suite takes
~3s locally, so the failure is invisible until it is not.
Always verbose. Policy requires the quiet-first / verbose-on-failure
pattern so CI and docker build output stays readable on success and
fully diagnostic on failure.
No -race, no -cover. The repo runs concurrent scanners and
checkers with progress goroutines and a test-suite mutex around a
process-global logger. That is exactly the code where the race detector
earns its keep, and it has never been run.

Definition of done

script/test's main() matches the canonical pattern:

go test -timeout 30s -race -cover ./... || \
    { echo "--- Rerunning with -v for details ---"; \
      go test -timeout 30s -race -v ./...; exit 1; }

The exit 1 is present, so the target fails even if a flaky test happens
to pass on the verbose rerun.
make test passes with -race enabled. Any data race the detector finds
is a real bug and is fixed in this PR, not silenced.
make test completes in under 20 seconds wall-clock, per policy.
make check passes. TODO.md updated in the same commit.

Implementation requirements

Raising the timeout is necessary but not sufficient. The underlying
unbounded gpg subprocess is tracked in #62; do not close that by widening
the budget here, and do not duplicate its fix here.
If -race surfaces a race in the CLI's process-global logger or in the
scanner/checker progress goroutines, fix the race. Do not remove -race,
do not add t.Skip, and do not serialize the tests further to hide it.
-race roughly doubles runtime. If that pushes the suite past 20s, the
answer is to make the slow tests cheaper (the gpg tests each generate a
fresh RSA-2048 key — a shared per-package key fixture would cut most of
it), not to drop the flag.
Keep the existing ensure_pb behavior in this PR; the fact that it can
mutate tracked files during make check is tracked separately.
Commit title must end with (closes #67).

## Context `script/test:20` is a single line that violates the test policy three ways: ```sh go test -v --timeout 10s ./... ``` 1. **Timeout is 10s, policy requires 30s.** This is the flaky-test problem: the gpg tests generate real RSA-2048 keys via a `gpg` subprocess, and on a slow or entropy-starved machine (CI, a container, a loaded laptop) that keygen races the 10s budget for the *entire* `./...` run. The suite takes ~3s locally, so the failure is invisible until it is not. 2. **Always verbose.** Policy requires the quiet-first / verbose-on-failure pattern so CI and `docker build` output stays readable on success and fully diagnostic on failure. 3. **No `-race`, no `-cover`.** The repo runs concurrent scanners and checkers with progress goroutines and a test-suite mutex around a process-global logger. That is exactly the code where the race detector earns its keep, and it has never been run. ## Definition of done - `script/test`'s `main()` matches the canonical pattern: ```sh go test -timeout 30s -race -cover ./... || \ { echo "--- Rerunning with -v for details ---"; \ go test -timeout 30s -race -v ./...; exit 1; } ``` - The `exit 1` is present, so the target fails even if a flaky test happens to pass on the verbose rerun. - `make test` passes with `-race` enabled. Any data race the detector finds is a real bug and is fixed in this PR, not silenced. - `make test` completes in under 20 seconds wall-clock, per policy. - `make check` passes. `TODO.md` updated in the same commit. ## Implementation requirements - Raising the timeout is necessary but **not** sufficient. The underlying unbounded gpg subprocess is tracked in #62; do not close that by widening the budget here, and do not duplicate its fix here. - If `-race` surfaces a race in the CLI's process-global logger or in the scanner/checker progress goroutines, fix the race. Do not remove `-race`, do not add `t.Skip`, and do not serialize the tests further to hide it. - `-race` roughly doubles runtime. If that pushes the suite past 20s, the answer is to make the slow tests cheaper (the gpg tests each generate a fresh RSA-2048 key — a shared per-package key fixture would cut most of it), not to drop the flag. - Keep the existing `ensure_pb` behavior in this PR; the fact that it can mutate tracked files during `make check` is tracked separately. - Commit title must end with ` (closes #67)`.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:40:00 +02:00

clawbot referenced this issue

2026-08-09 04:22:38 +02:00

Update golangci-lint to v2.12.2 with canonical config (closes #60) #59

clawbot referenced this issue

2026-08-09 07:39:41 +02:00

script/cibuild reports a green it did not earn: make check is served from Docker cache #89

clawbot referenced this issue

2026-08-09 07:41:07 +02:00

Update golangci-lint to v2.12.2 with canonical config (closes #60) #59

Sign in to join this conversation.