script/test always runs with -v instead of the conditional verbose rerun pattern #103

New Issue

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

clawbot commented

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

REPO_POLICIES.md requires the "conditional verbose rerun" pattern: run tests without -v first, and only if they fail, automatically rerun with -v to show full output. The stated purpose is that "this keeps CI logs and docker build output clean on success (just package/suite summaries) while providing full diagnostic detail on failure."

Current state (audited against `origin/main`, commit `9347a28`)

script/test:8 runs, unconditionally:

go test -v -race -timeout 30s -cover ./...

-v is always on and there is no || fallback/rerun block anywhere in script/test or the Makefile. Makefile:22-23 is a correct thin shim (@script/test), so the fix belongs in script/test.

The -timeout 30s part is already correct and must be preserved.

The practical cost is real: a green make check on main currently prints a --- PASS: line for every individual test case plus every slog line the tests emit, burying the actual result. The canonical form in REPO_POLICIES.md is:

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

Definition of done

script/test runs go test without -v on the first attempt, keeping -race, -timeout 30s, and -cover.
On failure it prints a marker line and reruns the identical command with -v added.
After the rerun it exits non-zero unconditionally. REPO_POLICIES.md is explicit about why: "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. The rerun exists solely for diagnostic output." Do not let the rerun's exit status decide the outcome.
script/test remains POSIX sh — #!/bin/sh, set -eu, no bashisms — because it runs inside minimal alpine containers with no bash. Note that set -e interacts with || blocks; make sure the failure path is actually reached and the script does not abort before the rerun.
It still locates the repo root with the existing $(cd "$(dirname "$0")/.." && pwd -P) idiom and cds there before running.
Verify both paths by hand before opening the PR: make test on a green tree prints the concise summary and exits 0; make test with a deliberately broken test prints the rerun marker, shows verbose output, and exits non-zero. Revert the deliberate breakage before committing — do not commit it.
make check is green, and TODO.md is updated in the same commit as the work.

The finishing commit's title must end with (closes #N) referencing this issue.

Out of scope

Do not change the test timeout, do not add -short, and do not add any skip flags — TESTING.md forbids both -short and timeout inflation, and DNS mocking is banned repo-wide. This issue is purely about verbosity of output.

`REPO_POLICIES.md` requires the "conditional verbose rerun" pattern: run tests **without** `-v` first, and only if they fail, automatically rerun with `-v` to show full output. The stated purpose is that "this keeps CI logs and `docker build` output clean on success (just package/suite summaries) while providing full diagnostic detail on failure." ## Current state (audited against `origin/main`, commit `9347a28`) `script/test:8` runs, unconditionally: ```sh go test -v -race -timeout 30s -cover ./... ``` `-v` is always on and there is no `||` fallback/rerun block anywhere in `script/test` or the `Makefile`. `Makefile:22-23` is a correct thin shim (`@script/test`), so the fix belongs in `script/test`. The `-timeout 30s` part is already correct and must be preserved. The practical cost is real: a green `make check` on `main` currently prints a `--- PASS:` line for every individual test case plus every `slog` line the tests emit, burying the actual result. The canonical form in `REPO_POLICIES.md` is: ```makefile test: @go test -timeout 30s -race -cover ./... || \ { echo "--- Rerunning with -v for details ---"; \ go test -timeout 30s -race -v ./...; exit 1; } ``` ## Definition of done 1. `script/test` runs `go test` **without** `-v` on the first attempt, keeping `-race`, `-timeout 30s`, and `-cover`. 2. On failure it prints a marker line and reruns the identical command with `-v` added. 3. After the rerun it exits non-zero **unconditionally**. `REPO_POLICIES.md` is explicit about why: "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. The rerun exists solely for diagnostic output." Do not let the rerun's exit status decide the outcome. 4. `script/test` remains POSIX `sh` — `#!/bin/sh`, `set -eu`, no bashisms — because it runs inside minimal alpine containers with no bash. Note that `set -e` interacts with `||` blocks; make sure the failure path is actually reached and the script does not abort before the rerun. 5. It still locates the repo root with the existing `$(cd "$(dirname "$0")/.." && pwd -P)` idiom and `cd`s there before running. 6. Verify both paths by hand before opening the PR: `make test` on a green tree prints the concise summary and exits 0; `make test` with a deliberately broken test prints the rerun marker, shows verbose output, and exits non-zero. Revert the deliberate breakage before committing — do not commit it. 7. `make check` is green, and `TODO.md` is updated in the same commit as the work. The finishing commit's title must end with ` (closes #N)` referencing this issue. ## Out of scope Do not change the test timeout, do not add `-short`, and do not add any skip flags — `TESTING.md` forbids both `-short` and timeout inflation, and DNS mocking is banned repo-wide. This issue is purely about verbosity of output.

clawbot added this to the 1.0 milestone 2026-08-09 03:37:37 +02:00

clawbot referenced this issue

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

1.0/mvp #66

clawbot referenced this issue

2026-08-10 15:58:57 +02:00

next #136

clawbot commented

2026-08-10 15:59:51 +02:00

[manager] Satisfied on next by 6f6bf3a (#136), as a side effect of the test-cache work — script/test's primary run was unconditionally -v, so the pattern was absent rather than merely at risk.

All seven items of the definition of done verified by independent review, including the set -e/|| interaction this issue warned about. One deliberate deviation: -timeout 30s here is superseded by the 90s backstop landed in 9cb2c2b under the 60s/20s org ruling.

Left open rather than closed, since the fix is on next and not yet on main, and the commit subject closes only #139. Close this by hand when next merges. Do not re-implement it.

**[manager] Satisfied on `next` by `6f6bf3a`** (https://git.eeqj.de/sneak/dnswatcher/pulls/136), as a side effect of the test-cache work — `script/test`'s primary run was unconditionally `-v`, so the pattern was absent rather than merely at risk. All seven items of the definition of done verified by independent review, including the `set -e`/`||` interaction this issue warned about. One deliberate deviation: `-timeout 30s` here is superseded by the `90s` backstop landed in `9cb2c2b` under the 60s/20s org ruling. Left open rather than closed, since the fix is on `next` and not yet on `main`, and the commit subject closes only https://git.eeqj.de/sneak/dnswatcher/issues/139. Close this by hand when `next` merges. **Do not re-implement it.**

clawbot referenced this issue

2026-08-10 15:59:55 +02:00

make test is served from Go's test cache, so a repeat green proves no DNS was queried #139

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/dnswatcher#103

script/test always runs with -v instead of the conditional verbose rerun pattern #103

Current state (audited against origin/main, commit 9347a28)

Definition of done

Out of scope

Current state (audited against `origin/main`, commit `9347a28`)