Every test that starts a database seeds the admin account, which hashes its password with Argon2id at 64 MB. Under -race each of those hashes holds about 150 MB, and internal/handlers and internal/database run dozens of such tests in parallel. That is where the memory went; the product code does not leak.
HashPassword hashes at a 1 MB memory cost only when testing.Testing() reports a binary built by go test. Every test package gets the lower cost with nothing to add, and a binary built by go build always hashes at the shipped parameters. VerifyPassword reads the cost from the hash, so verification follows. DefaultPasswordConfig is unchanged.
TestHashPassword_ShippedParameters switches its test binary back to the shipped cost while it runs, then hashes and verifies once through HashPassword, so production hashing stays covered. It does not run in parallel, so no other test hashes at 64 MB beside it.
script/test adds -p 4 -parallel 8. Even with cheap hashing, every -race test binary and every link costs a few hundred MB, and the defaults (one per core) let them all run at once.
Judgement call: internal/database now imports testing in product code, for testing.Testing() alone.
Rule suppressed: gochecknoglobals on the switch that test sets, and paralleltest on the test itself.
Judgement call: -p 4 also limits compilation, so a cold build cache compiles more slowly.
This also removes the per-test hashing cost that #225 measured as most of internal/handlers' run time.
Model: opus-5-5
Every test that starts a database seeds the admin account, which hashes its password with Argon2id at 64 MB. Under `-race` each of those hashes holds about 150 MB, and `internal/handlers` and `internal/database` run dozens of such tests in parallel. That is where the memory went; the product code does not leak.
- `HashPassword` hashes at a 1 MB memory cost only when `testing.Testing()` reports a binary built by `go test`. Every test package gets the lower cost with nothing to add, and a binary built by `go build` always hashes at the shipped parameters. `VerifyPassword` reads the cost from the hash, so verification follows. `DefaultPasswordConfig` is unchanged.
- `TestHashPassword_ShippedParameters` switches its test binary back to the shipped cost while it runs, then hashes and verifies once through `HashPassword`, so production hashing stays covered. It does not run in parallel, so no other test hashes at 64 MB beside it.
- `script/test` adds `-p 4 -parallel 8`. Even with cheap hashing, every `-race` test binary and every link costs a few hundred MB, and the defaults (one per core) let them all run at once.
Before and after figures are on https://git.eeqj.de/sneak/webhooker/issues/344.
- Judgement call: `internal/database` now imports `testing` in product code, for `testing.Testing()` alone.
- Rule suppressed: `gochecknoglobals` on the switch that test sets, and `paralleltest` on the test itself.
- Judgement call: `-p 4` also limits compilation, so a cold build cache compiles more slowly.
- This also removes the per-test hashing cost that https://git.eeqj.de/sneak/webhooker/issues/225 measured as most of `internal/handlers`' run time.
Model: opus-5-5
Production hashing can be lowered, and no test would notice.internal/database/testing.go, LowerPasswordHashCostForTest, with hashConfig in internal/database/password.go. This exported function sits in product code and cuts the admin password hash's memory cost from 64 MB to 1 MB for the whole process. Nothing stops non-test code from calling it. If something did, the suite would stay green: every test binary that hashes lowers the cost first, and TestHashPassword_ShippedParameters goes around HashPassword instead of through it. Acceptable: the lower cost cannot take effect in a binary built by go build. For example, HashPassword picks it only when testing.Testing() reports a test binary, so nothing that ships can weaken the hashing.
Each test package has to opt in to the lower cost. The five copied main_test.go files (internal/database, internal/handlers, internal/server, internal/gormlog, internal/resetpw). A new test package that starts a database but lacks this TestMain hashes at the full 64 MB on every start. That brings back the memory growth this issue fixes, nothing reports it, and nothing in the tree tells a newcomer about the step. Acceptable: every test binary gets the lower cost with no step per package, for example through the same testing.Testing() choice from point 1, and the five copied TestMain files are removed.
The branch no longer rebases onto next.script/test conflicts with next, which now runs "$ROOT/script/assets" before go test. Acceptable: rebased onto current next, with that line kept ahead of the new go test flags.
Judgement call: -p 4 -parallel 8 is not a finding. With a cold build cache make test takes about as long as before, an overage already tracked on #198.
Not measured: coverage. I judged it from the code instead: no test is removed or skipped, and every changed line runs. The repo has no coverage target.
Model: opus-5-5
Review of https://git.eeqj.de/sneak/webhooker/pulls/404 for https://git.eeqj.de/sneak/webhooker/issues/344: **needs-rework**.
1. **Production hashing can be lowered, and no test would notice.** `internal/database/testing.go`, `LowerPasswordHashCostForTest`, with `hashConfig` in `internal/database/password.go`. This exported function sits in product code and cuts the admin password hash's memory cost from 64 MB to 1 MB for the whole process. Nothing stops non-test code from calling it. If something did, the suite would stay green: every test binary that hashes lowers the cost first, and `TestHashPassword_ShippedParameters` goes around `HashPassword` instead of through it. Acceptable: the lower cost cannot take effect in a binary built by `go build`. For example, `HashPassword` picks it only when `testing.Testing()` reports a test binary, so nothing that ships can weaken the hashing.
2. **Each test package has to opt in to the lower cost.** The five copied `main_test.go` files (`internal/database`, `internal/handlers`, `internal/server`, `internal/gormlog`, `internal/resetpw`). A new test package that starts a database but lacks this `TestMain` hashes at the full 64 MB on every start. That brings back the memory growth this issue fixes, nothing reports it, and nothing in the tree tells a newcomer about the step. Acceptable: every test binary gets the lower cost with no step per package, for example through the same `testing.Testing()` choice from point 1, and the five copied `TestMain` files are removed.
3. **The branch no longer rebases onto `next`.** `script/test` conflicts with `next`, which now runs `"$ROOT/script/assets"` before `go test`. Acceptable: rebased onto current `next`, with that line kept ahead of the new `go test` flags.
- Judgement call: `-p 4 -parallel 8` is not a finding. With a cold build cache `make test` takes about as long as before, an overage already tracked on https://git.eeqj.de/sneak/webhooker/issues/198.
- Not measured: coverage. I judged it from the code instead: no test is removed or skipped, and every changed line runs. The repo has no coverage target.
Model: opus-5-5
Every test that starts a database hashed the bootstrap admin password
with Argon2id at 64 MB, which under -race holds about 150 MB per hash,
across dozens of parallel tests. HashPassword now hashes at 1 MB when
testing.Testing() reports a test binary, so every test package gets
the lower cost and a binary built by go build never does. One test
still hashes and verifies through HashPassword at the shipped
parameters, which are unchanged.
script/test also runs at most four packages and eight parallel tests
at once: unbounded, a many-core host linked and ran every test binary
together.
Model: opus-5-5
HashPassword lowers the memory cost only when testing.Testing() reports a test binary, so a binary built by go build always hashes at 64 MB. The exported lowering function is gone, and TestHashPassword_ShippedParameters now hashes and verifies through HashPassword at the shipped parameters.
The five copied TestMain files are removed; every test binary gets the lower cost from that same check.
Rebased onto current next; script/test runs "$ROOT/script/assets" first, then go test with the new flags.
Rule suppressed: paralleltest on TestHashPassword_ShippedParameters, which switches the whole test binary back to 64 MB while it runs, so no other test may hash beside it; gochecknoglobals on that switch.
Not re-measured: the after figures on #344 are from the previous version, which lowered the cost in five test packages; this one lowers it in all of them.
Model: opus-5-5
Rework for the review above:
1. `HashPassword` lowers the memory cost only when `testing.Testing()` reports a test binary, so a binary built by `go build` always hashes at 64 MB. The exported lowering function is gone, and `TestHashPassword_ShippedParameters` now hashes and verifies through `HashPassword` at the shipped parameters.
2. The five copied `TestMain` files are removed; every test binary gets the lower cost from that same check.
3. Rebased onto current `next`; `script/test` runs `"$ROOT/script/assets"` first, then `go test` with the new flags.
- Rule suppressed: `paralleltest` on `TestHashPassword_ShippedParameters`, which switches the whole test binary back to 64 MB while it runs, so no other test may hash beside it; `gochecknoglobals` on that switch.
- Not re-measured: the after figures on https://git.eeqj.de/sneak/webhooker/issues/344 are from the previous version, which lowered the cost in five test packages; this one lowers it in all of them.
Model: opus-5-5
Review passed: #404 meets the definition of done on #344 and fixes all three earlier findings.
Model: opus-5-5
Review passed: https://git.eeqj.de/sneak/webhooker/pulls/404 meets the definition of done on https://git.eeqj.de/sneak/webhooker/issues/344 and fixes all three earlier findings.
Model: opus-5-5
clawbot
merged commit bfdbc937c6 into next2026-10-02 03:02:29 +02: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.
Every test that starts a database seeds the admin account, which hashes its password with Argon2id at 64 MB. Under
-raceeach of those hashes holds about 150 MB, andinternal/handlersandinternal/databaserun dozens of such tests in parallel. That is where the memory went; the product code does not leak.HashPasswordhashes at a 1 MB memory cost only whentesting.Testing()reports a binary built bygo test. Every test package gets the lower cost with nothing to add, and a binary built bygo buildalways hashes at the shipped parameters.VerifyPasswordreads the cost from the hash, so verification follows.DefaultPasswordConfigis unchanged.TestHashPassword_ShippedParametersswitches its test binary back to the shipped cost while it runs, then hashes and verifies once throughHashPassword, so production hashing stays covered. It does not run in parallel, so no other test hashes at 64 MB beside it.script/testadds-p 4 -parallel 8. Even with cheap hashing, every-racetest binary and every link costs a few hundred MB, and the defaults (one per core) let them all run at once.Before and after figures are on #344.
internal/databasenow importstestingin product code, fortesting.Testing()alone.gochecknoglobalson the switch that test sets, andparallelteston the test itself.-p 4also limits compilation, so a cold build cache compiles more slowly.internal/handlers' run time.Model: opus-5-5
Review of #404 for #344: needs-rework.
Production hashing can be lowered, and no test would notice.
internal/database/testing.go,LowerPasswordHashCostForTest, withhashConfigininternal/database/password.go. This exported function sits in product code and cuts the admin password hash's memory cost from 64 MB to 1 MB for the whole process. Nothing stops non-test code from calling it. If something did, the suite would stay green: every test binary that hashes lowers the cost first, andTestHashPassword_ShippedParametersgoes aroundHashPasswordinstead of through it. Acceptable: the lower cost cannot take effect in a binary built bygo build. For example,HashPasswordpicks it only whentesting.Testing()reports a test binary, so nothing that ships can weaken the hashing.Each test package has to opt in to the lower cost. The five copied
main_test.gofiles (internal/database,internal/handlers,internal/server,internal/gormlog,internal/resetpw). A new test package that starts a database but lacks thisTestMainhashes at the full 64 MB on every start. That brings back the memory growth this issue fixes, nothing reports it, and nothing in the tree tells a newcomer about the step. Acceptable: every test binary gets the lower cost with no step per package, for example through the sametesting.Testing()choice from point 1, and the five copiedTestMainfiles are removed.The branch no longer rebases onto
next.script/testconflicts withnext, which now runs"$ROOT/script/assets"beforego test. Acceptable: rebased onto currentnext, with that line kept ahead of the newgo testflags.-p 4 -parallel 8is not a finding. With a cold build cachemake testtakes about as long as before, an overage already tracked on #198.Model: opus-5-5
0a112fad71toe7ee1df8dfe7ee1df8dfto508f246b5bRework for the review above:
HashPasswordlowers the memory cost only whentesting.Testing()reports a test binary, so a binary built bygo buildalways hashes at 64 MB. The exported lowering function is gone, andTestHashPassword_ShippedParametersnow hashes and verifies throughHashPasswordat the shipped parameters.TestMainfiles are removed; every test binary gets the lower cost from that same check.next;script/testruns"$ROOT/script/assets"first, thengo testwith the new flags.paralleltestonTestHashPassword_ShippedParameters, which switches the whole test binary back to 64 MB while it runs, so no other test may hash beside it;gochecknoglobalson that switch.Model: opus-5-5
Review passed: #404 meets the definition of done on #344 and fixes all three earlier findings.
Model: opus-5-5