test(download): wait for the work, not for the state that precedes it

TestManagerEndToEndAutoPick waits for StateComplete and then asserts
that staging was released and the library was rescanned. Those happen
*after* the state is recorded: manager.go sets StateComplete, then
satisfies the request, then releases staging, then scans. So waiting on
the state is not waiting on either assertion, and on a loaded machine
the worker is descheduled in between and the test reads the world one
step too early:

    manager_test.go:209: staging not released: 1 dirs remain
    manager_test.go:218: library scans = 0, want 1

It passed alone every time and failed three times under a full-suite
run, which is the signature of a test race rather than a broken
manager — nothing here is wrong except what the test chose to wait on.
It blocks pushes, since the pre-push hook is exactly the loaded run.

It polls for the side effects now, through the waitFor this package
already has and already uses for the same reason one file over
(service_test.go waits for a request to become satisfied after the same
StateComplete).

Not reproduced on demand: eight spinners and -count=5 did not provoke
it with or without the fix, so this rests on the ordering being plain
in the code rather than on a red-to-green demonstration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AfVYUVExXsx1nSWrXN8mAh
This commit is contained in:
2026-08-17 09:22:51 -04:00
co-authored by Claude Opus 5
parent 0eeef6048e
commit 409bfd5e89
+15 -16
View File
@@ -199,24 +199,23 @@ func TestManagerEndToEndAutoPick(t *testing.T) {
t.Errorf("expected imported file at %s: %v", want, err)
}
// Staging was released only after a successful import.
entries, err := os.ReadDir(f.staging.Root())
if err != nil {
t.Fatalf("read staging root: %v", err)
}
// Staging release and the rescan happen *after* the state is
// recorded (manager.go sets StateComplete, then releases, then
// scans), so waiting on the state is not waiting on these. Under
// load the worker is descheduled in between and asserting straight
// away reads the world one step too early -- which is exactly how
// this test failed on a busy machine while passing alone.
waitFor(t, func() bool {
entries, err := os.ReadDir(f.staging.Root())
if err != nil || len(entries) != 0 {
return false
}
if len(entries) != 0 {
t.Errorf("staging not released: %d dirs remain", len(entries))
}
f.lib.mu.Lock()
defer f.lib.mu.Unlock()
// The library was told to rescan.
f.lib.mu.Lock()
scanned := len(f.lib.scanned)
f.lib.mu.Unlock()
if scanned != 1 {
t.Errorf("library scans = %d, want 1", scanned)
}
return len(f.lib.scanned) == 1
}, "staging was never released, or the library was never rescanned")
}
// An ambiguous result set must park for the user rather than guess.