From 0b7078301d5cecdaf002a1a0ed2289664ea0b7ce Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sun, 4 Oct 2026 17:47:26 +0200 Subject: [PATCH] Test the remaining CLI cases (closes #16) Most of the command-line contract was already tested through run. This adds what was missing: report and trees with no database exit 1 with the message telling the user to run scan; a scan that skips an unreadable file prints its warning and counts the skip in its summary; the report and trees summary lines are checked exactly. The scan tests now capture the process's own stdout, which scan would write to directly, so a stray stdout write in scan fails them. The fatal-path test takes its subcommands from the command tree, so a new subcommand wired without runE fails it. The nonexistent-operand test gets an accurate name. Model: opus-5-5 --- TODO.md | 4 ++ cancel_test.go | 20 ++++-- main_test.go | 167 +++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 153 insertions(+), 38 deletions(-) diff --git a/TODO.md b/TODO.md index 1305b90..2e1fa6b 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,10 @@ # Completed Steps +- tests cover a missing database, `scan` keeping stdout empty, its skip + warning, the `report` and `trees` summary lines, and every subcommand + going through `runE` (2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/16) + - `.golangci.yml` replaced with the current canonical copy, which uses `gomodguard_v2`, so lint no longer prints a deprecation warning (2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/26) diff --git a/cancel_test.go b/cancel_test.go index 6058973..556e150 100644 --- a/cancel_test.go +++ b/cancel_test.go @@ -344,13 +344,14 @@ func storedPaths(t *testing.T, path string) []string { // TestRunScanInterrupted calls the scan entrypoint with a context that // is already cancelled, as when a signal arrives at once. It must return -// errInterrupted promptly with its one line on stderr, leave the -// database valid and as it was, and leave nothing in the way of the -// next scan, which must bring the database up to date. +// errInterrupted promptly with its one line on stderr and nothing on +// stdout, leave the database valid and as it was, and leave nothing in +// the way of the next scan, which must bring the database up to date. func TestRunScanInterrupted(t *testing.T) { path := testDBPath(t) t.Setenv(databaseEnv, path) + stdout := captureStdout(t) stderr := captureStderr(t) dir := buildSmokeTree(t) @@ -393,6 +394,10 @@ func TestRunScanInterrupted(t *testing.T) { t.Errorf("stderr = %q, want %q", got, want) } + if got := stdout(); got != "" { + t.Errorf("stdout = %q, want nothing (data only)", got) + } + assertNoSidecars(t, path) if got := storedPaths(t, path); !slices.Equal(got, before) { @@ -415,12 +420,13 @@ func TestRunScanInterrupted(t *testing.T) { // TestRunScanInterruptedMidHash interrupts the scan entrypoint part-way // through its hash phase, after the database is open. It must return // errInterrupted, release the lock, end stderr with its line counting -// every file the walk reached, close the database out of WAL mode, and -// keep the records it hashed. +// every file the walk reached, write nothing to stdout, close the +// database out of WAL mode, and keep the records it hashed. func TestRunScanInterruptedMidHash(t *testing.T) { path := testDBPath(t) t.Setenv(databaseEnv, path) + stdout := captureStdout(t) stderr := captureStderr(t) dir := buildWalkCancelTree(t) @@ -438,6 +444,10 @@ func TestRunScanInterruptedMidHash(t *testing.T) { t.Errorf("stderr = %q, want it to end with %q", got, want) } + if got := stdout(); got != "" { + t.Errorf("stdout = %q, want nothing (data only)", got) + } + assertNoSidecars(t, path) db, err := openReportDatabase(t.Context(), path) diff --git a/main_test.go b/main_test.go index bf4150d..7742365 100644 --- a/main_test.go +++ b/main_test.go @@ -85,21 +85,41 @@ func makeReadOnly(t *testing.T, path string) { // captureStderr redirects os.Stderr to a file for the rest of the test // and returns a function reading back everything written to it. scan -// writes its warnings and summary straight to os.Stderr, not to the -// stderr writer run is given. +// writes its warnings and summary, and report and trees their +// summaries, straight to os.Stderr, not to the stderr writer run is +// given. func captureStderr(t *testing.T) func() string { t.Helper() - f, err := os.Create(filepath.Join(t.TempDir(), "stderr")) + return capture(t, &os.Stderr) +} + +// captureStdout does for os.Stdout what captureStderr does for +// os.Stderr. scan is never given run's stdout writer, so anything it +// printed would go straight to os.Stdout. The scan tests pass os.Stdout +// as run's stdout too, so the one capture sees both. +func captureStdout(t *testing.T) func() string { + t.Helper() + + return capture(t, &os.Stdout) +} + +// capture redirects *std, which is os.Stdout or os.Stderr, to a file +// for the rest of the test and returns a function reading back +// everything written to it. +func capture(t *testing.T, std **os.File) func() string { + t.Helper() + + f, err := os.Create(filepath.Join(t.TempDir(), "output")) if err != nil { t.Fatal(err) } - saved := os.Stderr - os.Stderr = f + saved := *std + *std = f t.Cleanup(func() { - os.Stderr = saved + *std = saved _ = f.Close() }) @@ -184,30 +204,33 @@ func TestRunFatalAfterOpenClosesDatabase(t *testing.T) { // sidecar check is evidence of the close only for scan: report and // trees only read a database that is out of WAL mode, which leaves // nothing on disk whether they close it or not. - cases := map[string][]string{ - cmdScan: {cmdScan}, - cmdReport: {cmdReport}, - cmdTrees: {cmdTrees}, - } + // + // The subcommands come from the command tree, so a new one is + // checked too: one wired with a bare RunE instead of runE reports + // its failure as a usage error, exit 2 with the usage text. + for _, cmd := range newRootCommand(io.Discard, io.Discard).Commands() { + name := cmd.Name() - for name, args := range cases { t.Run(name, func(t *testing.T) { path := brokenDatabase(t) t.Setenv(databaseEnv, path) + args := []string{name} if name == cmdScan { args = append(args, t.TempDir()) } - var stdout, stderr bytes.Buffer + stdout := captureStdout(t) - code := run(args, &stdout, &stderr) + var stderr bytes.Buffer + + code := run(args, os.Stdout, &stderr) if code != exitFatal { t.Errorf("run(%v) = %d, want %d", args, code, exitFatal) } assertNoSidecars(t, path) - assertFatalOutput(t, stderr.String(), stdout.String()) + assertFatalOutput(t, stderr.String(), stdout()) // Proof that the failure happened after the open: only a // query against the opened database can report this. @@ -219,22 +242,24 @@ func TestRunFatalAfterOpenClosesDatabase(t *testing.T) { } } -func TestRunMissingOperandIsFatalNotUsage(t *testing.T) { +func TestRunNonexistentPathIsFatalNotUsage(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 stdout, stderr bytes.Buffer + stdout := captureStdout(t) + + var stderr bytes.Buffer missing := filepath.Join(t.TempDir(), "nope") - code := run([]string{cmdScan, missing}, &stdout, &stderr) + code := run([]string{cmdScan, missing}, os.Stdout, &stderr) if code != exitFatal { t.Errorf("run(scan %s) = %d, want %d", missing, code, exitFatal) } - assertFatalOutput(t, stderr.String(), stdout.String()) + assertFatalOutput(t, stderr.String(), stdout()) } // assertFatalOutput checks that a fatal error was reported the way @@ -258,6 +283,34 @@ func assertFatalOutput(t *testing.T, stderr, stdout string) { } } +func TestRunMissingDatabaseIsFatal(t *testing.T) { + // README §Database: report and trees need an existing database; a + // missing one exits 1 with a message telling the user to run scan. + for _, name := range []string{cmdReport, cmdTrees} { + t.Run(name, func(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + var stdout, stderr bytes.Buffer + + code := run([]string{name}, &stdout, &stderr) + if code != exitFatal { + t.Errorf("run(%s) = %d, want %d", name, code, exitFatal) + } + + want := "sfdupes: " + path + ": no database (run \"sfdupes " + + "scan\" first, or set " + databaseEnv + ")\n" + if got := stderr.String(); got != want { + t.Errorf("stderr = %q, want %q", got, want) + } + + if got := stdout.String(); got != "" { + t.Errorf("stdout = %q, want nothing (data only)", got) + } + }) + } +} + func TestRunUsageErrors(t *testing.T) { // Usage errors keep exiting 2 with cobra's own report on stderr. cases := map[string]struct { @@ -435,17 +488,16 @@ func scanFixture(t *testing.T) []string { func scanOK(t *testing.T, operands ...string) string { t.Helper() - var stdout bytes.Buffer - + stdout := captureStdout(t) stderr := captureStderr(t) - code := run(append([]string{cmdScan}, operands...), &stdout, os.Stderr) + code := run(append([]string{cmdScan}, operands...), os.Stdout, os.Stderr) if code != exitOK { t.Fatalf("run(scan %q) = %d, want %d; stderr: %s", operands, code, exitOK, stderr()) } - if got := stdout.String(); got != "" { + if got := stdout(); got != "" { t.Errorf("scan stdout = %q, want nothing (data only)", got) } @@ -453,10 +505,41 @@ func scanOK(t *testing.T, operands ...string) string { } func TestRunScanSucceedsDespiteWarnings(t *testing.T) { + // README §Error handling: a scan that skips a file it cannot read + // warns, counts the skip in its summary, and still exits 0, which + // scanOK checks along with the empty stdout. + if os.Geteuid() == 0 { + t.Skip("root ignores file permissions") + } + path := testDBPath(t) t.Setenv(databaseEnv, path) - scanFixture(t) + dir := t.TempDir() + writeFile(t, dir, "a.bin", pattern(1, 300)) + + // Same size as a.bin, so the scan reads it, and the read fails. + unreadable := writeFile(t, dir, "unreadable.bin", pattern(2, 300)) + + err := os.Chmod(unreadable, 0) + if err != nil { + t.Fatal(err) + } + + stderr := scanOK(t, dir) + + warning := "hash " + unreadable + ": open " + unreadable + + ": permission denied\n" + if !strings.Contains(stderr, warning) { + t.Errorf("stderr = %q, want %q", stderr, warning) + } + + summary := "scan: 1 files seen (1 added, 0 updated, 0 removed, " + + "0 unchanged), 1 skipped\n" + if !strings.Contains(stderr, summary) { + t.Errorf("stderr = %q, want %q", stderr, summary) + } + assertNoSidecars(t, path) } @@ -563,12 +646,14 @@ func TestRunReportSucceeds(t *testing.T) { dupes := scanFixture(t) - var stdout, stderr bytes.Buffer + var stdout bytes.Buffer - code := run([]string{cmdReport}, &stdout, &stderr) + stderr := captureStderr(t) + + code := run([]string{cmdReport}, &stdout, os.Stderr) if code != exitOK { t.Fatalf("run(report) = %d, want %d; stderr: %s", - code, exitOK, stderr.String()) + code, exitOK, stderr()) } want := "first\tdupe\tsize\n" + dupes[0] + "\t" + dupes[1] + "\t300\n" @@ -576,6 +661,12 @@ func TestRunReportSucceeds(t *testing.T) { t.Errorf("stdout = %q, want %q", got, want) } + want = "report: 2 records read, 1 duplicate groups, 1 dupe files, " + + "300 B reclaimable\n" + if got := stderr(); got != want { + t.Errorf("stderr = %q, want %q", got, want) + } + assertNoSidecars(t, path) } @@ -585,12 +676,14 @@ func TestRunTreesSucceeds(t *testing.T) { dupes := scanFixture(t) - var stdout, stderr bytes.Buffer + var stdout bytes.Buffer - code := run([]string{cmdTrees}, &stdout, &stderr) + stderr := captureStderr(t) + + code := run([]string{cmdTrees}, &stdout, os.Stderr) if code != exitOK { t.Fatalf("run(trees) = %d, want %d; stderr: %s", - code, exitOK, stderr.String()) + code, exitOK, stderr()) } // The two directories holding the duplicate pair are duplicate @@ -601,6 +694,12 @@ func TestRunTreesSucceeds(t *testing.T) { t.Errorf("stdout = %q, want %q", got, want) } + want = "trees: 2 records read, 1 duplicate tree groups, 1 dupe trees, " + + "300 B reclaimable\n" + if got := stderr(); got != want { + t.Errorf("stderr = %q, want %q", got, want) + } + assertNoSidecars(t, path) } @@ -730,9 +829,11 @@ func TestRunSecondScanFails(t *testing.T) { holdScanLock(t, path) - var stdout, stderr bytes.Buffer + stdout := captureStdout(t) - code := run([]string{cmdScan, t.TempDir()}, &stdout, &stderr) + var stderr bytes.Buffer + + code := run([]string{cmdScan, t.TempDir()}, os.Stdout, &stderr) if code != exitFatal { t.Errorf("run(scan) = %d, want %d", code, exitFatal) } @@ -743,7 +844,7 @@ func TestRunSecondScanFails(t *testing.T) { t.Errorf("stderr = %q, want %q", got, want) } - if got := stdout.String(); got != "" { + if got := stdout(); got != "" { t.Errorf("stdout = %q, want nothing (data only)", got) }