Cause: the internal/handlers tests ran one at a time, because config.New writes the global viper instance. Each test spends most of its time in SQLite under the race detector, so in a row they took longer than the 20-second target.
What changed, in the internal/handlers test files only:
Every test and subtest calls t.Parallel(), as the internal/ircserver and internal/db tests already do. The file-level //nolint:paralleltest lines are gone, so the linter asks it of new tests too.
newTestServerWith holds a mutex, viperMu, while it builds and starts a server, so servers start one at a time. config.New writes viper, and the server reads it as it starts, in internal/server/routes.go.
What the diff does not show:
Each test still has its own server and database, so no test sees another's data.
The Dockerfile and script/ are unchanged: -race, -p 4 and the test timeout stay as they are.
Disclosures:
Rule suppressed: gochecknoglobals on viperMu, which all tests must share; internal/db/export_test.go does the same for its test counter.
Judgement call: -p 4 limits how many packages test at once, not how many tests in one package run together. This package now does what most packages here already do.
Judgement call: tests that depend on timing keep their limits and now share the CPU with other tests. These are the long-poll deadlines and the login rate limit, which refills once a second.
Model: opus-5-5
Cause: the `internal/handlers` tests ran one at a time, because `config.New` writes the global viper instance. Each test spends most of its time in SQLite under the race detector, so in a row they took longer than the 20-second target.
What changed, in the `internal/handlers` test files only:
- Every test and subtest calls `t.Parallel()`, as the `internal/ircserver` and `internal/db` tests already do. The file-level `//nolint:paralleltest` lines are gone, so the linter asks it of new tests too.
- `newTestServerWith` holds a mutex, `viperMu`, while it builds and starts a server, so servers start one at a time. `config.New` writes viper, and the server reads it as it starts, in `internal/server/routes.go`.
What the diff does not show:
- Each test still has its own server and database, so no test sees another's data.
- The `Dockerfile` and `script/` are unchanged: `-race`, `-p 4` and the test timeout stay as they are.
Disclosures:
- Rule suppressed: `gochecknoglobals` on `viperMu`, which all tests must share; `internal/db/export_test.go` does the same for its test counter.
- Judgement call: `-p 4` limits how many packages test at once, not how many tests in one package run together. This package now does what most packages here already do.
- Judgement call: tests that depend on timing keep their limits and now share the CPU with other tests. These are the long-poll deadlines and the login rate limit, which refills once a second.
Model: opus-5-5
The handlers tests ran one at a time because config.New writes the
global viper instance. Each test spends most of its time in SQLite
under the race detector, so run in a row they took longer than the
20-second target.
Every test and subtest now calls t.Parallel(), as the ircserver and
db tests already do. A mutex in newTestServerWith makes tests start
their servers one at a time, since config.New writes viper and the
server reads it as it starts. No test is removed or otherwise
changed.
Model: opus-5-5
PASS: the internal/handlers tests now run in parallel, one lock covers every use of the global viper instance, each test still has its own server and database, and the package is well under the 20-second target of #194.
Judgement call: making the tests run in parallel, by removing the global-viper reason they had to run one at a time, counts as removing the cost at its cause.
Judgement call: TestIntegrationKill in internal/ircserver, which this PR does not touch, once timed out waiting for the victim's QUIT and did not fail again with or without this change, so I did not count it against this PR.
Unverified: whether the added lines were typed by hand; nothing in the change or its description points to a scripted edit.
Model: opus-5-5
PASS: the `internal/handlers` tests now run in parallel, one lock covers every use of the global viper instance, each test still has its own server and database, and the package is well under the 20-second target of https://git.eeqj.de/sneak/neoirc/issues/194.
- Judgement call: making the tests run in parallel, by removing the global-viper reason they had to run one at a time, counts as removing the cost at its cause.
- Judgement call: `TestIntegrationKill` in `internal/ircserver`, which this PR does not touch, once timed out waiting for the victim's QUIT and did not fail again with or without this change, so I did not count it against this PR.
- Unverified: whether the added lines were typed by hand; nothing in the change or its description points to a scripted edit.
Model: opus-5-5
clawbot
merged commit a7f9bebe6a into next2026-10-27 08:28:29 +01:00
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.
Cause: the
internal/handlerstests ran one at a time, becauseconfig.Newwrites the global viper instance. Each test spends most of its time in SQLite under the race detector, so in a row they took longer than the 20-second target.What changed, in the
internal/handlerstest files only:t.Parallel(), as theinternal/ircserverandinternal/dbtests already do. The file-level//nolint:paralleltestlines are gone, so the linter asks it of new tests too.newTestServerWithholds a mutex,viperMu, while it builds and starts a server, so servers start one at a time.config.Newwrites viper, and the server reads it as it starts, ininternal/server/routes.go.What the diff does not show:
Dockerfileandscript/are unchanged:-race,-p 4and the test timeout stay as they are.Disclosures:
gochecknoglobalsonviperMu, which all tests must share;internal/db/export_test.godoes the same for its test counter.-p 4limits how many packages test at once, not how many tests in one package run together. This package now does what most packages here already do.Model: opus-5-5
clawbot referenced this pull request2026-10-27 07:58:41 +01:00
PASS: the
internal/handlerstests now run in parallel, one lock covers every use of the global viper instance, each test still has its own server and database, and the package is well under the 20-second target of #194.TestIntegrationKillininternal/ircserver, which this PR does not touch, once timed out waiting for the victim's QUIT and did not fail again with or without this change, so I did not count it against this PR.Model: opus-5-5