make test: adopt the mandated timeout/race/cover + conditional verbose rerun pattern #2

New Issue

clawbot · 2026-08-09T03:38:27+02:00

clawbot commented

2026-08-09 03:38:27 +02:00

Problem

REPO_POLICIES.md mandates a specific shape for the test: target. rgoue's
current target is a bare invocation:

test:
	go test $(GO_PKGS)

This diverges from policy in four ways: no -timeout 30s, no -race, no
-cover, and no conditional verbose rerun on failure.

This is a genuine policy divergence, not a consequence of this repo's
scaffold exemption. The exemption covers the Dockerfile, CI config, and
REPO_POLICIES.md itself; it does not cover the content of the make targets
that do exist.

Policy text

REPO_POLICIES.md:

> make test must complete in under 20 seconds. Add a 30-second timeout in
> the Makefile.

> make test should use the conditional verbose rerun pattern. Run tests
> without -v (verbose) first. If tests fail, automatically rerun with -v
> to show full output. This keeps CI logs and docker build output clean on
> success while providing full diagnostic detail on failure.

The policy gives the Go form verbatim:

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

> The exit 1 ensures the target always fails after a rerun — the first run
> already proved the tests are broken, so the build must not pass even if a
> flaky test happens to succeed on the second attempt.

Definition of done

