There is no main_test.go. All 43 existing test functions call internal helpers — collectDupeGroups, collectTreeGroups, syncScan, hashRuns — and none of them ever calls runScan, runReport, runTrees, or main. The entire user-visible contract is untested.
README-mandated behaviours with no coverage at all:
Exit 2: no subcommand, unknown subcommand, scan with no PATH operand, report or trees with a positional argument (README §Error handling).
Exit 1: a PATH operand that does not exist, a missing database for report and trees.
Exit 0 with warnings after per-file skips.
"scan writes nothing to stdout" (README §scan mode) — nothing asserts stdout is empty.
The header line and exact row format of both reports, and both stderr summary lines.
These are also the behaviours most likely to regress silently, because nothing about them is exercised by the internal-helper tests.
Definition of done
A table-driven main_test.go builds the binary once via TestMain (or drives the cobra root command in-process, once #4 makes that possible without os.Exit) and, for each case, asserts the exit code, the exact stdout bytes, and the exact stderr line where the README specifies one.
Every case listed above is covered.
scan cases additionally assert stdout is empty.
The suite still finishes well inside the 20-second budget and the 30-second timeout in script/test.
make check green.
Best done after #4, which removes the os.Exit calls that currently make these paths untestable in-process.
There is no `main_test.go`. All 43 existing test functions call internal helpers — `collectDupeGroups`, `collectTreeGroups`, `syncScan`, `hashRuns` — and none of them ever calls `runScan`, `runReport`, `runTrees`, or `main`. The entire user-visible contract is untested.
README-mandated behaviours with no coverage at all:
- Exit 2: no subcommand, unknown subcommand, `scan` with no `PATH` operand, `report` or `trees` with a positional argument (README §Error handling).
- Exit 1: a `PATH` operand that does not exist, a missing database for `report` and `trees`.
- Exit 0 with warnings after per-file skips.
- "`scan` writes nothing to stdout" (README §scan mode) — nothing asserts stdout is empty.
- The header line and exact row format of both reports, and both stderr summary lines.
These are also the behaviours most likely to regress silently, because nothing about them is exercised by the internal-helper tests.
## Definition of done
1. A table-driven `main_test.go` builds the binary once via `TestMain` (or drives the cobra root command in-process, once #4 makes that possible without `os.Exit`) and, for each case, asserts the exit code, the exact stdout bytes, and the exact stderr line where the README specifies one.
2. Every case listed above is covered.
3. `scan` cases additionally assert stdout is empty.
4. The suite still finishes well inside the 20-second budget and the 30-second timeout in `script/test`.
5. `make check` green.
Best done after #4, which removes the `os.Exit` calls that currently make these paths untestable in-process.
clawbot
added this to the 1.0.0 milestone 2026-08-09 03:46:13 +02:00
Three additions to this issue's scope, all surfaced by the independent review of #4 (merged as 2a055c0). They belong here rather than in their own issues because each is about making the CLI surface properly testable.
1. Thread the output writers through the subcommands.run(args []string, stderr io.Writer) now exists as a single in-process entry point, but it only routes cobra's own output. The subcommands still write to the package-global os.Stdout and os.Stderr, so a test that wants to assert on program output has to monkey-patch os.Stdout. The concrete cost today: TestRunScanSucceedsDespiteWarnings cannot assert that the warning it is named for was actually emitted. Thread the writers down so tests can capture output by passing it in.
2. Add a guard test for runE. The exit-code classification added in #4 works because each subcommand is wired through a runE adapter that sets SilenceUsage/SilenceErrors and wraps failures in fatalError. That is opt-in: a future subcommand wired with a bare RunE would have its runtime failures misclassified as exit 2 with a usage dump — precisely the regression #4 existed to prevent, reintroduced silently. Walk root.Commands() in a test and assert every command goes through the adapter.
3. Rename TestRunMissingOperandIsFatalNotUsage. It covers a nonexistentPATH operand, which is fatal (exit 1). "Missing operand" is README's phrase for scan invoked with no operand at all, which is a usage error (exit 2) and is covered by a different test. Suggested: TestRunNonexistentPathIsFatalNotUsage.
Item 1 in particular should be done before the rest of this issue's table-driven tests are written, since it determines whether they can assert on output directly or have to go through the filesystem.
Three additions to this issue's scope, all surfaced by the independent review of #4 (merged as `2a055c0`). They belong here rather than in their own issues because each is about making the CLI surface properly testable.
**1. Thread the output writers through the subcommands.** `run(args []string, stderr io.Writer)` now exists as a single in-process entry point, but it only routes cobra's own output. The subcommands still write to the package-global `os.Stdout` and `os.Stderr`, so a test that wants to assert on program output has to monkey-patch `os.Stdout`. The concrete cost today: `TestRunScanSucceedsDespiteWarnings` cannot assert that the warning it is named for was actually emitted. Thread the writers down so tests can capture output by passing it in.
**2. Add a guard test for `runE`.** The exit-code classification added in #4 works because each subcommand is wired through a `runE` adapter that sets `SilenceUsage`/`SilenceErrors` and wraps failures in `fatalError`. That is opt-in: a future subcommand wired with a bare `RunE` would have its runtime failures misclassified as exit 2 with a usage dump — precisely the regression #4 existed to prevent, reintroduced silently. Walk `root.Commands()` in a test and assert every command goes through the adapter.
**3. Rename `TestRunMissingOperandIsFatalNotUsage`.** It covers a *nonexistent* `PATH` operand, which is fatal (exit 1). "Missing operand" is README's phrase for `scan` invoked with no operand at all, which is a usage error (exit 2) and is covered by a different test. Suggested: `TestRunNonexistentPathIsFatalNotUsage`.
Item 1 in particular should be done before the rest of this issue's table-driven tests are written, since it determines whether they can assert on output directly or have to go through the filesystem.
Plan (implementer's brief). Much of this exists now: main_test.go drives run(args, stdout, stderr) in-process, and many exit-code and output tests were added with later issues. First list every case in the definition of done and in the 2026-08-09 comment, check each against the existing tests, and write only what is missing; say in the PR which cases were already covered and by which test. The runE guard test (comment item 2) and the test rename (item 3, if that test still exists) are in scope. Threading stderr through the subcommands (item 1) only where a missing case cannot be asserted otherwise; keep it plain. Suite stays inside the time budget.
Model: opus-5-5
Plan (implementer's brief). Much of this exists now: `main_test.go` drives `run(args, stdout, stderr)` in-process, and many exit-code and output tests were added with later issues. First list every case in the definition of done and in the 2026-08-09 comment, check each against the existing tests, and write only what is missing; say in the PR which cases were already covered and by which test. The `runE` guard test (comment item 2) and the test rename (item 3, if that test still exists) are in scope. Threading stderr through the subcommands (item 1) only where a missing case cannot be asserted otherwise; keep it plain. Suite stays inside the time budget.
Model: opus-5-5
clawbot
self-assigned this 2026-10-04 12:02:37 +02:00
#90 adds the cases that were still missing, plus the runE guard and the test rename from the 2026-08-09 comment. Its description lists which cases were already covered and by which test.
Model: opus-5-5
https://git.eeqj.de/sneak/sfdupes/pulls/90 adds the cases that were still missing, plus the `runE` guard and the test rename from the 2026-08-09 comment. Its description lists which cases were already covered and by which test.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
There is no
main_test.go. All 43 existing test functions call internal helpers —collectDupeGroups,collectTreeGroups,syncScan,hashRuns— and none of them ever callsrunScan,runReport,runTrees, ormain. The entire user-visible contract is untested.README-mandated behaviours with no coverage at all:
scanwith noPATHoperand,reportortreeswith a positional argument (README §Error handling).PATHoperand that does not exist, a missing database forreportandtrees.scanwrites nothing to stdout" (README §scan mode) — nothing asserts stdout is empty.These are also the behaviours most likely to regress silently, because nothing about them is exercised by the internal-helper tests.
Definition of done
main_test.gobuilds the binary once viaTestMain(or drives the cobra root command in-process, once #4 makes that possible withoutos.Exit) and, for each case, asserts the exit code, the exact stdout bytes, and the exact stderr line where the README specifies one.scancases additionally assert stdout is empty.script/test.make checkgreen.Best done after #4, which removes the
os.Exitcalls that currently make these paths untestable in-process.Three additions to this issue's scope, all surfaced by the independent review of #4 (merged as
2a055c0). They belong here rather than in their own issues because each is about making the CLI surface properly testable.1. Thread the output writers through the subcommands.
run(args []string, stderr io.Writer)now exists as a single in-process entry point, but it only routes cobra's own output. The subcommands still write to the package-globalos.Stdoutandos.Stderr, so a test that wants to assert on program output has to monkey-patchos.Stdout. The concrete cost today:TestRunScanSucceedsDespiteWarningscannot assert that the warning it is named for was actually emitted. Thread the writers down so tests can capture output by passing it in.2. Add a guard test for
runE. The exit-code classification added in #4 works because each subcommand is wired through arunEadapter that setsSilenceUsage/SilenceErrorsand wraps failures infatalError. That is opt-in: a future subcommand wired with a bareRunEwould have its runtime failures misclassified as exit 2 with a usage dump — precisely the regression #4 existed to prevent, reintroduced silently. Walkroot.Commands()in a test and assert every command goes through the adapter.3. Rename
TestRunMissingOperandIsFatalNotUsage. It covers a nonexistentPATHoperand, which is fatal (exit 1). "Missing operand" is README's phrase forscaninvoked with no operand at all, which is a usage error (exit 2) and is covered by a different test. Suggested:TestRunNonexistentPathIsFatalNotUsage.Item 1 in particular should be done before the rest of this issue's table-driven tests are written, since it determines whether they can assert on output directly or have to go through the filesystem.
Plan (implementer's brief). Much of this exists now:
main_test.godrivesrun(args, stdout, stderr)in-process, and many exit-code and output tests were added with later issues. First list every case in the definition of done and in the 2026-08-09 comment, check each against the existing tests, and write only what is missing; say in the PR which cases were already covered and by which test. TherunEguard test (comment item 2) and the test rename (item 3, if that test still exists) are in scope. Threading stderr through the subcommands (item 1) only where a missing case cannot be asserted otherwise; keep it plain. Suite stays inside the time budget.Model: opus-5-5
#90 adds the cases that were still missing, plus the
runEguard and the test rename from the 2026-08-09 comment. Its description lists which cases were already covered and by which test.Model: opus-5-5