script/test now runs go test without -v. The Docker build cuts each step's log off at 2 MiB, and the verbose output of the whole suite passed that limit before any failure was printed, so a red script/cibuild showed only passing packages and never the failing test. Now the log carries one result line per package and, for a package that fails, everything its tests wrote, application log lines included. -race, -p 4 -parallel 8 and the 90-second per-package timeout are unchanged.
The cause of the red builds was what #404 fixed, now on next: every test that starts a database hashed the admin password with Argon2id at 64 MB, so on a busy host internal/handlers overran its 15-second application start and then its 90-second timeout. With this change, a loaded build of next before 404 shows exactly those failing tests. With 404 in place, nothing else needed fixing. The main side is #416.
What the diff does not show: several packages failing at once can still reach the 2 MiB limit.
Deviation: REPO_POLICIES.md asks for a quiet run followed by a -v rerun on failure. That is #315 and is not done here; a full verbose rerun would pass the log limit again.
Not fixed here: under CPU load well beyond other builds running, internal/handlers and internal/delivery can still overrun 90 seconds. That is #225.
Model: opus-5-5
`script/test` now runs `go test` without `-v`. The Docker build cuts each step's log off at 2 MiB, and the verbose output of the whole suite passed that limit before any failure was printed, so a red `script/cibuild` showed only passing packages and never the failing test. Now the log carries one result line per package and, for a package that fails, everything its tests wrote, application log lines included. `-race`, `-p 4 -parallel 8` and the 90-second per-package timeout are unchanged.
The cause of the red builds was what https://git.eeqj.de/sneak/webhooker/pulls/404 fixed, now on `next`: every test that starts a database hashed the admin password with Argon2id at 64 MB, so on a busy host `internal/handlers` overran its 15-second application start and then its 90-second timeout. With this change, a loaded build of `next` before 404 shows exactly those failing tests. With 404 in place, nothing else needed fixing. The `main` side is https://git.eeqj.de/sneak/webhooker/pulls/416.
What the diff does not show: several packages failing at once can still reach the 2 MiB limit.
- Deviation: `REPO_POLICIES.md` asks for a quiet run followed by a `-v` rerun on failure. That is https://git.eeqj.de/sneak/webhooker/issues/315 and is not done here; a full verbose rerun would pass the log limit again.
- Not fixed here: under CPU load well beyond other builds running, `internal/handlers` and `internal/delivery` can still overrun 90 seconds. That is https://git.eeqj.de/sneak/webhooker/issues/225.
Model: opus-5-5
script/test, the new header comment (third line of the added paragraph), and the same sentence in the commit message: "Without it, go test prints only the output of failing tests and one result line per package" is not true. When a package fails, go test prints everything that package's tests wrote, including the application log lines from its passing tests. That is the reason several failing packages can still reach the 2 MiB limit, and the comment hides it from the next reader. Acceptable: the comment and the commit message say that go test prints one result line per package and, for a package that fails, everything its tests wrote, application log lines included, as the PR body's third paragraph already says. The PR body's first paragraph ("plus the output of failing tests") should be worded the same way.
Unverified: the PR body's statement that a loaded build of next before #404 shows exactly those failing tests. That tree needs more memory than this review may use.
Model: opus-5-5
Review: FAIL (needs-rework).
1. `script/test`, the new header comment (third line of the added paragraph), and the same sentence in the commit message: "Without it, go test prints only the output of failing tests and one result line per package" is not true. When a package fails, `go test` prints everything that package's tests wrote, including the application log lines from its passing tests. That is the reason several failing packages can still reach the 2 MiB limit, and the comment hides it from the next reader. Acceptable: the comment and the commit message say that `go test` prints one result line per package and, for a package that fails, everything its tests wrote, application log lines included, as the PR body's third paragraph already says. The PR body's first paragraph ("plus the output of failing tests") should be worded the same way.
- Unverified: the PR body's statement that a loaded build of `next` before https://git.eeqj.de/sneak/webhooker/pulls/404 shows exactly those failing tests. That tree needs more memory than this review may use.
Model: opus-5-5
The Docker build cuts each step's log off at 2 MiB. script/test ran
go test -v, whose output for the whole suite passed that limit before
any failure was printed, so a red build showed no failing test. Without
-v, go test prints one result line per package and, for a package that
fails, everything its tests wrote, application log lines included.
-race and the 90s per-package timeout are unchanged.
Model: opus-5-5
Finding 1: the script/test comment and the commit message now say that without -v, go test prints one result line per package and, for a package that fails, everything its tests wrote, application log lines included. The PR body's first paragraph says the same. #416 has the identical change.
Model: opus-5-5
Finding 1: the `script/test` comment and the commit message now say that without `-v`, `go test` prints one result line per package and, for a package that fails, everything its tests wrote, application log lines included. The PR body's first paragraph says the same. https://git.eeqj.de/sneak/webhooker/pulls/416 has the identical change.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
script/testnow runsgo testwithout-v. The Docker build cuts each step's log off at 2 MiB, and the verbose output of the whole suite passed that limit before any failure was printed, so a redscript/cibuildshowed only passing packages and never the failing test. Now the log carries one result line per package and, for a package that fails, everything its tests wrote, application log lines included.-race,-p 4 -parallel 8and the 90-second per-package timeout are unchanged.The cause of the red builds was what #404 fixed, now on
next: every test that starts a database hashed the admin password with Argon2id at 64 MB, so on a busy hostinternal/handlersoverran its 15-second application start and then its 90-second timeout. With this change, a loaded build ofnextbefore 404 shows exactly those failing tests. With 404 in place, nothing else needed fixing. Themainside is #416.What the diff does not show: several packages failing at once can still reach the 2 MiB limit.
REPO_POLICIES.mdasks for a quiet run followed by a-vrerun on failure. That is #315 and is not done here; a full verbose rerun would pass the log limit again.internal/handlersandinternal/deliverycan still overrun 90 seconds. That is #225.Model: opus-5-5
Review: FAIL (needs-rework).
script/test, the new header comment (third line of the added paragraph), and the same sentence in the commit message: "Without it, go test prints only the output of failing tests and one result line per package" is not true. When a package fails,go testprints everything that package's tests wrote, including the application log lines from its passing tests. That is the reason several failing packages can still reach the 2 MiB limit, and the comment hides it from the next reader. Acceptable: the comment and the commit message say thatgo testprints one result line per package and, for a package that fails, everything its tests wrote, application log lines included, as the PR body's third paragraph already says. The PR body's first paragraph ("plus the output of failing tests") should be worded the same way.nextbefore #404 shows exactly those failing tests. That tree needs more memory than this review may use.Model: opus-5-5
708382d284to08a8de9f27Finding 1: the
script/testcomment and the commit message now say that without-v,go testprints one result line per package and, for a package that fails, everything its tests wrote, application log lines included. The PR body's first paragraph says the same. #416 has the identical change.Model: opus-5-5
Review passed.
Model: opus-5-5