processor/history_coupling_test.go GO 420 lines View on github.com → Search inside
1// SPDX-License-Identifier: MIT23package processor45import (6	"os"7	"path/filepath"8	"strings"9	"testing"10)1112// hasCandidate reports whether want is among the candidate forms.13func hasCandidate(got []string, want string) bool {14	for _, g := range got {15		if g == want {16			return true17		}18	}19	return false20}2122// A "./"-prefixed path is what a shell tab-completes to, and it must resolve to23// the bare git path. This was the original --coupling-for bug: the prefix was24// passed through and never matched, after a full history walk.25func TestCouplingTargetCandidatesStripsDotSlash(t *testing.T) {26	got := couplingTargetCandidates("/repo", "./processor/constants.go")27	if !hasCandidate(got, "processor/constants.go") {28		t.Errorf("expected ./ prefix to be stripped, got %v", got)29	}30}3132func TestCouplingTargetCandidatesBarePathUnchanged(t *testing.T) {33	got := couplingTargetCandidates("/repo", "processor/constants.go")34	if !hasCandidate(got, "processor/constants.go") {35		t.Errorf("expected bare path preserved, got %v", got)36	}37}3839func TestCouplingTargetCandidatesAbsoluteBecomesRepoRelative(t *testing.T) {40	got := couplingTargetCandidates("/repo", filepath.FromSlash("/repo/processor/constants.go"))41	if !hasCandidate(got, "processor/constants.go") {42		t.Errorf("expected absolute path made repo-relative, got %v", got)43	}44}4546// A path outside the repository can never be a git path, so it must yield no47// candidates rather than a "../" form that would be looked up and miss.48func TestCouplingTargetCandidatesRejectsEscapingPaths(t *testing.T) {49	if got := couplingTargetCandidates("/repo", filepath.FromSlash("/etc/passwd")); len(got) != 0 {50		t.Errorf("expected no candidates for a path outside the repo, got %v", got)51	}52	if got := couplingTargetCandidates("/repo", "../../../etc/passwd"); len(got) != 0 {53		t.Errorf("expected no candidates for an escaping relative path, got %v", got)54	}55}5657// When the typed form and the cwd-relative form agree, only one candidate should58// survive — the lookup is against HEAD and duplicates just cost tree reads.59func TestCouplingTargetCandidatesDedupes(t *testing.T) {60	cwd, err := os.Getwd()61	if err != nil {62		t.Skip("cannot determine working directory")63	}64	got := couplingTargetCandidates(cwd, "./x/y.go")65	if len(got) != 1 || got[0] != "x/y.go" {66		t.Errorf("expected exactly one deduped candidate x/y.go, got %v", got)67	}68}6970// Running scc from inside a subdirectory: `cd processor && scc --coupling-for71// constants.go` must offer processor/constants.go, since git keys from the root.72func TestCouplingTargetCandidatesResolvesFromSubdirectory(t *testing.T) {73	cwd, err := os.Getwd()74	if err != nil {75		t.Skip("cannot determine working directory")76	}77	repoRoot := filepath.Dir(cwd) // tests run in ./processor, so this is the repo root78	got := couplingTargetCandidates(repoRoot, "constants.go")79	want := filepath.Base(cwd) + "/constants.go"80	if !hasCandidate(got, want) {81		t.Errorf("expected %q among candidates for a subdirectory-relative path, got %v", want, got)82	}83}8485// headWith builds a HeadSnapshot whose Files map contains every named path, so86// the Finalise survivor filter keeps them.87func headWith(paths ...string) HeadSnapshot {88	h := HeadSnapshot{Files: map[string]HeadFile{}}89	for _, p := range paths {90		h.Files[p] = HeadFile{Path: p}91	}92	return h93}9495// commit is a tiny helper turning a list of paths into the []FileChange the96// observer consumes (only Path matters for coupling).97func commit(paths ...string) []FileChange {98	out := make([]FileChange, 0, len(paths))99	for _, p := range paths {100		out = append(out, FileChange{Path: p})101	}102	return out103}104105func findPair(pairs []CouplingCount, a, b string) (CouplingCount, bool) {106	if a > b {107		a, b = b, a108	}109	for _, p := range pairs {110		if p.A == a && p.B == b {111			return p, true112		}113	}114	return CouplingCount{}, false115}116117func TestCouplingBasicCounts(t *testing.T) {118	o := newCouplingObserver()119	// a+b change together twice; a alone once; b+c together once.120	o.Observe(CommitInfo{}, commit("a.go", "b.go"))121	o.Observe(CommitInfo{}, commit("a.go", "b.go"))122	o.Observe(CommitInfo{}, commit("a.go"))123	o.Observe(CommitInfo{}, commit("b.go", "c.go"))124	o.Finalise(HistoryWindow{}, headWith("a.go", "b.go", "c.go"))125126	ab, ok := findPair(o.pairs, "a.go", "b.go")127	if !ok {128		t.Fatalf("expected a.go↔b.go pair, got %+v", o.pairs)129	}130	if ab.Shared != 2 {131		t.Errorf("a↔b Shared = %d, want 2", ab.Shared)132	}133	if ab.CommitsA != 3 { // a.go changed in 3 commits134		t.Errorf("a.go CommitsA = %d, want 3", ab.CommitsA)135	}136	if ab.CommitsB != 3 { // b.go changed in 3 commits137		t.Errorf("b.go CommitsB = %d, want 3", ab.CommitsB)138	}139140	// b↔c shared only once, below CouplingMinShared (2) → must be filtered out.141	if _, ok := findPair(o.pairs, "b.go", "c.go"); ok {142		t.Errorf("b.go↔c.go shares 1 commit; should be below the min-shared floor")143	}144}145146func TestCouplingDegree(t *testing.T) {147	// a and b each change in exactly the same 2 commits → union 2, degree 100%.148	o := newCouplingObserver()149	o.Observe(CommitInfo{}, commit("a.go", "b.go"))150	o.Observe(CommitInfo{}, commit("a.go", "b.go"))151	o.Finalise(HistoryWindow{}, headWith("a.go", "b.go"))152153	ab, _ := findPair(o.pairs, "a.go", "b.go")154	if got := ab.Degree(); got != 100.0 {155		t.Errorf("degree = %.1f, want 100.0", got)156	}157}158159func TestCouplingLargeCommitSkipped(t *testing.T) {160	o := newCouplingObserver()161	o.maxFilesPerCommit = 3162163	// A 4-file commit exceeds the cap: no pairs counted from it, but each file's164	// own commit total still increments.165	o.Observe(CommitInfo{}, commit("a.go", "b.go", "c.go", "d.go"))166	// A normal 2-file commit still produces a pair.167	o.Observe(CommitInfo{}, commit("a.go", "b.go"))168	o.Observe(CommitInfo{}, commit("a.go", "b.go"))169	o.Finalise(HistoryWindow{}, headWith("a.go", "b.go", "c.go", "d.go"))170171	if o.skipped != 1 {172		t.Errorf("skipped = %d, want 1", o.skipped)173	}174	ab, ok := findPair(o.pairs, "a.go", "b.go")175	if !ok {176		t.Fatalf("expected a↔b pair from the small commits")177	}178	// Shared from the two small commits only — the big commit did not add one.179	if ab.Shared != 2 {180		t.Errorf("a↔b Shared = %d, want 2 (big commit excluded)", ab.Shared)181	}182	// But a.go's own total includes the big commit: 3 commits.183	if ab.CommitsA != 3 {184		t.Errorf("a.go CommitsA = %d, want 3", ab.CommitsA)185	}186	// c.go↔d.go only ever co-changed in the skipped big commit → no pair.187	if _, ok := findPair(o.pairs, "c.go", "d.go"); ok {188		t.Errorf("c↔d should not exist; their only co-change was the skipped commit")189	}190}191192func TestCouplingRenameFolds(t *testing.T) {193	o := newCouplingObserver()194	// old.go couples with b.go twice, then old.go is renamed to new.go and195	// couples with b.go once more under the new name.196	o.Observe(CommitInfo{}, commit("old.go", "b.go"))197	o.Observe(CommitInfo{}, commit("old.go", "b.go"))198	o.Observe(CommitInfo{}, []FileChange{{Path: "new.go", FromPath: "old.go"}, {Path: "b.go"}})199	o.Finalise(HistoryWindow{}, headWith("new.go", "b.go"))200201	// old.go is gone from HEAD; its history must fold into new.go.202	if _, ok := findPair(o.pairs, "old.go", "b.go"); ok {203		t.Errorf("old.go should have folded into new.go, not survive as a pair")204	}205	nb, ok := findPair(o.pairs, "new.go", "b.go")206	if !ok {207		t.Fatalf("expected new.go↔b.go after rename fold, got %+v", o.pairs)208	}209	if nb.Shared != 3 {210		t.Errorf("new.go↔b.go Shared = %d, want 3 (2 pre-rename + 1 post)", nb.Shared)211	}212	if nb.CommitsA != 3 {213		t.Errorf("new.go CommitsA = %d, want 3 (old.go's history folded in)", nb.CommitsA)214	}215}216217func TestCouplingPartnersDirectional(t *testing.T) {218	o := newCouplingObserver()219	// hub.h is touched in every commit; leaf.c in 3 of them, always with hub.h.220	// peer.c changes with leaf.c twice.221	o.Observe(CommitInfo{}, commit("hub.h", "leaf.c", "peer.c"))222	o.Observe(CommitInfo{}, commit("hub.h", "leaf.c", "peer.c"))223	o.Observe(CommitInfo{}, commit("hub.h", "leaf.c"))224	o.Observe(CommitInfo{}, commit("hub.h"))225	o.Observe(CommitInfo{}, commit("hub.h"))226	o.Finalise(HistoryWindow{}, headWith("hub.h", "leaf.c", "peer.c"))227228	partners := o.partnersFor("leaf.c")229	if len(partners) < 2 {230		t.Fatalf("expected leaf.c to couple with hub.h and peer.c, got %+v", partners)231	}232233	// leaf.c changed in 3 commits; hub.h co-changed all 3 → Couple(hub|leaf)=100%.234	// But Reverse(leaf|hub) = 3/5 = 60% — the asymmetry that marks hub.h a hub.235	hub := findPartner(partners, "hub.h")236	if hub.Couple() != 100.0 {237		t.Errorf("Couple(hub.h | leaf.c) = %.1f, want 100.0", hub.Couple())238	}239	if got := hub.Reverse(); got < 59.9 || got > 60.1 {240		t.Errorf("Reverse(leaf.c | hub.h) = %.1f, want ~60.0", got)241	}242243	// peer.c co-changed with leaf.c in 2 of leaf's 3 commits → Couple ~66.7%.244	peer := findPartner(partners, "peer.c")245	if got := peer.Couple(); got < 66.0 || got > 67.0 {246		t.Errorf("Couple(peer.c | leaf.c) = %.1f, want ~66.7", got)247	}248249	// Degree is base-rate corrected, so the hub scores BELOW the peer even though250	// its Couple is a perfect 100%:251	//   hub.h  → 3/(3+5-3) = 60.0%   (present for all of leaf's commits, but it is252	//                                 present for everyone's commits)253	//   peer.c → 2/(3+2-2) = 66.7%   (never changes without leaf.c — a real partner)254	if got := hub.Degree(); got < 59.9 || got > 60.1 {255		t.Errorf("Degree(hub.h, leaf.c) = %.1f, want ~60.0", got)256	}257	if got := peer.Degree(); got < 66.0 || got > 67.0 {258		t.Errorf("Degree(peer.c, leaf.c) = %.1f, want ~66.7", got)259	}260261	// Ranked by Degree: peer.c before hub.h. Ranking by Couple instead put the hub262	// first — the base-rate confound this report exists to avoid.263	if partners[0].Path != "peer.c" {264		t.Errorf("expected peer.c ranked first by Degree, got %s", partners[0].Path)265	}266}267268func TestCouplingPartnersUnknownTarget(t *testing.T) {269	o := newCouplingObserver()270	o.Observe(CommitInfo{}, commit("a.go", "b.go"))271	o.Finalise(HistoryWindow{}, headWith("a.go", "b.go"))272	if got := o.partnersFor("nope.go"); got != nil {273		t.Errorf("unknown target should yield nil partners, got %+v", got)274	}275}276277func findPartner(ps []CouplingPartner, path string) CouplingPartner {278	for _, p := range ps {279		if p.Path == path {280			return p281		}282	}283	return CouplingPartner{}284}285286func TestCouplingDropsFilesAbsentFromHead(t *testing.T) {287	o := newCouplingObserver()288	o.Observe(CommitInfo{}, commit("a.go", "gone.go"))289	o.Observe(CommitInfo{}, commit("a.go", "gone.go"))290	// gone.go is not in HEAD (deleted before the window end).291	o.Finalise(HistoryWindow{}, headWith("a.go"))292293	if len(o.pairs) != 0 {294		t.Errorf("pair referencing a deleted file should be dropped, got %+v", o.pairs)295	}296}297298// headWithComplexity builds a HeadSnapshot whose files carry the given HEAD299// cyclomatic complexity, for exercising the complexity-weighted ranking.300func headWithComplexity(cx map[string]int64) HeadSnapshot {301	h := HeadSnapshot{Files: map[string]HeadFile{}}302	for p, c := range cx {303		h.Files[p] = HeadFile{Path: p, Complexity: c}304	}305	return h306}307308// Weighted ranking must demote a high-churn pair that includes a309// zero-complexity (data/generated) file below a lower-churn pair of two complex310// files — the whole point of --coupling-weighted.311func TestCouplingWeightedDemotesDataFilePairs(t *testing.T) {312	prev := CouplingWeighted313	CouplingWeighted = true314	defer func() { CouplingWeighted = prev }()315316	o := newCouplingObserver()317	// data.json + gen.go co-change often (5), but data.json has zero complexity.318	for i := 0; i < 5; i++ {319		o.Observe(CommitInfo{}, commit("data.json", "gen.go"))320	}321	// logic_a.go + logic_b.go co-change less (3), but both are complex.322	for i := 0; i < 3; i++ {323		o.Observe(CommitInfo{}, commit("logic_a.go", "logic_b.go"))324	}325	o.Finalise(HistoryWindow{}, headWithComplexity(map[string]int64{326		"data.json": 0, "gen.go": 120,327		"logic_a.go": 100, "logic_b.go": 100,328	}))329330	if len(o.pairs) != 2 {331		t.Fatalf("expected 2 pairs, got %d: %+v", len(o.pairs), o.pairs)332	}333	top := o.pairs[0]334	if top.A != "logic_a.go" || top.B != "logic_b.go" {335		t.Errorf("weighted top pair = (%s, %s), want the two complex files first", top.A, top.B)336	}337	// The data-file pair's min-complexity is zero, so its weighted score is zero.338	dataPair, ok := findPair(o.pairs, "data.json", "gen.go")339	if !ok {340		t.Fatal("data.json/gen.go pair missing")341	}342	if dataPair.WeightedScore() != 0 {343		t.Errorf("data-file pair weighted score = %.1f, want 0", dataPair.WeightedScore())344	}345}346347// With weighting off (the default), ranking is pure co-change volume — the348// zero-complexity pair with more shared commits leads.349func TestCouplingUnweightedRanksByVolume(t *testing.T) {350	o := newCouplingObserver()351	for i := 0; i < 5; i++ {352		o.Observe(CommitInfo{}, commit("data.json", "gen.go"))353	}354	for i := 0; i < 3; i++ {355		o.Observe(CommitInfo{}, commit("logic_a.go", "logic_b.go"))356	}357	o.Finalise(HistoryWindow{}, headWithComplexity(map[string]int64{358		"data.json": 0, "gen.go": 120,359		"logic_a.go": 100, "logic_b.go": 100,360	}))361362	top := o.pairs[0]363	if top.A != "data.json" || top.B != "gen.go" {364		t.Errorf("unweighted top pair = (%s, %s), want the higher-volume data pair first", top.A, top.B)365	}366}367368// resolveCouplingTarget's error must be context-neutral: it names the path, not369// any CLI flag, so an MCP client (which passed a `file` argument and has never370// seen the flag) gets a message that reads correctly. The CLI re-attaches the371// flag name at its own call site.372func TestResolveCouplingTargetMissIsFlagNeutral(t *testing.T) {373	repo := makeFixtureRepo(t, []map[string]string{374		{"processor/workers.go": "package processor\n// v0\n", "main.go": "package main\n// v0\n"},375	})376377	_, err := resolveCouplingTarget(repo, "processor/wokers.go")378	if err == nil {379		t.Fatal("expected an error for a path not in HEAD, got nil")380	}381	msg := err.Error()382	if !strings.Contains(msg, "not in HEAD") {383		t.Errorf("error = %q, want it to mention the target is not in HEAD", msg)384	}385	if strings.Contains(msg, "--") {386		t.Errorf("error = %q, want no CLI flag names in the neutral core error", msg)387	}388	// A basename typo (wokers vs workers) changes the basename, so the389	// same-basename suggestion heuristic does not fire — no suggestion here.390	if strings.Contains(msg, "did you mean") {391		t.Errorf("error = %q, did not expect a suggestion for a basename typo", msg)392	}393}394395// Suggestions are built inside resolveCouplingTarget, so they reach every caller396// — CLI and MCP alike. A same-basename file in another directory triggers the397// "did you mean" hint; this asserts it is present in the returned error itself398// (not only in CLI-side rendering).399func TestResolveCouplingTargetSuggestsSameBasename(t *testing.T) {400	repo := makeFixtureRepo(t, []map[string]string{401		{"processor/workers.go": "package processor\n// v0\n", "main.go": "package main\n// v0\n"},402	})403404	// Right basename, wrong directory — the heuristic matches on basename.405	_, err := resolveCouplingTarget(repo, "workers.go")406	if err == nil {407		t.Fatal("expected an error for a path not in HEAD, got nil")408	}409	msg := err.Error()410	if !strings.Contains(msg, "did you mean") {411		t.Fatalf("error = %q, want a did-you-mean suggestion", msg)412	}413	if !strings.Contains(msg, "processor/workers.go") {414		t.Errorf("error = %q, want it to suggest processor/workers.go", msg)415	}416	if strings.Contains(msg, "--") {417		t.Errorf("error = %q, want no CLI flag names in the neutral core error", msg)418	}419}

Code quality findings 3

Defer inside loop; deferred calls accumulate until the function returns, not until the loop iteration ends. This can cause resource leaks
warning correctness defer-in-loop
defer func() { CouplingWeighted = prev }()
Multiple appends without pre-allocation; use make() with capacity when size is known
info performance append-without-prealloc
out = append(out, FileChange{Path: p})
Range over slice copies each element by value; use index or pointer receiver for large structs to avoid copies
info performance copy-large-struct
for p, c := range cx {

Get this view in your editor

Same data, no extra tab — call code_get_file + code_get_findings over MCP from Claude/Cursor/Copilot.