The Makefile test: target implements the policy pattern above, keeping
this repo's existing $(GO_PKGS) variable rather than hardcoding ./....
make test passes and completes in under 20 seconds wall clock.
make check is fully green (fmt-check, lint, test).
-race is genuinely enabled and the suite passes under it. If the race
detector surfaces a real data race, that is a finding to report, not to
paper over — stop and report rather than dropping the flag.
TODO.md updated in the same commit (rotate the step per the repo's
documented workflow).
Commit title ends with (closes #N).

Implementation requirements

make targets / script/ entrypoints only. Do not invoke go test
directly to verify; verify through make test and make check.
Do not modify .golangci.yml — it is byte-identical to the org
canonical config and may only change with sneak's explicit permission.
Behavior of the game must not change; this is a build-tooling commit only.
Report the measured make test wall-clock time in the PR body.

Out of scope

Adding a Dockerfile, CI config, or script/ entrypoints (this repo is
explicitly exempt — see TODO.md Future Steps note 3).
The golangci-lint version pin (tracked separately).

## Problem `REPO_POLICIES.md` mandates a specific shape for the `test:` target. rgoue's current target is a bare invocation: ```makefile test: go test $(GO_PKGS) ``` This diverges from policy in four ways: no `-timeout 30s`, no `-race`, no `-cover`, and no conditional verbose rerun on failure. This is a genuine policy divergence, **not** a consequence of this repo's scaffold exemption. The exemption covers the Dockerfile, CI config, and `REPO_POLICIES.md` itself; it does not cover the content of the make targets that do exist. ## Policy text `REPO_POLICIES.md`: > `make test` must complete in under 20 seconds. Add a 30-second timeout in > the Makefile. > **`make test` should use the conditional verbose rerun pattern.** Run tests > without `-v` (verbose) first. If tests fail, automatically rerun with `-v` > to show full output. This keeps CI logs and `docker build` output clean on > success while providing full diagnostic detail on failure. The policy gives the Go form verbatim: ```makefile test: @go test -timeout 30s -race -cover ./... || \ { echo "--- Rerunning with -v for details ---"; \ go test -timeout 30s -race -v ./...; exit 1; } ``` > The `exit 1` ensures the target always fails after a rerun — the first run > already proved the tests are broken, so the build must not pass even if a > flaky test happens to succeed on the second attempt. ## Definition of done 1. The Makefile `test:` target implements the policy pattern above, keeping this repo's existing `$(GO_PKGS)` variable rather than hardcoding `./...`. 2. `make test` passes and completes in **under 20 seconds** wall clock. 3. `make check` is fully green (`fmt-check`, `lint`, `test`). 4. `-race` is genuinely enabled and the suite passes under it. If the race detector surfaces a real data race, that is a **finding to report, not to paper over** — stop and report rather than dropping the flag. 5. `TODO.md` updated in the same commit (rotate the step per the repo's documented workflow). 6. Commit title ends with ` (closes #N)`. ## Implementation requirements - `make` targets / `script/` entrypoints only. Do **not** invoke `go test` directly to verify; verify through `make test` and `make check`. - Do **not** modify `.golangci.yml` — it is byte-identical to the org canonical config and may only change with sneak's explicit permission. - Behavior of the game must not change; this is a build-tooling commit only. - Report the measured `make test` wall-clock time in the PR body. ## Out of scope - Adding a Dockerfile, CI config, or `script/` entrypoints (this repo is explicitly exempt — see `TODO.md` Future Steps note 3). - The golangci-lint version pin (tracked separately).

clawbot referenced this issue

2026-08-09 03:38:53 +02:00

MEMORY.md and TODO.md carry stale claims that actively misdirect agents #3

clawbot commented

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

Implementation plan

Branching from main (d6cd418) into a temporary worktree on branch
make-test-policy-pattern.

1. Makefile test: target — replace the bare invocation with the policy
pattern, keeping this repo's $(GO_PKGS) rather than hardcoding ./...:

test:
	@go test -timeout 30s -race -cover $(GO_PKGS) || \
		{ echo "--- Rerunning with -v for details ---"; \
		  go test -timeout 30s -race -v $(GO_PKGS); exit 1; }

Notes on the shape: the recipe is @-prefixed and the rerun branch ends in
exit 1 so a flaky second-run pass still fails the target, exactly as the
policy text specifies. No other target changes; check: fmt-check lint test
already picks this up.

2. Verification — make test and make check only; no direct
go test / go vet / golangci-lint invocations, per the issue's
implementation requirements. I will time make test with the new flags and
report the wall-clock number against the 20s policy budget.

3. Race detector risk — this is the substantive part of the change, not
the Makefile edit. The suite drives a tcell terminal layer and has
os.Exit-path tests, so -race is the first time concurrent access in the
daemon/terminal code gets checked. If the detector reports a genuine data
race I will stop, post the full detector output here and to the parent, and
not drop -race, add a //nolint, or otherwise paper over it. Likewise if
the suite exceeds 20s I will report the measurement rather than quietly
removing a flag.

4. TODO.md — updated in the same commit as the Makefile change. Per the
precedent set by the golangci-v2.12.2 work (63d1e79), out-of-band policy
work adds an entry at the top of Completed Steps and leaves Next Step alone:
the current Next Step is "broaden unit test coverage", which is not this work,
so rotating it would falsely mark it done and would promote a deferred
Future Step into Next. Markdown formatted with make fmt.

Out of scope, explicitly untouched: .golangci.yml (byte-identical to
canonical, needs sneak's permission), Dockerfile / CI / script/ entrypoints
(repo is exempt per TODO.md Future Steps note 3), the golangci-lint version
pin, and any game behavior — this is a build-tooling commit only.

Commit title will end with (closes #2); PR against main, labeled
needs-review, assigned to clawbot.

## Implementation plan Branching from `main` (d6cd418) into a temporary worktree on branch `make-test-policy-pattern`. **1. Makefile `test:` target** — replace the bare invocation with the policy pattern, keeping this repo's `$(GO_PKGS)` rather than hardcoding `./...`: ```makefile test: @go test -timeout 30s -race -cover $(GO_PKGS) || \ { echo "--- Rerunning with -v for details ---"; \ go test -timeout 30s -race -v $(GO_PKGS); exit 1; } ``` Notes on the shape: the recipe is `@`-prefixed and the rerun branch ends in `exit 1` so a flaky second-run pass still fails the target, exactly as the policy text specifies. No other target changes; `check: fmt-check lint test` already picks this up. **2. Verification** — `make test` and `make check` only; no direct `go test` / `go vet` / `golangci-lint` invocations, per the issue's implementation requirements. I will time `make test` with the new flags and report the wall-clock number against the 20s policy budget. **3. Race detector risk** — this is the substantive part of the change, not the Makefile edit. The suite drives a tcell terminal layer and has `os.Exit`-path tests, so `-race` is the first time concurrent access in the daemon/terminal code gets checked. If the detector reports a genuine data race I will stop, post the full detector output here and to the parent, and not drop `-race`, add a `//nolint`, or otherwise paper over it. Likewise if the suite exceeds 20s I will report the measurement rather than quietly removing a flag. **4. `TODO.md`** — updated in the same commit as the Makefile change. Per the precedent set by the `golangci-v2.12.2` work (63d1e79), out-of-band policy work adds an entry at the top of Completed Steps and leaves Next Step alone: the current Next Step is "broaden unit test coverage", which is not this work, so rotating it would falsely mark it done and would promote a deferred Future Step into Next. Markdown formatted with `make fmt`. **Out of scope, explicitly untouched**: `.golangci.yml` (byte-identical to canonical, needs sneak's permission), Dockerfile / CI / `script/` entrypoints (repo is exempt per TODO.md Future Steps note 3), the golangci-lint version pin, and any game behavior — this is a build-tooling commit only. Commit title will end with ` (closes #2)`; PR against `main`, labeled `needs-review`, assigned to `clawbot`.

clawbot referenced this issue from a commit

2026-08-09 03:42:29 +02:00

build: adopt the mandated test target pattern (closes #2)

clawbot referenced a pull request that will close this issue

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

Adopt the mandated test target pattern (closes #2) #9

clawbot referenced this issue

2026-08-09 03:46:16 +02:00

Add a `make cover` target so coverage is measurable under the make-targets-only rule #17

clawbot referenced this issue

2026-08-09 03:49:25 +02:00

Adopt the mandated test target pattern (closes #2) #9