|
| 1 | +# Plan |
| 2 | + |
| 3 | +## Goal |
| 4 | + |
| 5 | +Ensure every command-line option in every non-hook CLI class has test coverage through `run()`, and refactor |
| 6 | +existing tests that call internal methods directly to go through `run()` instead. |
| 7 | + |
| 8 | +This follows the "Test Through run(), Not Internal Methods" convention added to `.claude/rules/java.md` in |
| 9 | +issue `2.1-thin-main-methods-and-add-run-tests`. |
| 10 | + |
| 11 | +## Background |
| 12 | + |
| 13 | +Issue `2.1-thin-main-methods-and-add-run-tests` added `run()` methods and `*MainTest.java` files for all |
| 14 | +non-hook CLI classes. However: |
| 15 | + |
| 16 | +1. Most `*MainTest.java` files only have null-check tests and 1-2 basic error-path tests |
| 17 | +2. Many command-line options (flags, subcommands, validation paths) lack test coverage through `run()` |
| 18 | +3. Existing tests in some files call internal methods (e.g., `getOutput()`, `getConcernBox()`) directly |
| 19 | + instead of going through `run()` |
| 20 | + |
| 21 | +## Scope |
| 22 | + |
| 23 | +### Part 1: Add missing argument-parsing tests through run() |
| 24 | + |
| 25 | +For each CLI class with `run()`, ensure every command-line option and error path has a test: |
| 26 | + |
| 27 | +- **Valid flag combinations** — verify `run()` returns 0 and produces expected output |
| 28 | +- **Missing required flags** — verify `run()` returns 1 |
| 29 | +- **Invalid flag values** — verify `run()` returns 1 |
| 30 | +- **Unknown flags** — verify `run()` returns 1 (where applicable) |
| 31 | +- **Help/usage output** — verify `run()` returns 0 and output contains "Usage" |
| 32 | + |
| 33 | +Priority files (have substantial arg parsing with gaps): |
| 34 | +- `EmpiricalTestRunner` — `--config`, `--trials`, `--model`, `--cwd`, `--output`, `--baseline` |
| 35 | +- `GetAddOutput` — `--type`, `--name`, `--version`, `--issue-type`, `--dependencies`, `--parent`, `--path` |
| 36 | +- `GetCheckpointOutput` — `--type`, `--issue-name`, `--tokens`, `--percent`, `--branch`, `--iteration`, `--total` |
| 37 | +- `GetCleanupOutput` — `--project-dir`, `--phase` |
| 38 | +- `GetIssueCompleteOutput` — `--issue-name`, `--target-branch`, `--scope-complete` |
| 39 | +- `GetNextIssueOutput` — `--completed-issue`, `--target-branch`, `--session-id`, `--project-dir`, `--exclude-pattern` |
| 40 | +- `SessionAnalyzer` — subcommands: `analyze`, `search`, `errors`, `file-history` |
| 41 | +- `MarkdownWrapper` — `--width`, positional file arg, stdin mode |
| 42 | + |
| 43 | +### Part 2: Refactor existing tests to use run() |
| 44 | + |
| 45 | +Find tests that call internal methods directly (e.g., `getOutput()`, `analyzeSession()`, `getConcernBox()`) |
| 46 | +and refactor them to go through `run()` with appropriate command-line arguments instead. |
| 47 | + |
| 48 | +## Post-conditions |
| 49 | + |
| 50 | +- [ ] Every `*MainTest.java` has tests for each command-line flag/option accepted by its `run()` method |
| 51 | +- [ ] No test calls an internal method when the same functionality can be exercised through `run()` |
| 52 | +- [ ] All tests pass (`mvn -f client/pom.xml verify`) |
| 53 | +- [ ] Error paths return non-zero exit codes (verified via `requireThat(result, "result").isEqualTo(1)`) |
| 54 | +- [ ] Happy paths return zero exit codes (verified via `requireThat(result, "result").isEqualTo(0)`) |
0 commit comments