wip: baseline before wholefile tests

This commit is contained in:
2026-07-02 09:20:06 -05:00
parent d2df2ec3d4
commit 27681bc56a
@@ -0,0 +1,654 @@
# Whole-File Single-Line Comment Stripping Tests — Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Add whole-file golden tests that run real source files through the file read→transform→write path and assert the resulting bytes, locking current single-line comment-stripping behavior before the multi-line block refactor.
**Architecture:** A tiny prod refactor injects `cfg *config.Config` into the two file-mutating processor methods (so tests are hermetic and don't read `~/.config/.shush.toml`). A new golden-file test driver copies each `testdata/<name>.<ext>` input to a temp dir, runs the real mutate path, and byte-compares the result against `testdata/<name>.<ext>.golden`.
**Tech Stack:** Go 1.24, standard `testing`, golden files under `internal/processor/testdata/`.
## Global Constraints
- Go module path: `github.com/carlosarraes/shush` (unchanged — do NOT rewrite import paths).
- Go version: 1.24.
- Version control: **jj only, never git.** Repo is colocated (`jj git init --colocate` already run by orchestrator). Commit with `jj describe -m "..."` then `jj new`. Never run `git commit`/`git add`.
- Gate before every commit: `make check` (runs `fmt` + `vet` + `test`) must be clean — zero failures, zero vet warnings.
- **Scope: single-line behavior ONLY.** Do NOT add multi-line block-comment fixtures — those are broken and handled in a later plan. Any multi-line block case is out of scope here.
- Goldens are **hand-specified expected values** from this plan, not machine-captured. Create the golden file with the exact bytes given, then run the test WITHOUT `-update`. If a case fails, STOP and report it (it means real behavior differs from the plan) — do NOT blindly run `-update` to paper over it.
- Fixture file extension drives language detection (`DetectLanguage` uses the extension) — name fixtures with the real source extension.
- Writer behavior the goldens must match: output is written line-by-line via `fmt.Fprintln`, so every kept line ends in `\n` and the file always ends in `\n`; comment-only lines are dropped entirely (unless `--preserve-lines`); inline results are right-trimmed of trailing spaces/tabs. A file with NO changes is left byte-identical (not rewritten).
---
## File Structure
- `internal/processor/processor.go` — MODIFY: load cfg once in `Process`, thread through `processDirectory`/`processFile` into `processFileInMemory` (new `cfg` param).
- `internal/processor/git_processor.go` — MODIFY: load cfg once in `processGitChanges`, pass into `processFileWithLineRanges` (new `cfg` param).
- `internal/processor/wholefile_test.go` — CREATE: golden-file test driver + case table.
- `internal/processor/testdata/*` — CREATE: input/`.golden` fixture pairs.
---
## Task 1: Inject `cfg` into file-mutating methods (prod refactor, no behavior change)
**Files:**
- Modify: `internal/processor/processor.go`
- Modify: `internal/processor/git_processor.go`
**Interfaces:**
- Produces (later tasks + tests rely on these exact signatures):
- `func (p *Processor) processFileInMemory(filename string, language types.Language, cfg *config.Config) error`
- `func (p *Processor) processFileWithLineRanges(filename string, lineRanges []git.LineRange, cfg *config.Config) error`
**Safety net:** This is a pure refactor. The existing test suite + `go build` are the verification — behavior must not change.
- [ ] **Step 1: Confirm baseline is green**
Run: `cd /home/mroberts/tmp/review/shush-fork && go build ./... && go test ./...`
Expected: build clean; all packages `ok` or `[no test files]`.
- [ ] **Step 2: `processor.go` — remove internal load from `processFileInMemory`, add `cfg` param**
In `internal/processor/processor.go`, change the `processFileInMemory` signature and delete its internal `config.Load()` block. Replace the top of the function:
```go
func (p *Processor) processFileInMemory(filename string, language types.Language, cfg *config.Config) error {
file, err := os.Open(filename)
if err != nil {
return err
}
defer file.Close()
```
(Delete the former `cfg, _, err := config.Load()` / verbose-warning / `cfg = config.Default()` lines that used to start this function.)
- [ ] **Step 3: `processor.go` — thread `cfg` through `Process`, `processDirectory`, `processFile`**
Change `Process()` so it loads cfg once for the non-git path and passes it down. Replace the body from the `os.Stat` line onward:
```go
cfg, _, err := config.Load()
if err != nil {
if p.cli.Verbose {
fmt.Printf("Warning: failed to load config, using defaults: %v\n", err)
}
cfg = config.Default()
}
info, err := os.Stat(p.cli.Path)
if os.IsNotExist(err) {
return fmt.Errorf("path not found: %s", p.cli.Path)
}
if err != nil {
return err
}
if info.IsDir() {
return p.processDirectory(p.cli.Path, cfg)
}
if IsIgnored(p.cli.Path) {
fmt.Printf("File %s is in .shushignore, ignoring\n", p.cli.Path)
return nil
}
return p.processFile(p.cli.Path, cfg)
}
```
Change `processDirectory` to accept and forward cfg — signature `func (p *Processor) processDirectory(dirPath string, cfg *config.Config) error`, and its file loop call becomes `if err := p.processFile(file, cfg); err != nil {`.
Change `processFile` — signature `func (p *Processor) processFile(filename string, cfg *config.Config) error`, and its non-dry-run tail becomes `return p.processFileInMemory(filename, language, cfg)`. (Leave the `p.cli.DryRun` → `p.showPreview(...)` branch untouched; `showPreview` keeps its own `config.Load()`.)
- [ ] **Step 4: `git_processor.go` — remove internal load from `processFileWithLineRanges`, add `cfg` param**
In `internal/processor/git_processor.go`, change the signature and delete the internal load. Replace the top of the function:
```go
func (p *Processor) processFileWithLineRanges(filename string, lineRanges []git.LineRange, cfg *config.Config) error {
language, err := DetectLanguage(filename)
if err != nil {
return err
}
if p.cli.Verbose {
```
(Delete the former `cfg, _, err := config.Load()` / verbose-warning block that sat between `DetectLanguage` and the verbose section.)
- [ ] **Step 5: `git_processor.go` — load cfg once in `processGitChanges`, pass it in**
In `processGitChanges`, after the `supportedChanges`/verbose block and before `totals := &GitTotals{}`, add:
```go
cfg, _, err := config.Load()
if err != nil {
if p.cli.Verbose {
fmt.Printf("Warning: failed to load config, using defaults: %v\n", err)
}
cfg = config.Default()
}
totals := &GitTotals{}
```
Then change the non-dry-run call inside the loop to pass cfg: `if err := p.processFileWithLineRanges(change.Path, change.LineRanges, cfg); err != nil {`. (Leave the `p.cli.DryRun` → `showGitPreviewWithTotals` branch untouched; it keeps its own `config.Load()`.)
Note: `processGitChanges` already declares `err` earlier via the `switch` block, so use `=` (not `:=`) if the compiler reports `err` redeclared — adjust to `cfg, _, cfgErr := config.Load()` and reference `cfgErr` if needed to satisfy the compiler.
- [ ] **Step 6: Build + run existing tests (must still pass, unchanged)**
Run: `go build ./... && go test ./...`
Expected: build clean; `internal/processor`, `internal/git`, `cmd/shush` all `ok`. No behavior change.
- [ ] **Step 7: Commit (jj)**
```bash
jj describe -m "refactor(processor): inject cfg into file-mutating methods"
jj new
```
---
## Task 2: Golden-file test driver + seed cases
**Files:**
- Create: `internal/processor/wholefile_test.go`
- Create: `internal/processor/testdata/py_comment_only.py`
- Create: `internal/processor/testdata/py_comment_only.py.golden`
- Create: `internal/processor/testdata/py_inline.py`
- Create: `internal/processor/testdata/py_inline.py.golden`
**Interfaces:**
- Consumes: `processFileInMemory(filename, language, cfg)`, `processFileWithLineRanges(filename, ranges, cfg)` from Task 1.
- Produces (later tasks append entries to this table): the `wholeFileCase` struct and `runWholeFileCase(t, tc)` helper below; `TestWholeFile`'s `cases` slice.
- [ ] **Step 1: Write the driver + seed table (the failing test)**
Create `internal/processor/wholefile_test.go`:
```go
package processor
import (
"bytes"
"flag"
"os"
"path/filepath"
"testing"
"github.com/carlosarraes/shush/internal/config"
"github.com/carlosarraes/shush/internal/git"
"github.com/carlosarraes/shush/internal/types"
)
var update = flag.Bool("update", false, "update golden files")
type wholeFileCase struct {
name string
file string // fixture under testdata/, e.g. "py_inline.py"
cli types.CLI
cfg *config.Config // nil => config.Default()
gitMode bool // true => processFileWithLineRanges (surgical git path)
ranges []git.LineRange // used only when gitMode; nil => whole file
wantBackup bool
}
func runWholeFileCase(t *testing.T, tc wholeFileCase) {
t.Helper()
src := filepath.Join("testdata", tc.file)
in, err := os.ReadFile(src)
if err != nil {
t.Fatalf("read input %s: %v", src, err)
}
tmp := t.TempDir()
work := filepath.Join(tmp, tc.file)
if err := os.WriteFile(work, in, 0644); err != nil {
t.Fatalf("write work copy: %v", err)
}
cfg := tc.cfg
if cfg == nil {
cfg = config.Default()
}
p := &Processor{cli: tc.cli}
if tc.gitMode {
err = p.processFileWithLineRanges(work, tc.ranges, cfg)
} else {
lang, derr := DetectLanguage(work)
if derr != nil {
t.Fatalf("detect language for %s: %v", work, derr)
}
err = p.processFileInMemory(work, lang, cfg)
}
if err != nil {
t.Fatalf("process: %v", err)
}
got, err := os.ReadFile(work)
if err != nil {
t.Fatalf("read result: %v", err)
}
golden := src + ".golden"
if *update {
if err := os.WriteFile(golden, got, 0644); err != nil {
t.Fatalf("update golden: %v", err)
}
}
want, err := os.ReadFile(golden)
if err != nil {
t.Fatalf("read golden %s: %v", golden, err)
}
if !bytes.Equal(got, want) {
t.Errorf("output mismatch:\n--- got ---\n%q\n--- want ---\n%q", got, want)
}
if tc.wantBackup {
bakData, err := os.ReadFile(work + ".bak")
if err != nil {
t.Fatalf("read backup: %v", err)
}
if !bytes.Equal(bakData, in) {
t.Errorf("backup mismatch:\n--- got ---\n%q\n--- want ---\n%q", bakData, in)
}
}
}
func TestWholeFile(t *testing.T) {
cases := []wholeFileCase{
{name: "python comment-only line removed", file: "py_comment_only.py"},
{name: "python inline comment stripped", file: "py_inline.py"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
runWholeFileCase(t, tc)
})
}
}
```
- [ ] **Step 2: Run to verify it fails (no fixtures yet)**
Run: `go test ./internal/processor -run TestWholeFile -v`
Expected: FAIL — `read input testdata/py_comment_only.py: ... no such file`.
- [ ] **Step 3: Create the seed fixtures with exact bytes**
`testdata/py_comment_only.py` (3 lines, trailing newline):
```
# header comment
x = 1
print(x)
```
`testdata/py_comment_only.py.golden` (comment-only line dropped):
```
x = 1
print(x)
```
`testdata/py_inline.py`:
```
x = 1 # set x
print(x) # show
```
`testdata/py_inline.py.golden` (inline stripped + right-trimmed):
```
x = 1
print(x)
```
- [ ] **Step 4: Run to verify it passes**
Run: `go test ./internal/processor -run TestWholeFile -v`
Expected: PASS — both subtests green.
- [ ] **Step 5: `make check` then commit (jj)**
Run: `make check`
Expected: fmt/vet/test clean.
```bash
jj describe -m "test(processor): whole-file golden driver + seed python cases"
jj new
```
---
## Task 3: Line-comment, string-protection, and structure cases
**Files:**
- Modify: `internal/processor/wholefile_test.go` (append entries to `cases`)
- Create fixtures + `.golden` for: `js_string_marker.js`, `go_nocomment.go`, `py_blanks.py`, `js_escaped.js`, `lua_string.lua`
**Interfaces:** Consumes `runWholeFileCase`/`wholeFileCase` from Task 2.
- [ ] **Step 1: Append table entries (failing test — fixtures missing)**
Add to the `cases` slice in `TestWholeFile`:
```go
{name: "comment marker inside string preserved", file: "js_string_marker.js"},
{name: "file with no comments is unchanged", file: "go_nocomment.go"},
{name: "intentional blank lines preserved", file: "py_blanks.py"},
{name: "escaped quote then line comment", file: "js_escaped.js"},
{name: "lua marker in string vs real comment", file: "lua_string.lua"},
```
- [ ] **Step 2: Run to verify it fails**
Run: `go test ./internal/processor -run TestWholeFile -v`
Expected: FAIL — missing `testdata/js_string_marker.js` (first new case).
- [ ] **Step 3: Create fixtures with exact bytes**
`testdata/js_string_marker.js`:
```
const u = "http://example.com"; // real
console.log("/* not */");
```
`testdata/js_string_marker.js.golden` (line 1 comment stripped; line 2 untouched — markers are inside strings):
```
const u = "http://example.com";
console.log("/* not */");
```
`testdata/go_nocomment.go`:
```
package main
func main() { println("hi") }
```
`testdata/go_nocomment.go.golden` (NO comments → file left byte-identical to input):
```
package main
func main() { println("hi") }
```
`testdata/py_blanks.py`:
```
x = 1
# gone
y = 2
```
`testdata/py_blanks.py.golden` (only the comment-only line is dropped; both blank lines stay):
```
x = 1
y = 2
```
`testdata/js_escaped.js`:
```
const s = "a\"b"; // c
```
`testdata/js_escaped.js.golden` (escaped `\"` does not end the string; trailing comment stripped):
```
const s = "a\"b";
```
`testdata/lua_string.lua`:
```
local s = "-- not"
x = 1 -- real
```
`testdata/lua_string.lua.golden` (line 1 marker is in a string → untouched; line 2 comment stripped):
```
local s = "-- not"
x = 1
```
- [ ] **Step 4: Run to verify it passes**
Run: `go test ./internal/processor -run TestWholeFile -v`
Expected: PASS — 7 subtests green.
- [ ] **Step 5: `make check` then commit (jj)**
```bash
jj describe -m "test(processor): line-comment, string-protection, structure cases"
jj new
```
---
## Task 4: Single-line block comments + `--inline`/`--block` flags
**Files:**
- Modify: `internal/processor/wholefile_test.go`
- Create fixtures + `.golden` for: `js_block.js`, `c_multiblock.c`, `js_inline_flag.js`, `js_block_flag.js`
**Interfaces:** Consumes `runWholeFileCase`/`wholeFileCase`. Uses `types.CLI{Inline: true}` / `types.CLI{Block: true}`.
- [ ] **Step 1: Append table entries (failing test)**
```go
{name: "single-line block comment removed", file: "js_block.js"},
{name: "multiple single-line blocks on one line", file: "c_multiblock.c"},
{name: "--inline removes only line comments", file: "js_inline_flag.js", cli: types.CLI{Inline: true}},
{name: "--block removes only block comments", file: "js_block_flag.js", cli: types.CLI{Block: true}},
```
- [ ] **Step 2: Run to verify it fails**
Run: `go test ./internal/processor -run TestWholeFile -v`
Expected: FAIL — missing `testdata/js_block.js`.
- [ ] **Step 3: Create fixtures with exact bytes**
`testdata/js_block.js`:
```
var x = 5; /* c */ var y = 10;
```
`testdata/js_block.js.golden` (block cut in place; note the two spaces where the block was):
```
var x = 5; var y = 10;
```
`testdata/c_multiblock.c`:
```
int a = 1; /* one */ int b = 2; /* two */ int c = 3;
```
`testdata/c_multiblock.c.golden` (both blocks removed; two spaces at each removal site):
```
int a = 1; int b = 2; int c = 3;
```
`testdata/js_inline_flag.js`:
```
code(); /* block */ // line
```
`testdata/js_inline_flag.js.golden` (`--inline`: line comment gone, block kept):
```
code(); /* block */
```
`testdata/js_block_flag.js`:
```
code(); /* block */ // line
```
`testdata/js_block_flag.js.golden` (`--block`: block gone with two spaces, line comment kept):
```
code(); // line
```
- [ ] **Step 4: Run to verify it passes**
Run: `go test ./internal/processor -run TestWholeFile -v`
Expected: PASS — 11 subtests green.
- [ ] **Step 5: `make check` then commit (jj)**
```bash
jj describe -m "test(processor): single-line block + inline/block flag cases"
jj new
```
---
## Task 5: Preservation, `--preserve-lines`, `--backup`
**Files:**
- Modify: `internal/processor/wholefile_test.go`
- Create fixtures + `.golden` for: `go_preserve.go`, `go_wildcard.go`, `py_preserve_lines.py`, `py_backup.py`
**Interfaces:** Consumes `runWholeFileCase`/`wholeFileCase`. Uses a custom `*config.Config` and `types.CLI{PreserveLines: true}` / `types.CLI{Backup: true}` with `wantBackup: true`.
- [ ] **Step 1: Append table entries (failing test)**
```go
{name: "preserve exact pattern TODO", file: "go_preserve.go"},
{name: "preserve wildcard IMPORTANT", file: "go_wildcard.go",
cfg: &config.Config{Preserve: []string{"*IMPORTANT*"}, ContextLines: 3}},
{name: "--preserve-lines keeps comment-only line as blank", file: "py_preserve_lines.py",
cli: types.CLI{PreserveLines: true}},
{name: "--backup writes .bak of original", file: "py_backup.py",
cli: types.CLI{Backup: true}, wantBackup: true},
```
- [ ] **Step 2: Run to verify it fails**
Run: `go test ./internal/processor -run TestWholeFile -v`
Expected: FAIL — missing `testdata/go_preserve.go`.
- [ ] **Step 3: Create fixtures with exact bytes**
`testdata/go_preserve.go`:
```
x := 1 // TODO: fix
y := 2 // remove me
```
`testdata/go_preserve.go.golden` (default preserve list includes `TODO:` → line 1 kept verbatim; line 2 stripped):
```
x := 1 // TODO: fix
y := 2
```
`testdata/go_wildcard.go`:
```
a := 1 // this is IMPORTANT stuff
b := 2 // trivial
```
`testdata/go_wildcard.go.golden` (custom cfg preserves `*IMPORTANT*` → line 1 kept; line 2 stripped):
```
a := 1 // this is IMPORTANT stuff
b := 2
```
`testdata/py_preserve_lines.py` (line 2 is indented with FOUR spaces before `#`):
```
x = 1
# indented comment
y = 2
```
`testdata/py_preserve_lines.py.golden` — **line 2 must be exactly four space characters** (the leading whitespace is preserved, the comment removed), then `x = 1` above and `y = 2` below:
```
x = 1
y = 2
```
(If your editor strips trailing spaces on save, write this file so the middle line contains 4 spaces and nothing else. Verify with `cat -A testdata/py_preserve_lines.py.golden` — the middle line must read ` $`.)
`testdata/py_backup.py`:
```
# gone
keep = 1
```
`testdata/py_backup.py.golden` (comment-only line dropped; `.bak` asserted equal to the original input by `wantBackup`):
```
keep = 1
```
- [ ] **Step 4: Run to verify it passes**
Run: `go test ./internal/processor -run TestWholeFile -v`
Expected: PASS — 15 subtests green (including the backup side-file assertion).
- [ ] **Step 5: `make check` then commit (jj)**
```bash
jj describe -m "test(processor): preservation, preserve-lines, backup cases"
jj new
```
---
## Task 6: Git-mode surgical line-range cases
**Files:**
- Modify: `internal/processor/wholefile_test.go`
- Create fixtures + `.golden` for: `git_whole.py`, `git_range.js`
**Interfaces:** Consumes `runWholeFileCase`/`wholeFileCase` with `gitMode: true` and `ranges` (a `[]git.LineRange`). `git.LineRange` has exported fields `Start int`, `End int`. No real git invocation — ranges are passed directly.
- [ ] **Step 1: Append table entries (failing test)**
```go
{name: "git whole file (untracked, nil ranges)", file: "git_whole.py", gitMode: true},
{name: "git range limits stripping to changed lines", file: "git_range.js",
gitMode: true, ranges: []git.LineRange{{Start: 1, End: 1}}},
```
- [ ] **Step 2: Run to verify it fails**
Run: `go test ./internal/processor -run TestWholeFile -v`
Expected: FAIL — missing `testdata/git_whole.py`.
- [ ] **Step 3: Create fixtures with exact bytes**
`testdata/git_whole.py`:
```
# gone
x = 1 # inline
```
`testdata/git_whole.py.golden` (nil ranges → whole file processed: comment-only dropped, inline stripped):
```
x = 1
```
`testdata/git_range.js`:
```
a(); // one
b(); // two
```
`testdata/git_range.js.golden` (range `{1,1}` → only line 1 stripped; line 2 comment kept because line 2 is outside the range):
```
a();
b(); // two
```
- [ ] **Step 4: Run to verify it passes**
Run: `go test ./internal/processor -run TestWholeFile -v`
Expected: PASS — 17 subtests green.
- [ ] **Step 5: Full suite + `make check`, then commit (jj)**
Run: `go test ./... && make check`
Expected: all packages green; fmt/vet/test clean.
```bash
jj describe -m "test(processor): git-mode whole-file and line-range cases"
jj new
```
---
## Self-Review (completed during authoring)
- **Spec coverage:** All 17 cases from the spec matrix map to Tasks 2–6 (2 seed + 5 + 4 + 4 + 2 = 17). Prod-refactor decision (inject cfg) = Task 1.
- **Placeholder scan:** Every fixture gives exact bytes; every step has a concrete command + expected output. No TBD/TODO.
- **Type consistency:** `processFileInMemory(filename, language, cfg)` and `processFileWithLineRanges(filename, ranges, cfg)` signatures are defined in Task 1 and consumed identically in Tasks 2–6. `git.LineRange{Start,End}` matches `internal/git/types.go`. `config.Config{Preserve, ContextLines}` matches `internal/config/config.go`.
- **Out of scope confirmed:** no multi-line block fixtures anywhere.