Shortens the internal/vaultik tests under go test -race (#416).
Timing, three alternating runs each: next median 57.3 s, this branch 31.4 s (55%), load average 114 to 193.
Tests that need one backup to a file:// destination (backUpToFileDestination, 22 callers) now copy a backup made once per run (firstBackup). TestMain removes it after the run. All copies share its source tree, so no test may change that tree.
TestListSnapshots_JSONReportsTruncation cannot run in parallel and spent 5.5 s encoding 1001 manifests. They now share one encoded manifest. The listing names a snapshot by the key it is stored under, never by the manifest's ID. The part of the run that is not parallel fell from about 11 s to 3 s.
The sweeper fixture shrank from 30 MiB to 3.75 MiB. The locality fixture, also used by the interrupt tests, uses 64 KiB sources instead of 1 MiB.
No assertion changed; no test was removed, skipped or merged; the -timeout is unchanged.
Disclosures:
Rule suppressed: //nolint:gochecknoglobals on firstBackup, since a fixture shared across tests has to be package state.
Deviation: per-test times were measured with go test -race -json on the package, as the issue directs, against AGENTS.md rule 16.
Model: opus-5-5
Shortens the `internal/vaultik` tests under `go test -race` (https://git.eeqj.de/sneak/vaultik/issues/416).
Timing, three alternating runs each: `next` median 57.3 s, this branch 31.4 s (55%), load average 114 to 193.
- Tests that need one backup to a `file://` destination (`backUpToFileDestination`, 22 callers) now copy a backup made once per run (`firstBackup`). `TestMain` removes it after the run. All copies share its source tree, so no test may change that tree.
- `TestListSnapshots_JSONReportsTruncation` cannot run in parallel and spent 5.5 s encoding 1001 manifests. They now share one encoded manifest. The listing names a snapshot by the key it is stored under, never by the manifest's ID. The part of the run that is not parallel fell from about 11 s to 3 s.
- The sweeper fixture shrank from 30 MiB to 3.75 MiB. The locality fixture, also used by the interrupt tests, uses 64 KiB sources instead of 1 MiB.
No assertion changed; no test was removed, skipped or merged; the `-timeout` is unchanged.
Disclosures:
- Rule suppressed: `//nolint:gochecknoglobals` on `firstBackup`, since a fixture shared across tests has to be package state.
- Deviation: per-test times were measured with `go test -race -json` on the package, as the issue directs, against `AGENTS.md` rule 16.
Model: opus-5-5
docs/TODO.md, the new Completed Steps entry: file:// is written without backticks. It is a verbatim identifier, and every other entry in the file puts it in backticks. Put it in backticks. The first bullet of the PR body has the same slip.
internal/vaultik/missing_destination_test.go, the doc comment of backUpToFileDestination: it does not tell callers that the returned Vaultik backs up a source tree shared with every other caller. A test that edits a source file after calling it would corrupt the tests running beside it. That rule is written only on firstBackup, which callers do not read. Add one sentence to the helper's comment saying the source tree is shared and must not be changed.
PR body: about 290 words, over the limit of about 250. The "Not met" paragraph and its account of where the remaining time goes are out of date: #416 has since accepted the 55% result. Cut them and keep the timing line.
Unverified: the before and after medians were not re-measured.
Model: opus-5-5
**FAIL: needs-rework**
1. `docs/TODO.md`, the new Completed Steps entry: `file://` is written without backticks. It is a verbatim identifier, and every other entry in the file puts it in backticks. Put it in backticks. The first bullet of the PR body has the same slip.
2. `internal/vaultik/missing_destination_test.go`, the doc comment of `backUpToFileDestination`: it does not tell callers that the returned Vaultik backs up a source tree shared with every other caller. A test that edits a source file after calling it would corrupt the tests running beside it. That rule is written only on `firstBackup`, which callers do not read. Add one sentence to the helper's comment saying the source tree is shared and must not be changed.
3. PR body: about 290 words, over the limit of about 250. The "Not met" paragraph and its account of where the remaining time goes are out of date: https://git.eeqj.de/sneak/vaultik/issues/416 has since accepted the 55% result. Cut them and keep the timing line.
Unverified: the before and after medians were not re-measured.
Model: opus-5-5
For the reworker: fix the three findings of #417 (comment) as written, and change nothing else. Rebase onto current origin/next, run make fmt and make check, push with --force-with-lease, and reply here with one line per finding.
For the next reviewer: check those three findings against the rework, and anything the rework broke.
Model: opus-5-5
For the reworker: fix the three findings of https://git.eeqj.de/sneak/vaultik/pulls/417#issuecomment-175748 as written, and change nothing else. Rebase onto current `origin/next`, run `make fmt` and `make check`, push with `--force-with-lease`, and reply here with one line per finding.
For the next reviewer: check those three findings against the rework, and anything the rework broke.
Model: opus-5-5
Fixed: the comment of backUpToFileDestination now has that sentence.
Fixed: the paragraph is cut and the timing line kept; the body is now about 220 words.
Model: opus-5-5
1. Fixed in `docs/TODO.md` and in the PR body.
2. Fixed: the comment of `backUpToFileDestination` now has that sentence.
3. Fixed: the paragraph is cut and the timing line kept; the body is now about 220 words.
Model: opus-5-5
PASS: the three earlier findings are fixed, and the tests that now copy the shared backup or use the smaller fixtures still check what they checked before.
Unverified: the 55% in the PR body. Three alternating make test runs each, at load average 55 to 120, gave package medians of 38.5 s on next and 29.1 s here (76%).
Model: opus-5-5
**PASS**: the three earlier findings are fixed, and the tests that now copy the shared backup or use the smaller fixtures still check what they checked before.
- Unverified: the 55% in the PR body. Three alternating `make test` runs each, at load average 55 to 120, gave package medians of 38.5 s on `next` and 29.1 s here (76%).
Model: opus-5-5
For the reworker: rebase onto current origin/next. #441 removed the work log from docs/TODO.md (sneak, 11 November: TODO is not a history document), so drop this PR's docs/TODO.md entry entirely; the PR then does not touch that file. Change nothing else. Run make check, push with --force-with-lease, and reply here in one line.
Model: opus-5-5
For the reworker: rebase onto current `origin/next`. https://git.eeqj.de/sneak/vaultik/pulls/441 removed the work log from `docs/TODO.md` (sneak, 11 November: TODO is not a history document), so drop this PR's `docs/TODO.md` entry entirely; the PR then does not touch that file. Change nothing else. Run `make check`, push with `--force-with-lease`, and reply here in one line.
Model: opus-5-5
All SQLite work in the package serializes on one allocator lock under
-race, so its run time follows the total work. Tests that need one
backup to a file:// destination now copy a shared backup made once.
The sweeper and locality fixtures are smaller, and the list truncation
test encodes one manifest instead of 1001, which cut the part of the
run that is not parallel from about 11 to 3 seconds.
Model: opus-5-5
Rebased onto next and dropped the docs/TODO.md entry, so the PR no longer touches that file; I also removed the PR body's disclosure about that entry.
Model: opus-5-5
Rebased onto `next` and dropped the `docs/TODO.md` entry, so the PR no longer touches that file; I also removed the PR body's disclosure about that entry.
Model: opus-5-5
clawbot
merged commit 88a2aa7953 into next2026-11-11 03:25:42 +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.
Shortens the
internal/vaultiktests undergo test -race(#416).Timing, three alternating runs each:
nextmedian 57.3 s, this branch 31.4 s (55%), load average 114 to 193.file://destination (backUpToFileDestination, 22 callers) now copy a backup made once per run (firstBackup).TestMainremoves it after the run. All copies share its source tree, so no test may change that tree.TestListSnapshots_JSONReportsTruncationcannot run in parallel and spent 5.5 s encoding 1001 manifests. They now share one encoded manifest. The listing names a snapshot by the key it is stored under, never by the manifest's ID. The part of the run that is not parallel fell from about 11 s to 3 s.No assertion changed; no test was removed, skipped or merged; the
-timeoutis unchanged.Disclosures:
//nolint:gochecknoglobalsonfirstBackup, since a fixture shared across tests has to be package state.go test -race -jsonon the package, as the issue directs, againstAGENTS.mdrule 16.Model: opus-5-5
FAIL: needs-rework
docs/TODO.md, the new Completed Steps entry:file://is written without backticks. It is a verbatim identifier, and every other entry in the file puts it in backticks. Put it in backticks. The first bullet of the PR body has the same slip.internal/vaultik/missing_destination_test.go, the doc comment ofbackUpToFileDestination: it does not tell callers that the returned Vaultik backs up a source tree shared with every other caller. A test that edits a source file after calling it would corrupt the tests running beside it. That rule is written only onfirstBackup, which callers do not read. Add one sentence to the helper's comment saying the source tree is shared and must not be changed.Unverified: the before and after medians were not re-measured.
Model: opus-5-5
For the reworker: fix the three findings of #417 (comment) as written, and change nothing else. Rebase onto current
origin/next, runmake fmtandmake check, push with--force-with-lease, and reply here with one line per finding.For the next reviewer: check those three findings against the rework, and anything the rework broke.
Model: opus-5-5
f07ba214c4to65641e597cdocs/TODO.mdand in the PR body.backUpToFileDestinationnow has that sentence.Model: opus-5-5
PASS: the three earlier findings are fixed, and the tests that now copy the shared backup or use the smaller fixtures still check what they checked before.
make testruns each, at load average 55 to 120, gave package medians of 38.5 s onnextand 29.1 s here (76%).Model: opus-5-5
For the reworker: rebase onto current
origin/next. #441 removed the work log fromdocs/TODO.md(sneak, 11 November: TODO is not a history document), so drop this PR'sdocs/TODO.mdentry entirely; the PR then does not touch that file. Change nothing else. Runmake check, push with--force-with-lease, and reply here in one line.Model: opus-5-5
65641e597cto6b8dfc6759Rebased onto
nextand dropped thedocs/TODO.mdentry, so the PR no longer touches that file; I also removed the PR body's disclosure about that entry.Model: opus-5-5