From 371f6d5ff34e881ed5eaf54d27c3694acbf36700 Mon Sep 17 00:00:00 2001 From: Youngsup Oh Date: Fri, 17 Jul 2026 15:56:32 +0900 Subject: [PATCH 1/2] test: cover tree selector, Python/Ruby/Global integration, and DirSize - ui: exercise the tree selector's toggle, parent/child selection propagation, cursor movement, quick-select keys, and Update key routing via injected KeyMsgs (internal/ui 35% -> 57%). - integration: add Python and Ruby project fixtures to the scan workspace and a Global-scanner pipeline test that injects HOME; regenerate the golden output for the new fixtures. - scanner: pin DirSize's sizing arithmetic (lower bound + growth), which the golden test can't verify since it zeroes Size out for portability. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01YSTf3ozF4SnboZHyEi7Ycc --- CHANGELOG.md | 1 + internal/integration_test.go | 71 ++++++- internal/scanner/sizing_test.go | 38 ++++ internal/testdata/scan_output.golden.json | 44 ++++- internal/ui/treeselector_test.go | 230 ++++++++++++++++++++++ 5 files changed, 376 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d525641..f7cbe36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The walk engine now reads each directory once and reuses those entries for both project-marker detection and recursion, instead of reading every directory twice (a `filepath.WalkDir` read plus a second `os.ReadDir`). Directory traversal — the dominant cost of scanning a large tree — is roughly 1.8× faster (~29% faster end-to-end on a workspace with tens of thousands of directories); scan results are unchanged. - Git classification now forks `git` across a bounded worker pool (`min(NumCPU, 8)`) instead of one repo at a time — root resolution, `status`/`log`, and `check-ignore` all run concurrently across repos. On a workspace spanning dozens of repos this cut the classify phase roughly in half (~1.24s → ~0.54s warm cache, ~19% faster end-to-end); protection and activity results are unchanged. - The status-badge, safety-icon, and relative-time display formatters, previously duplicated between the table output and the interactive tree selector, are now single shared functions in `internal/ui` (`StatusBadge`, `SafetyIcon`, `RelativeTime`). Table and selector rendering are unchanged; the formatters gained direct unit tests. +- Test coverage: the interactive tree selector's toggle, parent/child selection propagation, cursor movement, quick-select keys, and `Update` key routing are now tested (`internal/ui` 35% → 57%); the scan→classify integration workspace gained Python and Ruby project fixtures plus a Global-scanner (fixed-home) pipeline test; and `DirSize` now has a test that pins the sizing arithmetic (the golden test zeroes sizes out for portability). ### Fixed diff --git a/internal/integration_test.go b/internal/integration_test.go index f4bf39f..31a12a8 100644 --- a/internal/integration_test.go +++ b/internal/integration_test.go @@ -52,6 +52,22 @@ func setupTestWorkspace(t *testing.T) string { mustWriteFile(t, filepath.Join(rustProject, "target", "debug", "cli-tool"), make([]byte, 8192)) mustWriteFile(t, filepath.Join(rustProject, "Cargo.toml"), []byte("[package]")) + // Python project with __pycache__ and .pytest_cache + pyProject := filepath.Join(root, "py-svc") + mustMkdir(t, filepath.Join(pyProject, "__pycache__")) + mustWriteFile(t, filepath.Join(pyProject, "__pycache__", "mod.cpython-312.pyc"), make([]byte, 1024)) + mustMkdir(t, filepath.Join(pyProject, ".pytest_cache")) + mustWriteFile(t, filepath.Join(pyProject, ".pytest_cache", "lastfailed"), make([]byte, 512)) + mustWriteFile(t, filepath.Join(pyProject, "pyproject.toml"), []byte("[project]")) + + // Ruby project with vendor/bundle and .bundle + rbProject := filepath.Join(root, "rb-app") + mustMkdir(t, filepath.Join(rbProject, "vendor", "bundle", "ruby")) + mustWriteFile(t, filepath.Join(rbProject, "vendor", "bundle", "ruby", "gem.rb"), make([]byte, 2048)) + mustMkdir(t, filepath.Join(rbProject, ".bundle")) + mustWriteFile(t, filepath.Join(rbProject, ".bundle", "config"), make([]byte, 256)) + mustWriteFile(t, filepath.Join(rbProject, "Gemfile"), []byte("source 'https://rubygems.org'")) + return root } @@ -85,17 +101,16 @@ func TestIntegration_ScanClassifyPipeline(t *testing.T) { for _, r := range results { ecoFound[r.Ecosystem] = true } - if !ecoFound[model.EcoNode] { - t.Error("expected Node.js ecosystem in results") - } - if !ecoFound[model.EcoRust] { - t.Error("expected Rust ecosystem in results") + for _, eco := range []model.Ecosystem{model.EcoNode, model.EcoRust, model.EcoPython, model.EcoRuby} { + if !ecoFound[eco] { + t.Errorf("expected %s ecosystem in results", eco) + } } // 4. Group by project projects := model.GroupByProject(results) - if len(projects) < 2 { - t.Errorf("expected at least 2 projects (my-app, cli-tool), got %d", len(projects)) + if len(projects) < 4 { + t.Errorf("expected at least 4 projects (my-app, cli-tool, py-svc, rb-app), got %d", len(projects)) } } @@ -155,6 +170,48 @@ func TestIntegration_FilterByEcosystem(t *testing.T) { } } +// TestIntegration_GlobalScannerPipeline drives the Global stat scanner through +// the registry with an injected HOME, then classifies — exercising the full +// scan→classify path for a fixed-home scanner (the walk fixtures above only +// cover the tree-walking scanners). +func TestIntegration_GlobalScannerPipeline(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + npm := filepath.Join(home, ".npm") + mustMkdir(t, npm) + mustWriteFile(t, filepath.Join(npm, "cache.json"), make([]byte, 2048)) + + reg := scanner.DefaultRegistry() + globalScanners := reg.ForEcosystems([]model.Ecosystem{model.EcoGlobal}) + // Global ignores the path argument and scans the injected HOME. + results, err := reg.ScanWith(context.Background(), home, globalScanners) + if err != nil { + t.Fatalf("scan error: %v", err) + } + + var npmResult *model.ScanResult + for i := range results { + if results[i].Ecosystem != model.EcoGlobal { + t.Errorf("expected only global ecosystem, got %s for %s", results[i].Ecosystem, results[i].Path) + } + if results[i].Path == npm { + npmResult = &results[i] + } + } + if npmResult == nil { + t.Fatalf("expected ~/.npm to be detected under injected HOME") + } + if npmResult.Safety != model.SafetySafe { + t.Errorf("~/.npm: expected safety=safe, got %s", npmResult.Safety) + } + + classifier.ClassifyResults(results, classifier.DefaultThresholds()) + if npmResult.Activity == "" { + t.Error("classify should assign an activity status to the global cache") + } +} + func TestIntegration_FilterResults(t *testing.T) { root := setupTestWorkspace(t) diff --git a/internal/scanner/sizing_test.go b/internal/scanner/sizing_test.go index 5387f72..0a2ddb4 100644 --- a/internal/scanner/sizing_test.go +++ b/internal/scanner/sizing_test.go @@ -67,6 +67,44 @@ func TestSizePendingEmpty(t *testing.T) { } } +// TestDirSizeReflectsContents checks DirSize actually measures bytes, not just +// returns a positive number: the reported size must cover the logical content +// (du rounds up to block boundaries, so it is a lower bound) and must grow when +// more data is added. The golden test zeroes Size out for portability, so this +// is where the sizing arithmetic itself is pinned. +func TestDirSizeReflectsContents(t *testing.T) { + dir := t.TempDir() + + empty := DirSize(dir) + + const fileSize = 100_000 + for _, name := range []string{"a", "b", "c"} { + if err := os.WriteFile(filepath.Join(dir, name), make([]byte, fileSize), 0o644); err != nil { + t.Fatal(err) + } + } + withThree := DirSize(dir) + if withThree < 3*fileSize { + t.Errorf("DirSize with 300KB of files = %d, want >= %d", withThree, 3*fileSize) + } + if withThree <= empty { + t.Errorf("DirSize did not grow after adding files: empty=%d, withThree=%d", empty, withThree) + } + + // Adding a nested file must increase the measured size further. + sub := filepath.Join(dir, "nested") + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(sub, "d"), make([]byte, fileSize), 0o644); err != nil { + t.Fatal(err) + } + withFour := DirSize(dir) + if withFour <= withThree { + t.Errorf("DirSize did not grow after adding a nested file: withThree=%d, withFour=%d", withThree, withFour) + } +} + // TestDirSizeMissingPath locks the deferred-sizing TOCTOU contract: an artifact // deleted between walk discovery and sizing must yield 0, not a panic. The // walk→size gap widened when sizing moved to a post-walk phase. diff --git a/internal/testdata/scan_output.golden.json b/internal/testdata/scan_output.golden.json index 9dbe3cc..ccc4692 100644 --- a/internal/testdata/scan_output.golden.json +++ b/internal/testdata/scan_output.golden.json @@ -1,6 +1,6 @@ { "total_size": 0, - "total_count": 3, + "total_count": 7, "results": [ { "path": ".next", @@ -31,6 +31,48 @@ "activity": "active", "safety": "safe", "protected": false + }, + { + "path": ".bundle", + "ecosystem": "ruby", + "category": "cache", + "size": 0, + "last_modified": "0001-01-01T00:00:00Z", + "activity": "active", + "safety": "safe", + "protected": false + }, + { + "path": "bundle", + "ecosystem": "ruby", + "category": "deps", + "size": 0, + "last_modified": "0001-01-01T00:00:00Z", + "activity": "active", + "safety": "safe", + "protected": false + }, + { + "path": ".pytest_cache", + "ecosystem": "python", + "category": "cache", + "size": 0, + "last_modified": "0001-01-01T00:00:00Z", + "activity": "active", + "safety": "safe", + "protected": false, + "project_root": "/var/folders/nb/j_zv8t0x2cbbkl2yvh46qthw0000gn/T/TestGolden_JSONOutput3411242693/001/py-svc" + }, + { + "path": "__pycache__", + "ecosystem": "python", + "category": "build", + "size": 0, + "last_modified": "0001-01-01T00:00:00Z", + "activity": "active", + "safety": "safe", + "protected": false, + "project_root": "/var/folders/nb/j_zv8t0x2cbbkl2yvh46qthw0000gn/T/TestGolden_JSONOutput3411242693/001/py-svc" } ] } diff --git a/internal/ui/treeselector_test.go b/internal/ui/treeselector_test.go index f907d3c..8b624fe 100644 --- a/internal/ui/treeselector_test.go +++ b/internal/ui/treeselector_test.go @@ -5,7 +5,9 @@ package ui import ( "testing" + "time" + tea "github.com/charmbracelet/bubbletea" "github.com/ohing504/devclean/internal/model" ) @@ -56,6 +58,234 @@ func selectedPaths(m treeModel) map[string]bool { return out } +// newRichFixture builds two unprotected projects: an active "app" with two +// artifacts (for partial-selection tests) and an entirely-dormant "old" with +// one. Sizes make app sort first, giving a stable layout: +// [0]header [1]app [2]node_modules [3].next [4]old [5]cache. +func newRichFixture(t *testing.T) treeModel { + t.Helper() + // GroupByProject derives a project's Activity from its most recent item, so + // the results carry non-zero LastMod (the newest wins per project). + ts := time.Unix(1_700_000_000, 0) + results := []model.ScanResult{ + {Path: "/app/node_modules", Ecosystem: model.EcoNode, Size: 200, Safety: model.SafetySafe, Activity: model.StatusActive, LastMod: ts, ProjectRoot: "/app"}, + {Path: "/app/.next", Ecosystem: model.EcoNode, Size: 100, Safety: model.SafetySafe, Activity: model.StatusActive, LastMod: ts, ProjectRoot: "/app"}, + {Path: "/old/cache", Ecosystem: model.EcoNode, Size: 50, Safety: model.SafetySafe, Activity: model.StatusDormant, LastMod: ts, ProjectRoot: "/old"}, + } + m := treeModel{items: BuildTreeItems(results)} + if len(m.items) != 6 { + t.Fatalf("expected 6 items, got %d: %+v", len(m.items), m.items) + } + return m +} + +// firstProject returns the index of the first project row. +func firstProject(m treeModel) int { + for i, it := range m.items { + if it.Type == ItemProject { + return i + } + } + return -1 +} + +// protectedProject returns the index of the first protected project row. +func protectedProject(m treeModel) int { + for i, it := range m.items { + if it.Type == ItemProject && it.Protected { + return i + } + } + return -1 +} + +func TestToggleCurrent_ProjectPropagatesToChildren(t *testing.T) { + m := newRichFixture(t) + m.cursor = firstProject(m) // "app" + + m.toggleCurrent() + proj := m.items[m.cursor] + if !proj.Selected { + t.Error("toggling a project should select it") + } + for _, ci := range proj.Children { + if !m.items[ci].Selected { + t.Errorf("child %d should follow the project into selection", ci) + } + } + + m.toggleCurrent() + if m.items[m.cursor].Selected { + t.Error("toggling again should deselect the project") + } + for _, ci := range m.items[m.cursor].Children { + if m.items[ci].Selected { + t.Errorf("child %d should follow the project out of selection", ci) + } + } +} + +func TestToggleCurrent_ProtectedProjectIgnored(t *testing.T) { + m := newSelectorFixture(t) + m.cursor = protectedProject(m) + if m.cursor < 0 { + t.Fatal("fixture must contain a protected project") + } + + m.toggleCurrent() + if m.items[m.cursor].Selected { + t.Error("a protected project must not become selected") + } + for _, ci := range m.items[m.cursor].Children { + if m.items[ci].Selected { + t.Errorf("child %d of a protected project must not be selected", ci) + } + } +} + +func TestToggleCurrent_ArtifactUpdatesParentState(t *testing.T) { + m := newRichFixture(t) + projIdx := firstProject(m) + children := m.items[projIdx].Children + if len(children) != 2 { + t.Fatalf("expected 2 children, got %d", len(children)) + } + + // Select one child: parent is partially selected, not fully. + m.cursor = children[0] + m.toggleCurrent() + if m.items[projIdx].Selected { + t.Error("parent should not be fully selected with one child unselected") + } + if !m.isPartiallySelected(projIdx) { + t.Error("parent should be partially selected with one of two children on") + } + + // Select the second: parent becomes fully selected, no longer partial. + m.cursor = children[1] + m.toggleCurrent() + if !m.items[projIdx].Selected { + t.Error("parent should be fully selected once all children are on") + } + if m.isPartiallySelected(projIdx) { + t.Error("parent should not be partial once all children are on") + } +} + +func TestSelectNone_ClearsEverything(t *testing.T) { + m := newRichFixture(t) + m.selectAll() + if len(selectedPaths(m)) == 0 { + t.Fatal("selectAll selected nothing") + } + + m.selectNone() + if got := len(selectedPaths(m)); got != 0 { + t.Errorf("selectNone left %d artifacts selected", got) + } + for i, it := range m.items { + if it.Type == ItemProject && it.Selected { + t.Errorf("selectNone left project row %d selected", i) + } + } +} + +func TestSelectByActivity_PicksOnlyMatching(t *testing.T) { + m := newRichFixture(t) + m.selectByActivity(model.StatusDormant) + + // selectByActivity works at the project level: the entirely-dormant "old" + // project and its child are selected; the active "app" is left alone. + got := selectedPaths(m) + if !got["/old/cache"] { + t.Error("the dormant project's artifact should be selected") + } + if got["/app/node_modules"] || got["/app/.next"] { + t.Error("artifacts of an active project should not be selected by a dormant query") + } +} + +func TestMoveCursor_SkipsHeadersAndClamps(t *testing.T) { + m := newRichFixture(t) + m.cursor = 0 // eco header + + // Moving down from the header must land on a selectable row, never a header. + m.moveCursor(1) + if m.items[m.cursor].Type == ItemEcoHeader { + t.Errorf("moveCursor down landed on a header at %d", m.cursor) + } + + // Moving down past the end clamps to the last index. + for range len(m.items) + 2 { + m.moveCursor(1) + } + if m.cursor != len(m.items)-1 { + t.Errorf("cursor = %d after clamping down, want %d", m.cursor, len(m.items)-1) + } + + // Moving up past the start clamps to 0. + for range len(m.items) + 2 { + m.moveCursor(-1) + } + if m.cursor != 0 { + t.Errorf("cursor = %d after clamping up, want 0", m.cursor) + } +} + +func TestJumpProject_MovesBetweenProjects(t *testing.T) { + m := newRichFixture(t) + first := firstProject(m) + m.cursor = first + + m.jumpProject(1) + if m.items[m.cursor].Type != ItemProject || m.cursor == first { + t.Errorf("jumpProject(1) did not advance to the next project, cursor=%d", m.cursor) + } + second := m.cursor + + // No project below the last one: cursor stays put. + m.jumpProject(1) + if m.cursor != second { + t.Errorf("jumpProject(1) past the last project moved cursor to %d, want %d", m.cursor, second) + } + + m.jumpProject(-1) + if m.cursor != first { + t.Errorf("jumpProject(-1) returned to %d, want %d", m.cursor, first) + } +} + +func TestUpdate_KeyRouting(t *testing.T) { + rune := func(r rune) tea.KeyMsg { return tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{r}} } + + // space toggles the current project. + m := newRichFixture(t) + m.cursor = firstProject(m) + nm, _ := m.Update(rune(' ')) + if !nm.(treeModel).items[m.cursor].Selected { + t.Error("space should toggle the current project on") + } + + // "a" selects all, "n" clears. + nm, _ = nm.(treeModel).Update(rune('n')) + if len(selectedPaths(nm.(treeModel))) != 0 { + t.Error("n should clear the selection") + } + nm, _ = nm.(treeModel).Update(rune('a')) + if len(selectedPaths(nm.(treeModel))) == 0 { + t.Error("a should select all selectable artifacts") + } + + // esc aborts and quits. + aborted, cmd := m.Update(tea.KeyMsg{Type: tea.KeyEsc}) + if !aborted.(treeModel).aborted { + t.Error("esc should set aborted") + } + if cmd == nil { + t.Error("esc should return a quit command") + } +} + func TestSelectAllSkipsProtectedProjects(t *testing.T) { m := newSelectorFixture(t) m.selectAll() From e6c41afd4a43e05ec310094ec4966493cf29d97e Mon Sep 17 00:00:00 2001 From: Youngsup Oh Date: Fri, 17 Jul 2026 16:16:18 +0900 Subject: [PATCH 2/2] test: isolate global-scanner test and drop machine path from golden MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the P2-7 test additions: - Zero ProjectRoot alongside the other volatile fields before writing the golden file — it held the committer's absolute temp path, leaking machine-specific data into version control. - Build the Global scanner directly with an isolated TmpRoot and a stubbed process check instead of going through the registry, so the HOME-injected test no longer scans real machine temp state. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01YSTf3ozF4SnboZHyEi7Ycc --- internal/integration_test.go | 19 +++++++++++-------- internal/testdata/scan_output.golden.json | 6 ++---- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/internal/integration_test.go b/internal/integration_test.go index 31a12a8..6060b66 100644 --- a/internal/integration_test.go +++ b/internal/integration_test.go @@ -170,10 +170,11 @@ func TestIntegration_FilterByEcosystem(t *testing.T) { } } -// TestIntegration_GlobalScannerPipeline drives the Global stat scanner through -// the registry with an injected HOME, then classifies — exercising the full -// scan→classify path for a fixed-home scanner (the walk fixtures above only -// cover the tree-walking scanners). +// TestIntegration_GlobalScannerPipeline drives the Global stat scanner with an +// injected HOME, then classifies — exercising the full scan→classify path for a +// fixed-home scanner (the walk fixtures above only cover the tree-walking +// scanners). The scanner is constructed directly with an isolated TmpRoot and a +// stubbed process check so it never touches real machine temp state. func TestIntegration_GlobalScannerPipeline(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) @@ -182,10 +183,11 @@ func TestIntegration_GlobalScannerPipeline(t *testing.T) { mustMkdir(t, npm) mustWriteFile(t, filepath.Join(npm, "cache.json"), make([]byte, 2048)) - reg := scanner.DefaultRegistry() - globalScanners := reg.ForEcosystems([]model.Ecosystem{model.EcoGlobal}) - // Global ignores the path argument and scans the injected HOME. - results, err := reg.ScanWith(context.Background(), home, globalScanners) + s := &scanner.GlobalScanner{ + TmpRoot: t.TempDir(), // isolate from real browser code-sign clones + ProcessRunning: func(string) bool { return false }, + } + results, err := s.Scan(context.Background(), home) if err != nil { t.Fatalf("scan error: %v", err) } @@ -258,6 +260,7 @@ func TestGolden_JSONOutput(t *testing.T) { results[i].Path = filepath.Base(results[i].Path) results[i].Size = 0 // du returns block-aligned sizes results[i].LastMod = time.Time{} // zero out to make golden file stable + results[i].ProjectRoot = "" // holds the absolute temp path — machine-specific } var buf bytes.Buffer diff --git a/internal/testdata/scan_output.golden.json b/internal/testdata/scan_output.golden.json index ccc4692..1609267 100644 --- a/internal/testdata/scan_output.golden.json +++ b/internal/testdata/scan_output.golden.json @@ -60,8 +60,7 @@ "last_modified": "0001-01-01T00:00:00Z", "activity": "active", "safety": "safe", - "protected": false, - "project_root": "/var/folders/nb/j_zv8t0x2cbbkl2yvh46qthw0000gn/T/TestGolden_JSONOutput3411242693/001/py-svc" + "protected": false }, { "path": "__pycache__", @@ -71,8 +70,7 @@ "last_modified": "0001-01-01T00:00:00Z", "activity": "active", "safety": "safe", - "protected": false, - "project_root": "/var/folders/nb/j_zv8t0x2cbbkl2yvh46qthw0000gn/T/TestGolden_JSONOutput3411242693/001/py-svc" + "protected": false } ] }