From 712a82e08bc980c01d4f27e23e1191f11eb4eda9 Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 4 Oct 2026 12:12:32 +0000 Subject: [PATCH] Test both hashWorker cancellation checks on their own (closes #83) TestHashWorkerDropsQueuedRuns now passes hashWorker a hash function that records being called, so a worker that hashes a run after the scan is cancelled fails the test every time instead of only when it then chose to send its result. TestHashWorkerAbandonsBlockedSend cancels the scan from inside the hash function and leaves the result channel unread, so the worker can only return through the cancellation case beside its send. The scan tests could not show this, because stop drains results and frees a parked worker anyway. Model: opus-5-5 --- TODO.md | 3 ++ cancel_test.go | 74 +++++++++++++++++++++++++++++++++++--------------- 2 files changed, 55 insertions(+), 22 deletions(-) diff --git a/TODO.md b/TODO.md index d146d28..d37d1e8 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 database path holding `?`, `#` or `%` opens exactly the file it names (2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/55) diff --git a/cancel_test.go b/cancel_test.go index 8723bb4..6058973 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 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: filepath.Join(t.TempDir(), "missing"), size: 1}} close(jobs) + var hashed atomic.Bool + + hash := func(path string, size int64) (string, string, string, error) { + hashed.Store(true) + + return hashSignature(path, size) + } + 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: filepath.Join(t.TempDir(), "missing"), size: 1}} + + // The scan is cancelled while the worker hashes, so the worker has + // already passed the check that drops queued runs. + hash := func(path string, size int64) (string, string, string, error) { + cancel() + + return hashSignature(path, size) + } + + 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 -- 2.54.0