Test report and trees stdout write failures (closes #30)
check / check (push) Successful in 1m31s

report and trees already checked every stdout write and the final
flush. run now takes the stdout it hands to them, so tests pass a
closed file or a failing writer instead of swapping os.Stdout: a
closed stdout exits 1 with a one-line diagnostic, and the writer's
error reaches the caller.

README "Error handling" now states the two cases that never reach
sfdupes as a failed write: a pipe reader that exits early ends the
process with SIGPIPE, as with cat; and stdout closed with >&- is
replaced by /dev/null by the Go runtime before main runs, so the run
succeeds.

Model: opus-5-5
This commit is contained in:
2026-10-03 13:37:06 +00:00
parent c887f80f57
commit 7f863386f7
9 changed files with 128 additions and 103 deletions
+12
View File
@@ -561,6 +561,18 @@ Additional requirements:
- `2`: usage error (including `scan` with no `PATH` operand and - `2`: usage error (including `scan` with no `PATH` operand and
`report`/`trees` with any positional argument). `report`/`trees` with any positional argument).
A stdout write failure, such as a full disk, is reported in one line on
stderr and exits 1. Two cases never reach sfdupes as a failed write:
- When the reader of a stdout pipe exits early, as in
`sfdupes report | head`, the next write ends sfdupes with `SIGPIPE`,
quietly and without a summary, the way `cat` or `sort` end. The
shell reports the signal (status 141 in most shells), not exit 1.
- When stdout is closed outright (`sfdupes report >&-`), the Go
runtime opens `/dev/null` in its place before sfdupes starts, so
the output is discarded and the run succeeds, as with
`> /dev/null`.
## Entrypoints ## Entrypoints
This repository adheres to the This repository adheres to the
+4
View File
@@ -29,6 +29,10 @@
# Completed Steps # Completed Steps
- test stdout write failures in `report` and `trees`; README states that
`| head` ends sfdupes by `SIGPIPE` and `>&-` writes to `/dev/null`
(2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/30)
- escape tabs, newlines, carriage returns and backslashes in report, - escape tabs, newlines, carriage returns and backslashes in report,
trees and warning paths; the root directory's path is `/` trees and warning paths; the root directory's path is `/`
(2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/7) (2026-10-03, https://git.eeqj.de/sneak/sfdupes/issues/7)
+12 -7
View File
@@ -57,22 +57,27 @@ var errNoSubcommand = errors.New("no subcommand")
var Version = "dev" var Version = "dev"
func main() { func main() {
os.Exit(run(os.Args[1:], os.Stderr)) // Once the reader of a stdout pipe has gone, as in "sfdupes report |
// head", the Go runtime ends the process with SIGPIPE on the next
// write instead of returning an error (README "Error handling").
// Registering for SIGPIPE with os/signal would change that.
os.Exit(run(os.Args[1:], os.Stdout, os.Stderr))
} }
// run executes args against the command tree and returns the process // run executes args against the command tree and returns the process
// exit code. It is the program's single exit point: the subcommands // exit code. It is the program's single exit point: the subcommands
// return their errors instead of exiting, so every deferred cleanup — // return their errors instead of exiting, so every deferred cleanup —
// above all closing the database, which checkpoints the SQLite WAL — // above all closing the database, which checkpoints the SQLite WAL —
// runs before the process ends. // runs before the process ends. The report and trees subcommands write
func run(args []string, stderr io.Writer) int { // their data to stdout.
func run(args []string, stdout, stderr io.Writer) int {
// A nil slice makes cobra fall back to os.Args, which would let a // A nil slice makes cobra fall back to os.Args, which would let a
// test binary's own flags reach the command tree. // test binary's own flags reach the command tree.
if args == nil { if args == nil {
args = []string{} args = []string{}
} }
root := newRootCommand(stderr) root := newRootCommand(stdout, stderr)
root.SetArgs(args) root.SetArgs(args)
err := root.Execute() err := root.Execute()
@@ -98,7 +103,7 @@ func run(args []string, stderr io.Writer) int {
// newRootCommand builds the command tree. Everything on stdout is // newRootCommand builds the command tree. Everything on stdout is
// machine-readable data; all human-facing output (help, usage, errors) // machine-readable data; all human-facing output (help, usage, errors)
// goes to stderr. // goes to stderr.
func newRootCommand(stderr io.Writer) *cobra.Command { func newRootCommand(stdout, stderr io.Writer) *cobra.Command {
root := &cobra.Command{ root := &cobra.Command{
Use: "sfdupes", Use: "sfdupes",
Short: "Find candidate duplicate files by size and head/tail/content SHA-256", Short: "Find candidate duplicate files by size and head/tail/content SHA-256",
@@ -140,7 +145,7 @@ func newRootCommand(stderr io.Writer) *cobra.Command {
Short: "Read the scan database and print the file-level duplicates report", Short: "Read the scan database and print the file-level duplicates report",
Args: cobra.NoArgs, Args: cobra.NoArgs,
RunE: runE(func(ctx context.Context, _ []string) error { RunE: runE(func(ctx context.Context, _ []string) error {
return runReport(ctx) return runReport(ctx, stdout)
}), }),
} }
@@ -149,7 +154,7 @@ func newRootCommand(stderr io.Writer) *cobra.Command {
Short: "Read the scan database and print the duplicate-tree report", Short: "Read the scan database and print the duplicate-tree report",
Args: cobra.NoArgs, Args: cobra.NoArgs,
RunE: runE(func(ctx context.Context, _ []string) error { RunE: runE(func(ctx context.Context, _ []string) error {
return runTrees(ctx) return runTrees(ctx, stdout)
}), }),
} }
+86 -81
View File
@@ -44,50 +44,6 @@ func assertNoSidecars(t *testing.T, path string) {
} }
} }
// 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 // brokenDatabase writes a database that opens cleanly and passes the
// schema-version check but has no files table, so the first query // 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 // fails with the database already open: a fatal error on a path that
@@ -160,17 +116,15 @@ func TestRunFatalAfterOpenClosesDatabase(t *testing.T) {
args = append(args, t.TempDir()) args = append(args, t.TempDir())
} }
var stderr bytes.Buffer var stdout, stderr bytes.Buffer
stdout := captureStdout(t) code := run(args, &stdout, &stderr)
code := run(args, &stderr)
if code != exitFatal { if code != exitFatal {
t.Errorf("run(%v) = %d, want %d", args, code, exitFatal) t.Errorf("run(%v) = %d, want %d", args, code, exitFatal)
} }
assertNoSidecars(t, path) assertNoSidecars(t, path)
assertFatalOutput(t, stderr.String(), stdout()) assertFatalOutput(t, stderr.String(), stdout.String())
// Proof that the failure happened after the open: only a // Proof that the failure happened after the open: only a
// query against the opened database can report this. // query against the opened database can report this.
@@ -188,18 +142,16 @@ func TestRunMissingOperandIsFatalNotUsage(t *testing.T) {
// must not dump the usage text. // must not dump the usage text.
t.Setenv(databaseEnv, testDBPath(t)) t.Setenv(databaseEnv, testDBPath(t))
var stderr bytes.Buffer var stdout, stderr bytes.Buffer
stdout := captureStdout(t)
missing := filepath.Join(t.TempDir(), "nope") missing := filepath.Join(t.TempDir(), "nope")
code := run([]string{cmdScan, missing}, &stderr) code := run([]string{cmdScan, missing}, &stdout, &stderr)
if code != exitFatal { if code != exitFatal {
t.Errorf("run(scan %s) = %d, want %d", missing, code, exitFatal) t.Errorf("run(scan %s) = %d, want %d", missing, code, exitFatal)
} }
assertFatalOutput(t, stderr.String(), stdout()) assertFatalOutput(t, stderr.String(), stdout.String())
} }
// assertFatalOutput checks that a fatal error was reported the way // assertFatalOutput checks that a fatal error was reported the way
@@ -243,11 +195,9 @@ func TestRunUsageErrors(t *testing.T) {
// path that does not exist. // path that does not exist.
t.Setenv(databaseEnv, testDBPath(t)) t.Setenv(databaseEnv, testDBPath(t))
var stderr bytes.Buffer var stdout, stderr bytes.Buffer
stdout := captureStdout(t) code := run(tc.args, &stdout, &stderr)
code := run(tc.args, &stderr)
if code != exitUsage { if code != exitUsage {
t.Errorf("run(%v) = %d, want %d", tc.args, code, exitUsage) t.Errorf("run(%v) = %d, want %d", tc.args, code, exitUsage)
} }
@@ -256,7 +206,7 @@ func TestRunUsageErrors(t *testing.T) {
t.Errorf("stderr = %q, want %q", stderr.String(), tc.want) t.Errorf("stderr = %q, want %q", stderr.String(), tc.want)
} }
if got := stdout(); got != "" { if got := stdout.String(); got != "" {
t.Errorf("stdout = %q, want nothing (data only)", got) t.Errorf("stdout = %q, want nothing (data only)", got)
} }
}) })
@@ -265,9 +215,9 @@ func TestRunUsageErrors(t *testing.T) {
// TestRunHelpAndVersionSucceed checks that the two informational flags // TestRunHelpAndVersionSucceed checks that the two informational flags
// exit 0 and keep their human-facing output on stderr. // exit 0 and keep their human-facing output on stderr.
//
//nolint:paralleltest // captureStdout replaces the process-wide os.Stdout
func TestRunHelpAndVersionSucceed(t *testing.T) { func TestRunHelpAndVersionSucceed(t *testing.T) {
t.Parallel()
assertHumanOutput(t, "--help") assertHumanOutput(t, "--help")
assertHumanOutput(t, "--version") assertHumanOutput(t, "--version")
} }
@@ -278,11 +228,9 @@ func TestRunHelpAndVersionSucceed(t *testing.T) {
func assertHumanOutput(t *testing.T, arg string) { func assertHumanOutput(t *testing.T, arg string) {
t.Helper() t.Helper()
var stderr bytes.Buffer var stdout, stderr bytes.Buffer
stdout := captureStdout(t) code := run([]string{arg}, &stdout, &stderr)
code := run([]string{arg}, &stderr)
if code != exitOK { if code != exitOK {
t.Errorf("run(%s) = %d, want %d", arg, code, exitOK) t.Errorf("run(%s) = %d, want %d", arg, code, exitOK)
} }
@@ -291,7 +239,7 @@ func assertHumanOutput(t *testing.T, arg string) {
t.Errorf("run(%s) wrote nothing to stderr", arg) t.Errorf("run(%s) wrote nothing to stderr", arg)
} }
if got := stdout(); got != "" { if got := stdout.String(); got != "" {
t.Errorf("stdout = %q, want nothing (data only)", got) t.Errorf("stdout = %q, want nothing (data only)", got)
} }
} }
@@ -320,17 +268,15 @@ func scanFixture(t *testing.T) []string {
t.Fatal(err) t.Fatal(err)
} }
var stderr bytes.Buffer var stdout, stderr bytes.Buffer
stdout := captureStdout(t) code := run([]string{cmdScan, dir}, &stdout, &stderr)
code := run([]string{cmdScan, dir}, &stderr)
if code != exitOK { if code != exitOK {
t.Fatalf("run(scan) = %d, want %d; stderr: %s", t.Fatalf("run(scan) = %d, want %d; stderr: %s",
code, exitOK, stderr.String()) code, exitOK, stderr.String())
} }
if got := stdout(); got != "" { if got := stdout.String(); got != "" {
t.Errorf("scan stdout = %q, want nothing (data only)", got) t.Errorf("scan stdout = %q, want nothing (data only)", got)
} }
@@ -351,18 +297,16 @@ func TestRunReportSucceeds(t *testing.T) {
dupes := scanFixture(t) dupes := scanFixture(t)
var stderr bytes.Buffer var stdout, stderr bytes.Buffer
stdout := captureStdout(t) code := run([]string{cmdReport}, &stdout, &stderr)
code := run([]string{cmdReport}, &stderr)
if code != exitOK { if code != exitOK {
t.Fatalf("run(report) = %d, want %d; stderr: %s", t.Fatalf("run(report) = %d, want %d; stderr: %s",
code, exitOK, stderr.String()) code, exitOK, stderr.String())
} }
want := "first\tdupe\tsize\n" + dupes[0] + "\t" + dupes[1] + "\t300\n" want := "first\tdupe\tsize\n" + dupes[0] + "\t" + dupes[1] + "\t300\n"
if got := stdout(); got != want { if got := stdout.String(); got != want {
t.Errorf("stdout = %q, want %q", got, want) t.Errorf("stdout = %q, want %q", got, want)
} }
@@ -375,11 +319,9 @@ func TestRunTreesSucceeds(t *testing.T) {
dupes := scanFixture(t) dupes := scanFixture(t)
var stderr bytes.Buffer var stdout, stderr bytes.Buffer
stdout := captureStdout(t) code := run([]string{cmdTrees}, &stdout, &stderr)
code := run([]string{cmdTrees}, &stderr)
if code != exitOK { if code != exitOK {
t.Fatalf("run(trees) = %d, want %d; stderr: %s", t.Fatalf("run(trees) = %d, want %d; stderr: %s",
code, exitOK, stderr.String()) code, exitOK, stderr.String())
@@ -389,9 +331,72 @@ func TestRunTreesSucceeds(t *testing.T) {
// trees of each other. // trees of each other.
want := "first\tdupe\tfiles\tsize\n" + want := "first\tdupe\tfiles\tsize\n" +
filepath.Dir(dupes[0]) + "\t" + filepath.Dir(dupes[1]) + "\t1\t300\n" filepath.Dir(dupes[0]) + "\t" + filepath.Dir(dupes[1]) + "\t1\t300\n"
if got := stdout(); got != want { if got := stdout.String(); got != want {
t.Errorf("stdout = %q, want %q", got, want) t.Errorf("stdout = %q, want %q", got, want)
} }
assertNoSidecars(t, path) assertNoSidecars(t, path)
} }
func TestRunStdoutClosedIsFatal(t *testing.T) {
// README §Error handling: a stdout write failure exits 1, reported
// in one line on stderr.
for _, name := range []string{cmdReport, cmdTrees} {
t.Run(name, func(t *testing.T) {
t.Setenv(databaseEnv, testDBPath(t))
scanFixture(t)
stdout, err := os.Create(filepath.Join(t.TempDir(), "stdout"))
if err != nil {
t.Fatal(err)
}
err = stdout.Close()
if err != nil {
t.Fatal(err)
}
var stderr bytes.Buffer
code := run([]string{name}, stdout, &stderr)
if code != exitFatal {
t.Errorf("run(%s) = %d, want %d", name, code, exitFatal)
}
got := stderr.String()
if !strings.HasPrefix(got, "sfdupes: write stdout: ") ||
!strings.Contains(got, os.ErrClosed.Error()) ||
strings.Count(got, "\n") != 1 {
t.Errorf("stderr = %q, want one line reporting the "+
"failed stdout write", got)
}
})
}
}
// errWriteFailed is the error failingWriter returns.
var errWriteFailed = errors.New("write failed")
// failingWriter is a stdout that fails every write.
type failingWriter struct{}
func (failingWriter) Write([]byte) (int, error) { return 0, errWriteFailed }
func TestStdoutWriteErrorPropagates(t *testing.T) {
t.Setenv(databaseEnv, testDBPath(t))
scanFixture(t)
cases := map[string]func(context.Context, io.Writer) error{
cmdReport: runReport,
cmdTrees: runTrees,
}
for name, fn := range cases {
err := fn(t.Context(), failingWriter{})
if !errors.Is(err, errWriteFailed) {
t.Errorf("%s: error = %v, want %v", name, err, errWriteFailed)
}
}
}
+3 -2
View File
@@ -4,6 +4,7 @@ import (
"bufio" "bufio"
"context" "context"
"fmt" "fmt"
"io"
"os" "os"
"slices" "slices"
"strings" "strings"
@@ -65,7 +66,7 @@ type dupeGroup struct {
// from the database and prints the file-level duplicates report as TSV // from the database and prints the file-level duplicates report as TSV
// on stdout. It never touches the scanned filesystem; its only I/O is // on stdout. It never touches the scanned filesystem; its only I/O is
// the database, stdout, and stderr. // the database, stdout, and stderr.
func runReport(ctx context.Context) error { func runReport(ctx context.Context, stdout io.Writer) error {
recs, err := loadRecords(ctx) recs, err := loadRecords(ctx)
if err != nil { if err != nil {
return err return err
@@ -73,7 +74,7 @@ func runReport(ctx context.Context) error {
dupes := collectDupeGroups(recs) dupes := collectDupeGroups(recs)
out := bufio.NewWriterSize(os.Stdout, ioBufSize) out := bufio.NewWriterSize(stdout, ioBufSize)
_, err = fmt.Fprintln(out, "first\tdupe\tsize") _, err = fmt.Fprintln(out, "first\tdupe\tsize")
if err != nil { if err != nil {
+3 -5
View File
@@ -50,11 +50,9 @@ func seedDatabase(t *testing.T, recs []scanRec) string {
func TestRunReportEscapesPaths(t *testing.T) { func TestRunReportEscapesPaths(t *testing.T) {
t.Setenv(databaseEnv, seedDatabase(t, awkwardPairRecs())) t.Setenv(databaseEnv, seedDatabase(t, awkwardPairRecs()))
var stderr bytes.Buffer var stdout, stderr bytes.Buffer
stdout := captureStdout(t) code := run([]string{cmdReport}, &stdout, &stderr)
code := run([]string{cmdReport}, &stderr)
if code != exitOK { if code != exitOK {
t.Fatalf("run(report) = %d, want %d; stderr: %s", t.Fatalf("run(report) = %d, want %d; stderr: %s",
code, exitOK, stderr.String()) code, exitOK, stderr.String())
@@ -62,7 +60,7 @@ func TestRunReportEscapesPaths(t *testing.T) {
want := "first\tdupe\tsize\n" + want := "first\tdupe\tsize\n" +
`/d/\tone\ntwo\rthree\\four/f` + "\t/d/A/f\t5\n" `/d/\tone\ntwo\rthree\\four/f` + "\t/d/A/f\t5\n"
if got := stdout(); got != want { if got := stdout.String(); got != want {
t.Errorf("stdout = %q, want %q", got, want) t.Errorf("stdout = %q, want %q", got, want)
} }
} }
+2 -1
View File
@@ -7,6 +7,7 @@ import (
"database/sql" "database/sql"
"encoding/hex" "encoding/hex"
"fmt" "fmt"
"io"
"os" "os"
"path/filepath" "path/filepath"
"runtime" "runtime"
@@ -1548,7 +1549,7 @@ func TestScanHashWriteFailureUnwindsPool(t *testing.T) {
code := run([]string{ code := run([]string{
cmdScan, "--workers", strconv.Itoa(hashLeakWorkers), dir, cmdScan, "--workers", strconv.Itoa(hashLeakWorkers), dir,
}, &stderr) }, io.Discard, &stderr)
if code != exitFatal { if code != exitFatal {
t.Fatalf("run(scan) = %d, want %d; stderr: %s", t.Fatalf("run(scan) = %d, want %d; stderr: %s",
code, exitFatal, stderr.String()) code, exitFatal, stderr.String())
+3 -2
View File
@@ -5,6 +5,7 @@ import (
"context" "context"
"crypto/sha256" "crypto/sha256"
"fmt" "fmt"
"io"
"os" "os"
"slices" "slices"
"strconv" "strconv"
@@ -36,7 +37,7 @@ type treeNode struct {
// maximal duplicate-tree groups as TSV on stdout. It never touches the // maximal duplicate-tree groups as TSV on stdout. It never touches the
// scanned filesystem; its only I/O is the database, stdout, and // scanned filesystem; its only I/O is the database, stdout, and
// stderr. // stderr.
func runTrees(ctx context.Context) error { func runTrees(ctx context.Context, stdout io.Writer) error {
recs, err := loadRecords(ctx) recs, err := loadRecords(ctx)
if err != nil { if err != nil {
return err return err
@@ -47,7 +48,7 @@ func runTrees(ctx context.Context) error {
dupes := collectTreeGroups(allDirs, super) dupes := collectTreeGroups(allDirs, super)
out := bufio.NewWriterSize(os.Stdout, ioBufSize) out := bufio.NewWriterSize(stdout, ioBufSize)
_, err = fmt.Fprintln(out, "first\tdupe\tfiles\tsize") _, err = fmt.Fprintln(out, "first\tdupe\tfiles\tsize")
if err != nil { if err != nil {
+3 -5
View File
@@ -108,11 +108,9 @@ func TestBuildHierarchyRootPath(t *testing.T) {
func TestRunTreesEscapesPaths(t *testing.T) { func TestRunTreesEscapesPaths(t *testing.T) {
t.Setenv(databaseEnv, seedDatabase(t, awkwardPairRecs())) t.Setenv(databaseEnv, seedDatabase(t, awkwardPairRecs()))
var stderr bytes.Buffer var stdout, stderr bytes.Buffer
stdout := captureStdout(t) code := run([]string{cmdTrees}, &stdout, &stderr)
code := run([]string{cmdTrees}, &stderr)
if code != exitOK { if code != exitOK {
t.Fatalf("run(trees) = %d, want %d; stderr: %s", t.Fatalf("run(trees) = %d, want %d; stderr: %s",
code, exitOK, stderr.String()) code, exitOK, stderr.String())
@@ -120,7 +118,7 @@ func TestRunTreesEscapesPaths(t *testing.T) {
want := "first\tdupe\tfiles\tsize\n" + want := "first\tdupe\tfiles\tsize\n" +
`/d/\tone\ntwo\rthree\\four` + "\t/d/A\t1\t5\n" `/d/\tone\ntwo\rthree\\four` + "\t/d/A\t1\t5\n"
if got := stdout(); got != want { if got := stdout.String(); got != want {
t.Errorf("stdout = %q, want %q", got, want) t.Errorf("stdout = %q, want %q", got, want)
} }
} }