Format Markdown with prettier in make fmt and make fmt-check (closes #19)

script/fmt and script/fmt-check run prettier over every Markdown file
again, next to gofmt. prettier is pinned by hash through package.json
and yarn.lock, copied from the prompts repo with .prettierrc and
.prettierignore, and is never installed on a host: a new prettier stage
of the Dockerfile installs it into a digest-pinned node image, and both
scripts build that stage and run it with the repository mounted. CI
checks the Markdown in a markdown stage that the build stage waits on.
Because make fmt-check now runs docker, the Dockerfile runs gofmt
directly in its lint stage instead. All Markdown is reformatted.

Model: opus-5-5

Dockerfile

@@ -29,10 +29,14 @@ ARG CHECK_EPOCH
 # nesting docker inside this image. Same reason `make check` is gone
 # from the build stage below, and `make fmt-check` from both stages: it
 # runs prettier through docker too. Its gofmt half is the step below,
-# its Markdown half the markdown stage further down.
+# its Markdown half the markdown stage further down. gofmt's output is
+# assigned to a variable first so that its own exit status, as when it
+# cannot parse a file, still fails the step.
 RUN echo "gate gofmt, epoch ${CHECK_EPOCH}" && \
-    test -z "$(gofmt -s -l .)" || \
-    { echo "gofmt: files not formatted:" >&2; gofmt -s -l . >&2; exit 1; }
+    files="$(gofmt -s -l .)" && \
+    if [ -n "$files" ]; then \
+        echo "gofmt: files not formatted:" >&2; echo "$files" >&2; exit 1; \
+    fi
 
 # The FROM above and the one in Dockerfile.lint pin the same linter
 # twice, and nothing else keeps them in sync; this fails the build when

TODO.md

@@ -32,6 +32,16 @@
   CI checks it; all Markdown reformatted (2026-10-04,
   https://git.eeqj.de/sneak/sfdupes/issues/19)
 
+- a test fails when either walk cancellation check in `scan.go` is removed
+  (2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/81)
+
+- test that `scan` refuses a database with another schema version (2026-10-04,
+  https://git.eeqj.de/sneak/sfdupes/issues/64)
+
+- correct four inaccurate comments in `cancel_test.go` and rename
+  `walkCancelInFlightDirs` to `walkCancelInFlightFiles` (2026-10-04,
+  https://git.eeqj.de/sneak/sfdupes/issues/33)
+
 - test the `-x` filesystem-boundary rules in `subdirJob` (2026-10-04,
   https://git.eeqj.de/sneak/sfdupes/issues/17)
 

cancel_test.go

@@ -18,11 +18,21 @@ import (
 	"time"
 )
 
-// poolUnwind bounds how long a goroutine is given to leave a pool
-// after its context is cancelled. Only a failing run ever waits this
-// long: a pool that ignored its cancellation parks forever, and this
-// is what turns that into a failed assertion instead of a suite that
-// hangs until the test binary's own timeout.
+// This file gathers the tests for scan cancellation and worker-pool
+// unwinding. Everything it exercises lives in scan.go, so by the repo's
+// convention of one test file per source file it would belong in
+// scan_test.go. It is kept separate on purpose: cancellation behaviour
+// cuts across both the walk pool and the hash pool as a single concern,
+// and scan_test.go is already over 1,600 lines. That is the deliberate
+// exception the convention otherwise expects to be stated.
+
+// poolUnwind bounds how long a test waits for a cancellation to take
+// effect: for a goroutine to return or a channel to close once its
+// context is cancelled, or for a signal to cancel the scan's context.
+// Only a failing run waits this long, and the bound is what makes that
+// failure an assertion instead of a hang. A call made without it, as
+// most of this file's scans are, has no bound: a regression that parks
+// it is caught only as the test binary's own timeout.
 const poolUnwind = 2 * time.Second
 
 // walkClock is a context whose cancellation is driven by the scan's
@@ -33,9 +43,14 @@ const poolUnwind = 2 * time.Second
 //
 // The accounting behind the n chosen by each test: every blocking
 // channel operation in the walk selects on Done, so the walk spends
-// one consultation per file event plus a couple per directory, while
-// the index load that runs ahead of it spends a small fixed number
-// (three) whatever the record count.
+// one consultation per file event plus a couple per directory. The
+// index load that runs ahead of it also consults Done, but a bounded
+// number of times that does not grow with the record count. The tests
+// depend on that property, not on the bound's exact value: each test
+// sets n from the consultations of the walk, plus those of the hash
+// phase when it cancels mid-hash, far from both ends of the phase it
+// interrupts, so the cancellation lands inside that phase whatever the
+// record count.
 type walkClock struct {
 	n    int64
 	seen atomic.Int64
@@ -91,11 +106,14 @@ func (c *walkClock) Value(_ any) any {
 // directory still queued and only the handful already in flight can
 // emit anything more.
 const (
-	walkCancelDirs         = 100
-	walkCancelFilesPerDir  = 20
-	walkCancelFiles        = walkCancelDirs * walkCancelFilesPerDir
-	walkCancelWorkers      = 4
-	walkCancelInFlightDirs = walkCancelWorkers * walkCancelFilesPerDir
+	walkCancelDirs        = 100
+	walkCancelFilesPerDir = 20
+	walkCancelFiles       = walkCancelDirs * walkCancelFilesPerDir
+	walkCancelWorkers     = 4
+	// The most files the walkCancelWorkers directories already in
+	// flight when the scan is cancelled can still emit, at
+	// walkCancelFilesPerDir each. A file count, not a directory count.
+	walkCancelInFlightFiles = walkCancelWorkers * walkCancelFilesPerDir
 )
 
 // walkCancelAtDone is the consultation on which the fixture's context
@@ -158,6 +176,11 @@ func assertRecordsIntact(t *testing.T, db *sql.DB, before []string) {
 // unreachable, makes the scan carry its truncated view into the update
 // phase, which counts every record the walk never reached for removal.
 //
+// The syncScan call here is not bounded by poolUnwind: a regression
+// that left a worker pool parked would hang it, and that regression is
+// caught only by the test binary's own timeout, not by a quick
+// assertion.
+//
 //nolint:paralleltest // counts goroutines: must not run beside others
 func TestSyncScanCancelledMidWalkKeepsRecords(t *testing.T) {
 	dir := buildWalkCancelTree(t)
@@ -209,10 +232,11 @@ func assertWalkGuardAborted(t *testing.T, st scanStats, err error) {
 	}
 
 	// The workers drop every directory still queued once the scan is
-	// cancelled, so only the directories already in flight can add to
-	// the census after the fact. A census beyond that bound would mean
-	// the cancellation was not observed where it should have been.
-	limit := walkCancelAtDone + walkCancelInFlightDirs
+	// cancelled, so only the files in the directories already in flight
+	// can add to the census after the fact. A census beyond that bound
+	// would mean the cancellation was not observed where it should have
+	// been.
+	limit := walkCancelAtDone + walkCancelInFlightFiles
 	if st.unchanged > limit {
 		t.Errorf("census covers %d files, want at most %d: the walk kept "+
 			"taking directories off the queue after cancellation",
@@ -562,20 +586,49 @@ func TestSendEventAbandonsBlockedSend(t *testing.T) {
 	awaitReturn(t, done, "sendEvent")
 }
 
-// TestWalkWorkersDropQueuedDirs checks that cancelled walk workers keep
-// reading jobs and drop the directories rather than stopping their
-// read: the range over jobs has to run out for the pool to tear down
-// and close its event stream.
-func TestWalkWorkersDropQueuedDirs(t *testing.T) {
+// TestWalkOneDirStopsWhenCancelled checks that a cancelled scan stops
+// reading a directory instead of going through the rest of its
+// entries. A walk that kept going would return the subdirectory below
+// to descend into. Unlike a file event, that return is not a send the
+// cancellation can abandon, so the test catches the regression every
+// time.
+func TestWalkOneDirStopsWhenCancelled(t *testing.T) {
 	t.Parallel()
 
 	dir := t.TempDir()
-	writeEmptyFiles(t, dir, walkCancelFilesPerDir)
+
+	err := os.Mkdir(filepath.Join(dir, "sub"), 0o750)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Unbuffered and unread: on a cancelled scan every send gives up.
+	events := make(chan walkEvent)
+
+	subs := walkOneDir(cancelledContext(t), dirJob{path: dir}, false, events)
+	if len(subs) != 0 {
+		t.Errorf("cancelled walkOneDir returned %+v to descend into, "+
+			"want none", subs)
+	}
+}
+
+// TestWalkWorkersDropQueuedDirs checks that cancelled walk workers keep
+// reading jobs and drop the directories rather than stopping their
+// read: the range over jobs has to run out for the pool to tear down
+// and close its event stream. The queued directory does not exist, so
+// a worker that walked it anyway would send a warning before
+// walkOneDir's own cancellation check could stop it. On a cancelled
+// scan that send delivers or gives up at random, so with 64 jobs
+// queued the regression has a one in 2^64 chance of passing.
+func TestWalkWorkersDropQueuedDirs(t *testing.T) {
+	t.Parallel()
+
+	missing := filepath.Join(t.TempDir(), "missing")
 
 	jobs, _, events := startWalkWorkers(cancelledContext(t), 2, false)
 
-	for range 4 {
-		jobs <- dirJob{path: dir}
+	for range 64 {
+		jobs <- dirJob{path: missing}
 	}
 
 	close(jobs)
@@ -646,7 +699,11 @@ func TestDispatchDirsClosesJobsWhenCancelled(t *testing.T) {
 
 // TestFeedHashJobsClosesJobsWhenCancelled checks that the hash feeder
 // abandons the runs it has not queued yet and still closes the job
-// channel, which is what lets the workers' range terminate.
+// channel, which is what lets the workers' range terminate. The
+// receive on jobs below is not bounded: a feeder that returned without
+// closing jobs would leave that receive with no sender and no close, so
+// this regression is caught by the test binary's timeout rather than by
+// a bounded assertion.
 func TestFeedHashJobsClosesJobsWhenCancelled(t *testing.T) {
 	t.Parallel()
 
@@ -674,6 +731,16 @@ func TestFeedHashJobsClosesJobsWhenCancelled(t *testing.T) {
 // 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.
 func TestHashWorkerDropsQueuedRuns(t *testing.T) {
 	t.Parallel()
 
@@ -706,7 +773,10 @@ func TestHashWorkerDropsQueuedRuns(t *testing.T) {
 // 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
-// receive that cannot happen.
+// receive that cannot happen. This call is not bounded by poolUnwind: a
+// loop that dropped its cancellation case would block on that receive,
+// so the regression surfaces as the test binary's timeout rather than
+// as a bounded assertion.
 func TestHashPhaseCancelledReturnsContextError(t *testing.T) {
 	t.Parallel()
 

db_test.go

@@ -157,9 +157,11 @@ func TestOpenReportDatabaseMissing(t *testing.T) {
 	}
 }
 
-func TestOpenReportDatabaseVersionMismatch(t *testing.T) {
+func TestOpenDatabaseVersionMismatch(t *testing.T) {
 	t.Parallel()
 
+	// A database stamped with a schema version other than 0 and
+	// schemaVersion. report, trees and scan must all refuse it.
 	path := testDBPath(t)
 
 	db, err := openScanDatabase(t.Context(), path)
@@ -176,7 +178,12 @@ func TestOpenReportDatabaseVersionMismatch(t *testing.T) {
 
 	_, err = openReportDatabase(t.Context(), path)
 	if !errors.Is(err, errSchemaVersion) {
-		t.Fatalf("err = %v, want errSchemaVersion", err)
+		t.Fatalf("report: err = %v, want errSchemaVersion", err)
+	}
+
+	_, err = openScanDatabase(t.Context(), path)
+	if !errors.Is(err, errSchemaVersion) {
+		t.Fatalf("scan: err = %v, want errSchemaVersion", err)
 	}
 }