Guarantee the database is closed on every fatal exit path (closes #4)

fatalf called os.Exit(1), which does not run deferred functions, so
every defer db.Close() was dead on the fatal path: the SQLite WAL was
left uncheckpointed and the -wal/-shm sidecars were left for the next
process to recover. It also made those paths impossible to exercise
in-process.

fatalf is gone. runScan, runReport, runTrees, loadRecords and
resolveRoots return their errors, so the deferred close always runs,
and the only exit point is run() in main.go.

Mapping errors to exit codes needs care: cobra prints the error and
the command's usage text for anything RunE returns, and main mapped
every Execute() error to exit 2. A runtime failure is not a usage
problem, so the runE adapter silences both for the subcommands and
marks their errors fatalError; run() reports a fatalError as
"sfdupes: ..." on stderr and exits 1, and leaves everything else --
cobra's own argument, flag and unknown-command errors, which cobra has
already reported with its usage text -- on exit 2. The bare
"sfdupes" invocation still prints usage and exits 2.

Exit codes and message text are unchanged: 0 on success even with
per-file warnings, 1 fatal, 2 usage, per README section "Error
handling and exit codes". Everything on stdout is still data only.

main_test.go drives the CLI in-process and covers all three: a fatal
error raised after the database is open (a database with no files
table) closes it and leaves no -wal or -shm behind for scan, report
and trees; a nonexistent PATH operand is fatal, not usage, and prints
no usage text; the usage errors still exit 2; and a scan that skipped
an unreadable file still exits 0.

main_test.go

@@ -0,0 +1,397 @@
+package main
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"io"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"testing"
+)
+
+// usageMarker is the first line of cobra's usage text, which a usage
+// error prints and a runtime failure must not.
+const usageMarker = "Usage:"
+
+// walSuffixes are the SQLite sidecar files a WAL-mode database keeps
+// while it is open. A clean close checkpoints the WAL and removes
+// both; finding either afterwards means the database was never closed.
+//
+//nolint:gochecknoglobals // a constant list, immutable by convention
+var walSuffixes = []string{"-wal", "-shm"}
+
+// assertNoSidecars fails when a WAL sidecar is still present beside the
+// database at path.
+func assertNoSidecars(t *testing.T, path string) {
+	t.Helper()
+
+	for _, suffix := range walSuffixes {
+		_, err := os.Stat(path + suffix)
+		if err == nil {
+			t.Errorf("%s%s still present: the database was not closed",
+				path, suffix)
+
+			continue
+		}
+
+		if !errors.Is(err, fs.ErrNotExist) {
+			t.Fatal(err)
+		}
+	}
+}
+
+// captureStdout redirects os.Stdout to a file for the rest of the test
+// and returns a function reading back everything written to it. Only
+// machine-readable data belongs on stdout (README design goal 4), so
+// the tests assert on it directly.
+func captureStdout(t *testing.T) func() string {
+	t.Helper()
+
+	f, err := os.Create(filepath.Join(t.TempDir(), "stdout"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	saved := os.Stdout
+	os.Stdout = f
+
+	t.Cleanup(func() {
+		os.Stdout = saved
+
+		_ = f.Close()
+	})
+
+	return func() string {
+		// Read what has been written without disturbing the write
+		// offset, so the capture can be inspected more than once.
+		size, err := f.Seek(0, io.SeekCurrent)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if size == 0 {
+			return ""
+		}
+
+		b := make([]byte, size)
+
+		_, err = f.ReadAt(b, 0)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		return string(b)
+	}
+}
+
+// brokenDatabase writes a database that opens cleanly and passes the
+// schema-version check but has no files table, so the first query
+// fails with the database already open: a fatal error on a path that
+// owns an open database.
+func brokenDatabase(t *testing.T) string {
+	t.Helper()
+
+	path := testDBPath(t)
+
+	db, err := openDB(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = db.ExecContext(context.Background(),
+		"PRAGMA user_version = "+strconv.Itoa(schemaVersion))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = db.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return path
+}
+
+func TestOpenDatabaseKeepsWALWhileOpen(t *testing.T) {
+	t.Parallel()
+
+	// The premise of the fatal-path tests below: an open database has
+	// a -wal sidecar, so its absence afterwards is evidence that the
+	// database was closed and its WAL checkpointed.
+	path := testDBPath(t)
+
+	db, err := openScanDatabase(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = os.Stat(path + "-wal")
+	if err != nil {
+		t.Fatalf("no -wal beside an open database: %v", err)
+	}
+
+	err = db.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assertNoSidecars(t, path)
+}
+
+func TestRunFatalAfterOpenClosesDatabase(t *testing.T) {
+	// Every subcommand that owns an open database must close it when
+	// it fails: no os.Exit between the open and the return.
+	cases := map[string][]string{
+		cmdScan:   {cmdScan},
+		cmdReport: {cmdReport},
+		cmdTrees:  {cmdTrees},
+	}
+
+	for name, args := range cases {
+		t.Run(name, func(t *testing.T) {
+			path := brokenDatabase(t)
+			t.Setenv(databaseEnv, path)
+
+			if name == cmdScan {
+				args = append(args, t.TempDir())
+			}
+
+			var stderr bytes.Buffer
+
+			stdout := captureStdout(t)
+
+			code := run(args, &stderr)
+			if code != exitFatal {
+				t.Errorf("run(%v) = %d, want %d", args, code, exitFatal)
+			}
+
+			assertNoSidecars(t, path)
+			assertFatalOutput(t, stderr.String(), stdout())
+
+			// Proof that the failure happened after the open: only a
+			// query against the opened database can report this.
+			if !strings.Contains(stderr.String(), "no such table: files") {
+				t.Errorf("stderr = %q, want the failure to come from a "+
+					"query on the open database", stderr.String())
+			}
+		})
+	}
+}
+
+func TestRunMissingOperandIsFatalNotUsage(t *testing.T) {
+	// README §Error handling: a PATH operand that does not exist is a
+	// fatal error (1), not a usage error (2) — and a runtime failure
+	// must not dump the usage text.
+	t.Setenv(databaseEnv, testDBPath(t))
+
+	var stderr bytes.Buffer
+
+	stdout := captureStdout(t)
+
+	missing := filepath.Join(t.TempDir(), "nope")
+
+	code := run([]string{cmdScan, missing}, &stderr)
+	if code != exitFatal {
+		t.Errorf("run(scan %s) = %d, want %d", missing, code, exitFatal)
+	}
+
+	assertFatalOutput(t, stderr.String(), stdout())
+}
+
+// assertFatalOutput checks that a fatal error was reported the way
+// README §Error handling and design goal 4 require: the message on
+// stderr, prefixed with the program name, no usage text, and nothing
+// at all on stdout.
+func assertFatalOutput(t *testing.T, stderr, stdout string) {
+	t.Helper()
+
+	if !strings.Contains(stderr, "sfdupes: ") {
+		t.Errorf("stderr = %q, want a \"sfdupes: \" error report", stderr)
+	}
+
+	if strings.Contains(stderr, usageMarker) {
+		t.Errorf("stderr = %q, want no usage text for a runtime failure",
+			stderr)
+	}
+
+	if stdout != "" {
+		t.Errorf("stdout = %q, want nothing (data only)", stdout)
+	}
+}
+
+func TestRunUsageErrors(t *testing.T) {
+	// Usage errors keep exiting 2 with cobra's own report on stderr.
+	cases := map[string]struct {
+		args []string
+		want string
+	}{
+		"no subcommand":      {[]string{}, usageMarker},
+		"scan without paths": {[]string{cmdScan}, usageMarker},
+		"report with args":   {[]string{cmdReport, "x"}, usageMarker},
+		"trees with args":    {[]string{cmdTrees, "x"}, usageMarker},
+		"unknown flag":       {[]string{cmdScan, "--nope", "/"}, usageMarker},
+		"unknown subcommand": {[]string{"nope"}, "unknown command"},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			// No usage error may reach the database, so point it at a
+			// path that does not exist.
+			t.Setenv(databaseEnv, testDBPath(t))
+
+			var stderr bytes.Buffer
+
+			stdout := captureStdout(t)
+
+			code := run(tc.args, &stderr)
+			if code != exitUsage {
+				t.Errorf("run(%v) = %d, want %d", tc.args, code, exitUsage)
+			}
+
+			if !strings.Contains(stderr.String(), tc.want) {
+				t.Errorf("stderr = %q, want %q", stderr.String(), tc.want)
+			}
+
+			if got := stdout(); got != "" {
+				t.Errorf("stdout = %q, want nothing (data only)", got)
+			}
+		})
+	}
+}
+
+// TestRunHelpAndVersionSucceed checks that the two informational flags
+// exit 0 and keep their human-facing output on stderr.
+//
+//nolint:paralleltest // captureStdout replaces the process-wide os.Stdout
+func TestRunHelpAndVersionSucceed(t *testing.T) {
+	assertHumanOutput(t, "--help")
+	assertHumanOutput(t, "--version")
+}
+
+// assertHumanOutput runs sfdupes with one informational flag and checks
+// that it succeeds with its output on stderr and stdout untouched
+// (README design goal 4).
+func assertHumanOutput(t *testing.T, arg string) {
+	t.Helper()
+
+	var stderr bytes.Buffer
+
+	stdout := captureStdout(t)
+
+	code := run([]string{arg}, &stderr)
+	if code != exitOK {
+		t.Errorf("run(%s) = %d, want %d", arg, code, exitOK)
+	}
+
+	if stderr.Len() == 0 {
+		t.Errorf("run(%s) wrote nothing to stderr", arg)
+	}
+
+	if got := stdout(); got != "" {
+		t.Errorf("stdout = %q, want nothing (data only)", got)
+	}
+}
+
+// scanFixture builds a small tree holding one duplicate pair and one
+// unreadable file and scans it into the database the caller has
+// pointed SFDUPES_DATABASE at. The unreadable file makes the scan warn
+// and skip, which README §Error handling still calls a successful run.
+// It returns the duplicate pair's paths.
+func scanFixture(t *testing.T) []string {
+	t.Helper()
+
+	dir := t.TempDir()
+
+	dupes := []string{
+		writeFile(t, dir, "one/a.bin", pattern(1, 300)),
+		writeFile(t, dir, "two/a.bin", pattern(1, 300)),
+	}
+
+	// Same size as the pair, so the scan queues it for hashing and the
+	// read fails.
+	unreadable := writeFile(t, dir, "unreadable.bin", pattern(2, 300))
+
+	err := os.Chmod(unreadable, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var stderr bytes.Buffer
+
+	stdout := captureStdout(t)
+
+	code := run([]string{cmdScan, dir}, &stderr)
+	if code != exitOK {
+		t.Fatalf("run(scan) = %d, want %d; stderr: %s",
+			code, exitOK, stderr.String())
+	}
+
+	if got := stdout(); got != "" {
+		t.Errorf("scan stdout = %q, want nothing (data only)", got)
+	}
+
+	return dupes
+}
+
+func TestRunScanSucceedsDespiteWarnings(t *testing.T) {
+	path := testDBPath(t)
+	t.Setenv(databaseEnv, path)
+
+	scanFixture(t)
+	assertNoSidecars(t, path)
+}
+
+func TestRunReportSucceeds(t *testing.T) {
+	path := testDBPath(t)
+	t.Setenv(databaseEnv, path)
+
+	dupes := scanFixture(t)
+
+	var stderr bytes.Buffer
+
+	stdout := captureStdout(t)
+
+	code := run([]string{cmdReport}, &stderr)
+	if code != exitOK {
+		t.Fatalf("run(report) = %d, want %d; stderr: %s",
+			code, exitOK, stderr.String())
+	}
+
+	want := "first\tdupe\tsize\n" + dupes[0] + "\t" + dupes[1] + "\t300\n"
+	if got := stdout(); got != want {
+		t.Errorf("stdout = %q, want %q", got, want)
+	}
+
+	assertNoSidecars(t, path)
+}
+
+func TestRunTreesSucceeds(t *testing.T) {
+	path := testDBPath(t)
+	t.Setenv(databaseEnv, path)
+
+	dupes := scanFixture(t)
+
+	var stderr bytes.Buffer
+
+	stdout := captureStdout(t)
+
+	code := run([]string{cmdTrees}, &stderr)
+	if code != exitOK {
+		t.Fatalf("run(trees) = %d, want %d; stderr: %s",
+			code, exitOK, stderr.String())
+	}
+
+	// The two directories holding the duplicate pair are duplicate
+	// trees of each other.
+	want := "first\tdupe\tfiles\tsize\n" +
+		filepath.Dir(dupes[0]) + "\t" + filepath.Dir(dupes[1]) + "\t1\t300\n"
+	if got := stdout(); got != want {
+		t.Errorf("stdout = %q, want %q", got, want)
+	}
+
+	assertNoSidecars(t, path)
+}