script/test only. Its last two operative lines were:
go vet ./...
go test ./... || go test -v ./...
The verbose rerun was the last command, so its exit status became the script's. A test that failed on the first run and passed on the retry produced exit 0. The Dockerfile runs make test and CI runs script/cibuild which runs docker build, so that hole made flaky failures invisible across the whole repo.
Replaced with the pattern from REPO_POLICIES.md, verbatim:
go test -timeout 30s -race -cover ./... ||\
{echo"--- Rerunning with -v for details ---";\
go test -timeout 30s -race -v ./...;exit 1;}
The trailing exit 1 is the point: the first run already proved the tests are broken, so the rerun is diagnostic only and can never turn a failed run green. go vet ./... still runs first, unchanged. The script stays POSIX sh with set -eu, the existing CGO_ENABLED=1 export, and the repo-root cd idiom. No other script is touched and .golangci.yml is untouched. TODO.md gets one additive entry in the same commit.
Read this before merging: CI is red, deliberately
Enabling -race exposed a real conflict with the policy timing budget. make check does not pass on this branch, and nothing here has been bent to make it pass.
Measured in the CI-equivalent container — script/cibuild, i.e. docker build --ulimit memlock=-1:-1 ., which runs make test in the builder stage. The host is unusable for this measurement: its memlock hard limit is 8192 KB, so the memguard 10MB case aborts long before the suite can be timed.
make test layer, wall clock
slowest package
main today, no -race
18.7s (~9s of it go vet + compile)
internal/cli 9.6s
this branch, -race on
fails
internal/cli killed at the 30s timeout
panic: test timed out after 30s
running tests:
TestImportSecretVariousSizes (5s)
TestImportSecretVariousSizes/99MB_file (3s)
FAIL git.eeqj.de/sneak/secret/internal/cli 30.809s
ok git.eeqj.de/sneak/secret/internal/secret 12.698s coverage: 43.1%
ok git.eeqj.de/sneak/secret/internal/vault 15.076s coverage: 62.8%
ok git.eeqj.de/sneak/secret/pkg/agehd 3.166s coverage: 86.3%
ok git.eeqj.de/sneak/secret/pkg/bip85 1.329s coverage: 87.2%
No data race was found. Every package that ran to completion is clean under the detector. The failure is pure slowness, not a hang and not a deadlock — the stack at the timeout sits in memguard/core.Wipe under cli.(*Instance).ImportSecret (internal/cli/secrets.go:579), making ordinary forward progress. TestAddSecretVariousSizes alone eats 24.19s of the 30s budget, with 99MB at 10.25s and 100MB minus 1 at 10.19s; TestImportSecretVariousSizes is the same size ladder over the import path, which is where the package runs out of clock. internal/cli needs roughly 55-60s under -race against 9.6s without.
Every available lever is one the issue explicitly forbids: raising the timeout papers over it (and the 20s budget would still be missed by 3x), t.Skip is a skip, and shrinking the sizes deletes boundary coverage for the real 100MB cap at internal/cli/secrets.go:245 and :515. Making the large-secret path genuinely faster is out of scope here. No small in-scope change closes a 6x gap, so this stops and reports rather than forcing one.
#52 has the full measurements and four options for a decision. That decision is yours; my read is the build-tag split, since it is the only option that gives up nothing. This PR should land together with whatever #52 concludes, not before it.
Also worth stating plainly: the clean -race result is not evidence that #34's missing file locking is harmless. The suite has no test that touches the vault from two goroutines at once, so there was nothing for the detector to catch. #34 stands entirely on its own.
Verification
make lint — clean, 0 issues.
make fmt-check — clean.
make test — fails as described above; the new pattern was confirmed working end to end, printing --- Rerunning with -v for details --- and exiting non-zero after the rerun.
script/cibuild — run on both main and this branch for the before/after numbers in the table.
Closes #32.
## What changed
`script/test` only. Its last two operative lines were:
```sh
go vet ./...
go test ./... || go test -v ./...
```
The verbose rerun was the last command, so its exit status became the script's. A test that failed on the first run and passed on the retry produced exit 0. The `Dockerfile` runs `make test` and CI runs `script/cibuild` which runs `docker build`, so that hole made flaky failures invisible across the whole repo.
Replaced with the pattern from `REPO_POLICIES.md`, verbatim:
```sh
go test -timeout 30s -race -cover ./... || \
{ echo "--- Rerunning with -v for details ---"; \
go test -timeout 30s -race -v ./...; exit 1; }
```
The trailing `exit 1` is the point: the first run already proved the tests are broken, so the rerun is diagnostic only and can never turn a failed run green. `go vet ./...` still runs first, unchanged. The script stays POSIX sh with `set -eu`, the existing `CGO_ENABLED=1` export, and the repo-root `cd` idiom. No other script is touched and `.golangci.yml` is untouched. `TODO.md` gets one additive entry in the same commit.
## Read this before merging: CI is red, deliberately
Enabling `-race` exposed a real conflict with the policy timing budget. **`make check` does not pass on this branch**, and nothing here has been bent to make it pass.
Measured in the CI-equivalent container — `script/cibuild`, i.e. `docker build --ulimit memlock=-1:-1 .`, which runs `make test` in the builder stage. The host is unusable for this measurement: its `memlock` hard limit is 8192 KB, so the memguard 10MB case aborts long before the suite can be timed.
| | `make test` layer, wall clock | slowest package |
|---|---|---|
| `main` today, no `-race` | **18.7s** (~9s of it `go vet` + compile) | `internal/cli` 9.6s |
| this branch, `-race` on | **fails** | `internal/cli` killed at the 30s timeout |
```
panic: test timed out after 30s
running tests:
TestImportSecretVariousSizes (5s)
TestImportSecretVariousSizes/99MB_file (3s)
FAIL git.eeqj.de/sneak/secret/internal/cli 30.809s
ok git.eeqj.de/sneak/secret/internal/secret 12.698s coverage: 43.1%
ok git.eeqj.de/sneak/secret/internal/vault 15.076s coverage: 62.8%
ok git.eeqj.de/sneak/secret/pkg/agehd 3.166s coverage: 86.3%
ok git.eeqj.de/sneak/secret/pkg/bip85 1.329s coverage: 87.2%
```
**No data race was found.** Every package that ran to completion is clean under the detector. The failure is pure slowness, not a hang and not a deadlock — the stack at the timeout sits in `memguard/core.Wipe` under `cli.(*Instance).ImportSecret` (`internal/cli/secrets.go:579`), making ordinary forward progress. `TestAddSecretVariousSizes` alone eats 24.19s of the 30s budget, with `99MB` at 10.25s and `100MB minus 1` at 10.19s; `TestImportSecretVariousSizes` is the same size ladder over the import path, which is where the package runs out of clock. `internal/cli` needs roughly 55-60s under `-race` against 9.6s without.
Every available lever is one the issue explicitly forbids: raising the timeout papers over it (and the 20s budget would still be missed by 3x), `t.Skip` is a skip, and shrinking the sizes deletes boundary coverage for the real 100MB cap at `internal/cli/secrets.go:245` and `:515`. Making the large-secret path genuinely faster is out of scope here. No small in-scope change closes a 6x gap, so this stops and reports rather than forcing one.
**#52 has the full measurements and four options for a decision.** That decision is yours; my read is the build-tag split, since it is the only option that gives up nothing. This PR should land together with whatever #52 concludes, not before it.
Also worth stating plainly: the clean `-race` result is not evidence that #34's missing file locking is harmless. The suite has no test that touches the vault from two goroutines at once, so there was nothing for the detector to catch. #34 stands entirely on its own.
## Verification
- `make lint` — clean, `0 issues.`
- `make fmt-check` — clean.
- `make test` — fails as described above; the new pattern was confirmed working end to end, printing `--- Rerunning with -v for details ---` and exiting non-zero after the rerun.
- `script/cibuild` — run on both `main` and this branch for the before/after numbers in the table.
script/test ended with `go test ./... || go test -v ./...`, so the
verbose rerun's exit status became the script's. A test that failed
once and passed on the retry produced exit 0, and since the Dockerfile
runs `make test` and CI runs script/cibuild, flaky failures were
invisible repo-wide.
Replace that with the pattern from REPO_POLICIES.md: run
`go test -timeout 30s -race -cover ./...`, and on failure rerun
verbosely and then exit 1, so the rerun is diagnostic only and can
never turn a failed run green. `go vet ./...` still runs first; the
script stays POSIX sh with set -eu, the CGO_ENABLED=1 export, and the
repo-root cd idiom.
Enabling -race surfaced a genuine timing conflict rather than a data
race: no package that ran to completion reported one, but internal/cli
now exceeds the 30s timeout because the 99MB/100MB boundary cases in
secrets_size_test.go are about 10x slower under race instrumentation.
Measured in the CI-equivalent container (script/cibuild, with the
memlock ulimit): 18.7s for the make test layer without -race, versus
internal/cli alone needing roughly 55-60s with it. Neither the flags
nor the tests were weakened to hide this; the conflict is filed as #52
for a decision.
clawbot
removed their assignment 2026-09-03 15:41:48 +02:00
sneak
was assigned by clawbot2026-09-03 15:41:48 +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.
Closes #32.
What changed
script/testonly. Its last two operative lines were:The verbose rerun was the last command, so its exit status became the script's. A test that failed on the first run and passed on the retry produced exit 0. The
Dockerfilerunsmake testand CI runsscript/cibuildwhich runsdocker build, so that hole made flaky failures invisible across the whole repo.Replaced with the pattern from
REPO_POLICIES.md, verbatim:The trailing
exit 1is the point: the first run already proved the tests are broken, so the rerun is diagnostic only and can never turn a failed run green.go vet ./...still runs first, unchanged. The script stays POSIX sh withset -eu, the existingCGO_ENABLED=1export, and the repo-rootcdidiom. No other script is touched and.golangci.ymlis untouched.TODO.mdgets one additive entry in the same commit.Read this before merging: CI is red, deliberately
Enabling
-raceexposed a real conflict with the policy timing budget.make checkdoes not pass on this branch, and nothing here has been bent to make it pass.Measured in the CI-equivalent container —
script/cibuild, i.e.docker build --ulimit memlock=-1:-1 ., which runsmake testin the builder stage. The host is unusable for this measurement: itsmemlockhard limit is 8192 KB, so the memguard 10MB case aborts long before the suite can be timed.make testlayer, wall clockmaintoday, no-racego vet+ compile)internal/cli9.6s-raceoninternal/clikilled at the 30s timeoutNo data race was found. Every package that ran to completion is clean under the detector. The failure is pure slowness, not a hang and not a deadlock — the stack at the timeout sits in
memguard/core.Wipeundercli.(*Instance).ImportSecret(internal/cli/secrets.go:579), making ordinary forward progress.TestAddSecretVariousSizesalone eats 24.19s of the 30s budget, with99MBat 10.25s and100MB minus 1at 10.19s;TestImportSecretVariousSizesis the same size ladder over the import path, which is where the package runs out of clock.internal/clineeds roughly 55-60s under-raceagainst 9.6s without.Every available lever is one the issue explicitly forbids: raising the timeout papers over it (and the 20s budget would still be missed by 3x),
t.Skipis a skip, and shrinking the sizes deletes boundary coverage for the real 100MB cap atinternal/cli/secrets.go:245and:515. Making the large-secret path genuinely faster is out of scope here. No small in-scope change closes a 6x gap, so this stops and reports rather than forcing one.#52 has the full measurements and four options for a decision. That decision is yours; my read is the build-tag split, since it is the only option that gives up nothing. This PR should land together with whatever #52 concludes, not before it.
Also worth stating plainly: the clean
-raceresult is not evidence that #34's missing file locking is harmless. The suite has no test that touches the vault from two goroutines at once, so there was nothing for the detector to catch. #34 stands entirely on its own.Verification
make lint— clean,0 issues.make fmt-check— clean.make test— fails as described above; the new pattern was confirmed working end to end, printing--- Rerunning with -v for details ---and exiting non-zero after the rerun.script/cibuild— run on bothmainand this branch for the before/after numbers in the table.clawbot referenced this pull request2026-09-03 15:42:05 +02:00
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.