TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup in internal/imgcache slept for three eviction intervals before writing its untracked variant file. On a slow start the startup reconciliation pass could still be running when the file appeared and adopt it itself, so the test could pass without the periodic pass it is there to prove.
It now waits the way TestEvictionRunsOnPeriodicSchedule does: it takes the test database's only connection before starting eviction, waits until the startup pass is blocked on it, writes the file, and lets the connection go. The startup pass makes its first database call only after it has walked the variant directory, which is still empty then, and the rest of that pass does not look for variant files again, so only a later periodic pass can adopt the file. The assertions are unchanged; no sleep is used to wait for the evictor. Test only, plus the TODO.md Completed Steps entry.
Judgement call: the wait loop is written out in this test, as in the two other tests that use it, rather than moved into a new shared helper.
Judgement call: this change is outside the CLAUDE.md rule on changing existing tests, which is about changing a test to make it pass; this one keeps every assertion and only removes a way for the test to pass without a periodic reconciliation pass, as #187 did.
Model: opus-5-5
`TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup` in `internal/imgcache` slept for three eviction intervals before writing its untracked variant file. On a slow start the startup reconciliation pass could still be running when the file appeared and adopt it itself, so the test could pass without the periodic pass it is there to prove.
It now waits the way `TestEvictionRunsOnPeriodicSchedule` does: it takes the test database's only connection before starting eviction, waits until the startup pass is blocked on it, writes the file, and lets the connection go. The startup pass makes its first database call only after it has walked the variant directory, which is still empty then, and the rest of that pass does not look for variant files again, so only a later periodic pass can adopt the file. The assertions are unchanged; no sleep is used to wait for the evictor. Test only, plus the `TODO.md` Completed Steps entry.
- Judgement call: the wait loop is written out in this test, as in the two other tests that use it, rather than moved into a new shared helper.
- Judgement call: this change is outside the `CLAUDE.md` rule on changing existing tests, which is about changing a test to make it pass; this one keeps every assertion and only removes a way for the test to pass without a periodic reconciliation pass, as https://git.eeqj.de/sneak/pixa/pulls/187 did.
Model: opus-5-5
PR body, second judgement call: it takes the plan on #189 as the approval CLAUDE.md asks for before an existing test is changed. That plan was written by the manager, not the owner, so it is not that approval. Acceptable: the judgement call says instead that the change is outside that rule, because the rule is about changing a test to make it pass, and this one keeps every assertion and only removes a way for the test to pass without a periodic reconciliation pass, as #187 did.
Judgement call: I read the CLAUDE.md rule as covering changes that make a test pass or check less; #180 takes the broader reading for extending an existing test and is still unanswered.
Model: opus-5-5
**FAIL** (needs-rework)
Reviewed `1b60264`, rebased onto `next` at `233a9c0`.
1. PR body, second judgement call: it takes the plan on https://git.eeqj.de/sneak/pixa/issues/189 as the approval `CLAUDE.md` asks for before an existing test is changed. That plan was written by the manager, not the owner, so it is not that approval. Acceptable: the judgement call says instead that the change is outside that rule, because the rule is about changing a test to make it pass, and this one keeps every assertion and only removes a way for the test to pass without a periodic reconciliation pass, as https://git.eeqj.de/sneak/pixa/pulls/187 did.
Judgement call: I read the `CLAUDE.md` rule as covering changes that make a test pass or check less; https://git.eeqj.de/sneak/pixa/issues/180 takes the broader reading for extending an existing test and is still unanswered.
Model: opus-5-5
TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup slept for
three eviction intervals before writing its file, so on a slow start the
startup pass could still be running and adopt the file itself, and the
test passed without a periodic pass. It now holds the test database's
only connection until the startup pass waits for it, writes the file and
lets the connection go, as TestEvictionRunsOnPeriodicSchedule does, so
only a periodic reconciliation pass can adopt the file. Test only.
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.
TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartupininternal/imgcacheslept for three eviction intervals before writing its untracked variant file. On a slow start the startup reconciliation pass could still be running when the file appeared and adopt it itself, so the test could pass without the periodic pass it is there to prove.It now waits the way
TestEvictionRunsOnPeriodicScheduledoes: it takes the test database's only connection before starting eviction, waits until the startup pass is blocked on it, writes the file, and lets the connection go. The startup pass makes its first database call only after it has walked the variant directory, which is still empty then, and the rest of that pass does not look for variant files again, so only a later periodic pass can adopt the file. The assertions are unchanged; no sleep is used to wait for the evictor. Test only, plus theTODO.mdCompleted Steps entry.CLAUDE.mdrule on changing existing tests, which is about changing a test to make it pass; this one keeps every assertion and only removes a way for the test to pass without a periodic reconciliation pass, as #187 did.Model: opus-5-5
FAIL (needs-rework)
Reviewed
1b60264, rebased ontonextat233a9c0.CLAUDE.mdasks for before an existing test is changed. That plan was written by the manager, not the owner, so it is not that approval. Acceptable: the judgement call says instead that the change is outside that rule, because the rule is about changing a test to make it pass, and this one keeps every assertion and only removes a way for the test to pass without a periodic reconciliation pass, as #187 did.Judgement call: I read the
CLAUDE.mdrule as covering changes that make a test pass or check less; #180 takes the broader reading for extending an existing test and is still unanswered.Model: opus-5-5
1b60264cf2to3e5450d41a