diff --git a/TODO.md b/TODO.md index 53e2e68..d700e33 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,9 @@ # Completed Steps +- a test fails when either `hashWorker` cancellation check in `scan.go` + is removed (2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/83) + - a test fails when either walk cancellation check in `scan.go` is removed (2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/81) diff --git a/cancel_test.go b/cancel_test.go index 8723bb4..ed0525e 100644 --- a/cancel_test.go +++ b/cancel_test.go @@ -729,47 +729,77 @@ func TestFeedHashJobsClosesJobsWhenCancelled(t *testing.T) { // TestHashWorkerDropsQueuedRuns checks that a cancelled hash worker // keeps reading jobs and drops the runs rather than reading files // nobody wants the hashes of — while still letting the range run out -// so the pool tears down. The queued run names a file that does not -// exist, so a worker that hashed it anyway would produce a result. -// -// hashWorker's other cancellation exit, abandoning the send of a -// result, is reachable from the scan: stop cancels the pool before it -// drains results, so a worker waiting on that send can leave through -// it. The tests that stop a scan mid-hash, among them -// TestScanHashWriteFailureUnwindsPool, reach it in some runs only, -// depending on timing, and no test fails without it, since stop's -// drain frees a waiting worker anyway. This test, for its part, catches -// a removed drop check in some runs only: a worker that hashes the run -// anyway then picks at random between sending the result and leaving. +// so the pool tears down. The hash function only records that it was +// called, so a worker that hashed the queued run anyway is caught +// every time. func TestHashWorkerDropsQueuedRuns(t *testing.T) { t.Parallel() done := make(chan struct{}) jobs := make(chan []fileRec, 1) - results := make(chan hashResult, 1) + results := make(chan hashResult) - run := []fileRec{{path: filepath.Join(t.TempDir(), "missing"), size: 1}} - - jobs <- run + jobs <- []fileRec{{path: "a", size: 1}} close(jobs) + var hashed atomic.Bool + + hash := func(string, int64) (string, string, string, error) { + hashed.Store(true) + + return "", "", "", nil + } + go func() { defer close(done) - hashWorker(cancelledContext(t), jobs, results, hashSignature) + hashWorker(cancelledContext(t), jobs, results, hash) }() awaitReturn(t, done, "hashWorker") - select { - case r := <-results: - t.Errorf("cancelled hash worker produced %+v, want the run dropped", - r) - default: + if hashed.Load() { + t.Error("cancelled hash worker hashed the queued run, want it dropped") } } +// TestHashWorkerAbandonsBlockedSend checks that a hash worker with a +// result to deliver and nobody to deliver it to leaves once the scan +// is cancelled, instead of holding the pool open. The scan tests do +// not catch this: stop drains results, which frees a parked worker +// anyway. +func TestHashWorkerAbandonsBlockedSend(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + + done := make(chan struct{}) + jobs := make(chan []fileRec, 1) + // Unbuffered and unread, with jobs left open: the worker's only way + // out is the cancellation case beside its send. + results := make(chan hashResult) + + jobs <- []fileRec{{path: "a", size: 1}} + + // The scan is cancelled while the worker hashes, so the worker has + // already passed the check that drops queued runs. + hash := func(string, int64) (string, string, string, error) { + cancel() + + return "", "", "", nil + } + + go func() { + defer close(done) + + hashWorker(ctx, jobs, results, hash) + }() + + awaitReturn(t, done, "hashWorker") +} + // TestHashPhaseCancelledReturnsContextError checks the result loop's // own exit: with the pool cancelled, no result will ever arrive, and // the loop must leave through the cancellation rather than wait for a