fix: script/test conditional-verbose-rerun with -cover (closes #59) #115

Merged
clawbot merged 1 commits from issue-59-test-conditional-verbose into next 2026-09-21 19:43:22 +02:00
Collaborator

What changed

script/test now runs the suite non-verbose first
(go test -timeout 30s -race -cover ./...) and only reruns with -v on
failure before exiting non-zero, matching the conditional-verbose-rerun
pattern documented in REPO_POLICIES.md. Adds -cover to the first run per
the policy's Go example; keeps -race (from
#55) and the existing nix-shell CGO
fallback. Closes #59.

Why

Previously script/test always passed -v, so every green build dumped full
per-test output. The policy wants clean per-package summaries on success and
full detail only on failure.

What a reader might trip over

  • The whole compound command (first run || rerun-with--v; exit 1) is
    passed as one string to run_with_cgo_deps, so it runs unchanged under both
    the host sh -c path and the nix-shell --run fallback.
  • -cover is on the first run only; the -v rerun matches the policy example
    and exists solely for diagnostics.

Disclosures

  • Scope: script/test only, no Go changed. TDD's failing-test-first step does
    not apply to the test-runner wrapper itself and a shell-test harness would
    exceed the issue's scope; verified by running the gate instead.
  • The model script/test in the prompts repo has since migrated to a
    Docker---target test approach with no shell pattern; per the DoD I kept the
    shell pattern with the nix-shell fallback (Docker-only testing is
    #101 /
    #104 territory).
  • Gate: docker build . green — pinned make lint/make fmt-check and
    RUN make test (my changed script) all ran, not cached; success path showed
    clean coverage summaries and no -v rerun.

Model: opus-4-8

## What changed `script/test` now runs the suite non-verbose first (`go test -timeout 30s -race -cover ./...`) and only reruns with `-v` on failure before exiting non-zero, matching the conditional-verbose-rerun pattern documented in `REPO_POLICIES.md`. Adds `-cover` to the first run per the policy's Go example; keeps `-race` (from https://git.eeqj.de/sneak/pixa/pulls/55) and the existing nix-shell CGO fallback. Closes https://git.eeqj.de/sneak/pixa/issues/59. ## Why Previously `script/test` always passed `-v`, so every green build dumped full per-test output. The policy wants clean per-package summaries on success and full detail only on failure. ## What a reader might trip over - The whole compound command (first run `||` rerun-with-`-v`; `exit 1`) is passed as one string to `run_with_cgo_deps`, so it runs unchanged under both the host `sh -c` path and the `nix-shell --run` fallback. - `-cover` is on the first run only; the `-v` rerun matches the policy example and exists solely for diagnostics. ## Disclosures - Scope: `script/test` only, no Go changed. TDD's failing-test-first step does not apply to the test-runner wrapper itself and a shell-test harness would exceed the issue's scope; verified by running the gate instead. - The model `script/test` in the `prompts` repo has since migrated to a Docker-`--target test` approach with no shell pattern; per the DoD I kept the shell pattern with the nix-shell fallback (Docker-only testing is https://git.eeqj.de/sneak/pixa/issues/101 / https://git.eeqj.de/sneak/pixa/issues/104 territory). - Gate: `docker build .` green — pinned `make lint`/`make fmt-check` and `RUN make test` (my changed script) all ran, not cached; success path showed clean coverage summaries and no `-v` rerun. Model: opus-4-8
clawbot added the needs-review label 2026-09-21 09:49:57 +02:00
clawbot self-assigned this 2026-09-21 09:49:57 +02:00
Author
Collaborator

FAIL.

Finding — Model: line format. The landing commit message and the PR body at #115 both end with model: claude-opus-4-8. This retains the claude- product-name prefix that repository policy requires stripped from the identifier, and uses a lowercase model: rather than the specified Model:. Acceptable: a single trailing line Model: opus-4-8 on both the commit message and the PR body.

The change implementing #59 otherwise satisfies the definition of done and rebases cleanly onto next.

Model: opus-4-8

FAIL. Finding — `Model:` line format. The landing commit message and the PR body at https://git.eeqj.de/sneak/pixa/pulls/115 both end with `model: claude-opus-4-8`. This retains the `claude-` product-name prefix that repository policy requires stripped from the identifier, and uses a lowercase `model:` rather than the specified `Model:`. Acceptable: a single trailing line `Model: opus-4-8` on both the commit message and the PR body. The change implementing https://git.eeqj.de/sneak/pixa/issues/59 otherwise satisfies the definition of done and rebases cleanly onto `next`. Model: opus-4-8
clawbot added needs-rework and removed needs-review labels 2026-09-21 18:44:24 +02:00
clawbot added 1 commit 2026-09-21 19:21:33 +02:00
Run the suite without -v first so a passing build shows only per-package
summaries; on failure rerun with -v for full diagnostics and exit
non-zero, matching the conditional-verbose-rerun pattern in
REPO_POLICIES.md. Add -cover to the first run per the policy's Go
example; keep -race (added in #55) and the existing nix-shell CGO
fallback for hosts without pkg-config/vips.

Model: opus-4-8
clawbot force-pushed issue-59-test-conditional-verbose from 497f5bf6ec to 2dbc76fa36 2026-09-21 19:21:33 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-09-21 19:22:07 +02:00
Author
Collaborator

Reworked the one finding: the commit message and the PR body now end with the single line Model: opus-4-8 (previously a lowercase, untruncated identifier). No file content changed — rebased onto current next, which already carries the golangci-lint update from #54, so that commit dropped out and the branch is now just the single script/test change. make check passes locally.

Model: opus-4-8

Reworked the one finding: the commit message and the PR body now end with the single line `Model: opus-4-8` (previously a lowercase, untruncated identifier). No file content changed — rebased onto current `next`, which already carries the golangci-lint update from https://git.eeqj.de/sneak/pixa/pulls/54, so that commit dropped out and the branch is now just the single `script/test` change. `make check` passes locally. Model: opus-4-8
Author
Collaborator

PASS — rebased onto current next (single clean commit), script/test matches the REPO_POLICIES conditional-verbose-rerun pattern (non-verbose -race -cover first, -v rerun then non-zero exit on failure, 30s timeout), the failure path was confirmed to trigger the verbose rerun and a non-zero exit while the success path stays quiet, make check is green in the Docker gate, the landing title carries (closes #59), and the commit message and PR body each end with Model: opus-4-8.

Model: opus-4-8

PASS — rebased onto current `next` (single clean commit), `script/test` matches the REPO_POLICIES conditional-verbose-rerun pattern (non-verbose `-race -cover` first, `-v` rerun then non-zero exit on failure, 30s timeout), the failure path was confirmed to trigger the verbose rerun and a non-zero exit while the success path stays quiet, `make check` is green in the Docker gate, the landing title carries `(closes #59)`, and the commit message and PR body each end with `Model: opus-4-8`. Model: opus-4-8
clawbot merged commit b95ef1eb69 into next 2026-09-21 19:43:22 +02:00
clawbot deleted branch issue-59-test-conditional-verbose 2026-09-21 19:43:22 +02:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#115