Author SHA1 Message Date
clawbot 1317d66589 Stop script/lint writing an image it never uses (closes #48)
check / check (push) Waiting to run
script/lint builds Dockerfile.lint only for the exit status, but every
run exported the result as an image: seconds spent exporting, and one
untagged image left behind each time. It now builds with
--output=type=cacheonly, so nothing is exported. CHECK_EPOCH still
changes on every run, so the gate steps still run each time; the build
cache is kept as before.

Model: opus-5-5
2026-10-04 18:01:29 +02:00
clawbot 0b7078301d Test the remaining CLI cases (closes #16)
check / check (push) Waiting to run
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
2026-10-04 17:47:26 +02:00
5 changed files with 164 additions and 39 deletions
+3 -1
View File
@@ -831,7 +831,9 @@ and may be invoked directly. The provided entrypoints are:
copies the repository into the digest-pinned copies the repository into the digest-pinned
`golangci/golangci-lint` image and runs `golangci/golangci-lint` image and runs
`golangci-lint config verify` and `golangci-lint run` as build `golangci-lint config verify` and `golangci-lint run` as build
steps, so a successful build is a clean lint. The linter is never steps, so a successful build is a clean lint. That exit status is
all it produces, so it runs with `--output=type=cacheonly` and
writes no image; a run leaves only build cache. The linter is never
run on the host, which makes a working `docker` the one run on the host, which makes a working `docker` the one
prerequisite for linting — and therefore for `make check` and the prerequisite for linting — and therefore for `make check` and the
pre-commit hook. Offline machines: the gate steps themselves make pre-commit hook. Offline machines: the gate steps themselves make
+7
View File
@@ -29,6 +29,13 @@
# Completed Steps # Completed Steps
- `script/lint` writes no image, so a run no longer leaves an untagged
one behind (2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/48)
- 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 - `.golangci.yml` replaced with the current canonical copy, which uses
`gomodguard_v2`, so lint no longer prints a deprecation warning `gomodguard_v2`, so lint no longer prints a deprecation warning
(2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/26) (2026-10-04, https://git.eeqj.de/sneak/sfdupes/issues/26)
+15 -5
View File
@@ -344,13 +344,14 @@ func storedPaths(t *testing.T, path string) []string {
// TestRunScanInterrupted calls the scan entrypoint with a context that // TestRunScanInterrupted calls the scan entrypoint with a context that
// is already cancelled, as when a signal arrives at once. It must return // is already cancelled, as when a signal arrives at once. It must return
// errInterrupted promptly with its one line on stderr, leave the // errInterrupted promptly with its one line on stderr and nothing on
// database valid and as it was, and leave nothing in the way of the // stdout, leave the database valid and as it was, and leave nothing in
// next scan, which must bring the database up to date. // the way of the next scan, which must bring the database up to date.
func TestRunScanInterrupted(t *testing.T) { func TestRunScanInterrupted(t *testing.T) {
path := testDBPath(t) path := testDBPath(t)
t.Setenv(databaseEnv, path) t.Setenv(databaseEnv, path)
stdout := captureStdout(t)
stderr := captureStderr(t) stderr := captureStderr(t)
dir := buildSmokeTree(t) dir := buildSmokeTree(t)
@@ -393,6 +394,10 @@ func TestRunScanInterrupted(t *testing.T) {
t.Errorf("stderr = %q, want %q", got, want) t.Errorf("stderr = %q, want %q", got, want)
} }
if got := stdout(); got != "" {
t.Errorf("stdout = %q, want nothing (data only)", got)
}
assertNoSidecars(t, path) assertNoSidecars(t, path)
if got := storedPaths(t, path); !slices.Equal(got, before) { 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 // TestRunScanInterruptedMidHash interrupts the scan entrypoint part-way
// through its hash phase, after the database is open. It must return // through its hash phase, after the database is open. It must return
// errInterrupted, release the lock, end stderr with its line counting // errInterrupted, release the lock, end stderr with its line counting
// every file the walk reached, close the database out of WAL mode, and // every file the walk reached, write nothing to stdout, close the
// keep the records it hashed. // database out of WAL mode, and keep the records it hashed.
func TestRunScanInterruptedMidHash(t *testing.T) { func TestRunScanInterruptedMidHash(t *testing.T) {
path := testDBPath(t) path := testDBPath(t)
t.Setenv(databaseEnv, path) t.Setenv(databaseEnv, path)
stdout := captureStdout(t)
stderr := captureStderr(t) stderr := captureStderr(t)
dir := buildWalkCancelTree(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) 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) assertNoSidecars(t, path)
db, err := openReportDatabase(t.Context(), path) db, err := openReportDatabase(t.Context(), path)
+134 -33
View File
@@ -85,21 +85,41 @@ func makeReadOnly(t *testing.T, path string) {
// captureStderr redirects os.Stderr to a file for the rest of the test // captureStderr redirects os.Stderr to a file for the rest of the test
// and returns a function reading back everything written to it. scan // and returns a function reading back everything written to it. scan
// writes its warnings and summary straight to os.Stderr, not to the // writes its warnings and summary, and report and trees their
// stderr writer run is given. // summaries, straight to os.Stderr, not to the stderr writer run is
// given.
func captureStderr(t *testing.T) func() string { func captureStderr(t *testing.T) func() string {
t.Helper() 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 { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
saved := os.Stderr saved := *std
os.Stderr = f *std = f
t.Cleanup(func() { t.Cleanup(func() {
os.Stderr = saved *std = saved
_ = f.Close() _ = f.Close()
}) })
@@ -184,30 +204,33 @@ func TestRunFatalAfterOpenClosesDatabase(t *testing.T) {
// sidecar check is evidence of the close only for scan: report and // 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 // trees only read a database that is out of WAL mode, which leaves
// nothing on disk whether they close it or not. // nothing on disk whether they close it or not.
cases := map[string][]string{ //
cmdScan: {cmdScan}, // The subcommands come from the command tree, so a new one is
cmdReport: {cmdReport}, // checked too: one wired with a bare RunE instead of runE reports
cmdTrees: {cmdTrees}, // 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) { t.Run(name, func(t *testing.T) {
path := brokenDatabase(t) path := brokenDatabase(t)
t.Setenv(databaseEnv, path) t.Setenv(databaseEnv, path)
args := []string{name}
if name == cmdScan { if name == cmdScan {
args = append(args, t.TempDir()) 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 { 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.String()) assertFatalOutput(t, stderr.String(), stdout())
// 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.
@@ -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 // README §Error handling: a PATH operand that does not exist is a
// fatal error (1), not a usage error (2) — and a runtime failure // fatal error (1), not a usage error (2) — and a runtime failure
// must not dump the usage text. // must not dump the usage text.
t.Setenv(databaseEnv, testDBPath(t)) t.Setenv(databaseEnv, testDBPath(t))
var stdout, stderr bytes.Buffer stdout := captureStdout(t)
var stderr bytes.Buffer
missing := filepath.Join(t.TempDir(), "nope") missing := filepath.Join(t.TempDir(), "nope")
code := run([]string{cmdScan, missing}, &stdout, &stderr) code := run([]string{cmdScan, missing}, os.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.String()) assertFatalOutput(t, stderr.String(), stdout())
} }
// assertFatalOutput checks that a fatal error was reported the way // 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) { func TestRunUsageErrors(t *testing.T) {
// Usage errors keep exiting 2 with cobra's own report on stderr. // Usage errors keep exiting 2 with cobra's own report on stderr.
cases := map[string]struct { cases := map[string]struct {
@@ -435,17 +488,16 @@ func scanFixture(t *testing.T) []string {
func scanOK(t *testing.T, operands ...string) string { func scanOK(t *testing.T, operands ...string) string {
t.Helper() t.Helper()
var stdout bytes.Buffer stdout := captureStdout(t)
stderr := captureStderr(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 { if code != exitOK {
t.Fatalf("run(scan %q) = %d, want %d; stderr: %s", t.Fatalf("run(scan %q) = %d, want %d; stderr: %s",
operands, code, exitOK, stderr()) operands, code, exitOK, stderr())
} }
if got := stdout.String(); got != "" { if got := stdout(); got != "" {
t.Errorf("scan stdout = %q, want nothing (data only)", 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) { 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) path := testDBPath(t)
t.Setenv(databaseEnv, path) 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) assertNoSidecars(t, path)
} }
@@ -563,12 +646,14 @@ func TestRunReportSucceeds(t *testing.T) {
dupes := scanFixture(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 { 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())
} }
want := "first\tdupe\tsize\n" + dupes[0] + "\t" + dupes[1] + "\t300\n" 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) 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) assertNoSidecars(t, path)
} }
@@ -585,12 +676,14 @@ func TestRunTreesSucceeds(t *testing.T) {
dupes := scanFixture(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 { 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())
} }
// The two directories holding the duplicate pair are duplicate // 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) 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) assertNoSidecars(t, path)
} }
@@ -730,9 +829,11 @@ func TestRunSecondScanFails(t *testing.T) {
holdScanLock(t, path) 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 { if code != exitFatal {
t.Errorf("run(scan) = %d, want %d", 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) 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) t.Errorf("stdout = %q, want nothing (data only)", got)
} }
+5
View File
@@ -14,6 +14,10 @@
# 0 in well under a second having run no linter. The PID is in the value # 0 in well under a second having run no linter. The PID is in the value
# as well as the epoch because two lint runs land inside the same second # as well as the epoch because two lint runs land inside the same second
# easily, and `date +%s` alone would cache the second one. # easily, and `date +%s` alone would cache the second one.
#
# The result is the build's exit status and the image is never used, so
# --output=type=cacheonly writes none. Without it every run spends
# seconds exporting an image and leaves it behind untagged.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
@@ -22,6 +26,7 @@ main() {
cd "$ROOT" cd "$ROOT"
docker build \ docker build \
--build-arg CHECK_EPOCH="$(date +%s)-$$" \ --build-arg CHECK_EPOCH="$(date +%s)-$$" \
--output=type=cacheonly \
-f Dockerfile.lint \ -f Dockerfile.lint \
. .
} }