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

README.md

@@ -283,9 +283,7 @@ All three subcommands operate on a single SQLite database file:
 
 -   Location: the value of the `SFDUPES_DATABASE` environment variable
     when set and non-empty, otherwise `/var/lib/sfdupes/db.sqlite`.
-    There is no command-line flag. The path names the file exactly,
-    whatever characters it holds (`?`, `#` and `%` included); a
-    relative path is relative to the working directory.
+    There is no command-line flag.
 -   `scan` creates the database (and its parent directory) on first
     use. `report` and `trees` require an existing database; a missing
     database file is a fatal error (exit 1) telling the user to run

TODO.md

@@ -29,8 +29,8 @@
 
 # Completed Steps
 
-- a database path holding `?`, `#` or `%` opens exactly the file it names
-  (2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/55)
+- a test fails when either `hashWorker` cancellation check in `scan.go`
+  is removed (2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/83)
 
 - `scan` rejects `--workers` below 1 as a usage error instead of
   running single-threaded (2026-10-04,

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

db.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"io/fs"
-	"net/url"
 	"os"
 	"path/filepath"
 	"slices"
@@ -108,19 +107,7 @@ const reportParams = "mode=ro" +
 // openDB opens the SQLite database at path with the connection
 // parameters params. It does not create or verify the schema.
 func openDB(path, params string) (*sql.DB, error) {
-	// The path is escaped into a file: URI, so ?, # and % in it stay
-	// part of the file name. SQLite reads what follows file:// up to
-	// the next / as a host name, so an absolute path goes after an
-	// empty host (file:///abs) and a relative path goes without one
-	// (file:rel).
-	uri := url.URL{
-		Scheme:   "file",
-		OmitHost: !filepath.IsAbs(path),
-		Path:     path,
-		RawQuery: params,
-	}
-
-	db, err := sql.Open("sqlite", uri.String())
+	db, err := sql.Open("sqlite", "file:"+path+"?"+params)
 	if err != nil {
 		return nil, fmt.Errorf("open database %s: %w", path, err)
 	}

main_test.go

@@ -8,7 +8,6 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
-	"slices"
 	"strconv"
 	"strings"
 	"testing"
@@ -640,73 +639,6 @@ func TestRunReportsNeedOnlyReadAccess(t *testing.T) {
 	}
 }
 
-// assertRunsUseDatabase runs scan, then report and trees, against the
-// database that SFDUPES_DATABASE names, the file name in dir. It fails
-// unless the reports find the duplicate pair the scan recorded and dir
-// then holds only that file and its lock file: nothing was created
-// under a shortened name.
-func assertRunsUseDatabase(t *testing.T, dir, name string) {
-	t.Helper()
-
-	dupes := scanFixture(t)
-
-	want := "first\tdupe\tsize\n" + dupes[0] + "\t" + dupes[1] + "\t300\n"
-	if got := runStdout(t, cmdReport); got != want {
-		t.Errorf("report stdout = %q, want %q", got, want)
-	}
-
-	want = "first\tdupe\tfiles\tsize\n" +
-		filepath.Dir(dupes[0]) + "\t" + filepath.Dir(dupes[1]) + "\t1\t300\n"
-	if got := runStdout(t, cmdTrees); got != want {
-		t.Errorf("trees stdout = %q, want %q", got, want)
-	}
-
-	entries, err := os.ReadDir(dir)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	got := make([]string, 0, len(entries))
-	for _, e := range entries {
-		got = append(got, e.Name())
-	}
-
-	if wantFiles := []string{name, name + ".lock"}; !slices.Equal(got, wantFiles) {
-		t.Errorf("%s holds %q, want %q", dir, got, wantFiles)
-	}
-}
-
-func TestRunDatabasePathUsedAsGiven(t *testing.T) {
-	// README §Database: the path names the database file exactly. In
-	// SQLite's connection string an unescaped ? or # would end the file
-	// name and % would start an escape, and a path starting with //
-	// could be read as a host name. %25 is a valid escape, so unescaped
-	// this name opens a file named a without any error.
-	const name = "a?b#c%25d e.sqlite"
-
-	t.Run("absolute", func(t *testing.T) {
-		dir := t.TempDir()
-		t.Setenv(databaseEnv, filepath.Join(dir, name))
-
-		assertRunsUseDatabase(t, dir, name)
-	})
-
-	t.Run("leading double slash", func(t *testing.T) {
-		dir := t.TempDir()
-		t.Setenv(databaseEnv, "/"+filepath.Join(dir, name))
-
-		assertRunsUseDatabase(t, dir, name)
-	})
-
-	t.Run("relative", func(t *testing.T) {
-		dir := t.TempDir()
-		t.Chdir(dir)
-		t.Setenv(databaseEnv, name)
-
-		assertRunsUseDatabase(t, dir, name)
-	})
-}
-
 // holdScanLock takes the lock on the database at path, as a running
 // scan does, and holds it until the test ends. It fails the test when
 // the lock is already held.