Skip to content

Commit 878fa58

Browse files
mpywclaude
mpyw
and
claude
committed
test: improve test coverage from 85.7% to 88.2%
Add test cases for higher-order function patterns that return variables: - goroutine: goodHigherOrderReturnsVariableWithCtx, badHigherOrderReturnsVariableWithoutCtx - errgroup: goodHigherOrderReturnsVariableWithCtx, badHigherOrderReturnsVariableWithoutCtx - waitgroup: goodHigherOrderReturnsVariableWithCtx, badHigherOrderReturnsVariableWithoutCtx - goroutinederive: Higher-order and variable function patterns Add unit tests for ignore directive package: - TestAllCheckerNames - TestParseIgnoreComment - TestBuild - TestShouldIgnore - TestGetUnusedIgnores Key coverage improvements: - returnedValueSatisfiesDerive: 15.8% -> 68.4% - ReturnedValueUsesContext: 13.3% -> 73.3% - AllCheckerNames: 0% -> 100% - usesContextDeep: 85.7% -> 95.2% - checkCallResultFuncUsesContext: 68.8% -> 87.5% 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 3fe60db commit 878fa58

15 files changed

+687
-0
lines changed

‎internal/directives/ignore/directive_test.go‎

Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
package ignore
2+
3+
import (
4+
"go/ast"
5+
"go/parser"
6+
"go/token"
7+
"testing"
8+
)
9+
10+
func TestAllCheckerNames(t *testing.T) {
11+
names := AllCheckerNames()
12+
if len(names) != 7 {
13+
t.Errorf("Expected 7 checker names, got %d", len(names))
14+
}
15+
16+
expected := map[CheckerName]bool{
17+
Goroutine: true,
18+
GoroutineDerive: true,
19+
Waitgroup: true,
20+
Errgroup: true,
21+
Spawner: true,
22+
Spawnerlabel: true,
23+
Gotask: true,
24+
}
25+
26+
for _, name := range names {
27+
if !expected[name] {
28+
t.Errorf("Unexpected checker name: %s", name)
29+
}
30+
}
31+
}
32+
33+
func TestParseIgnoreComment(t *testing.T) {
34+
tests := []struct {
35+
name string
36+
text string
37+
want []CheckerName
38+
wantOk bool
39+
}{
40+
{
41+
name: "basic ignore all",
42+
text: "//goroutinectx:ignore",
43+
want: nil,
44+
wantOk: true,
45+
},
46+
{
47+
name: "ignore specific checker",
48+
text: "//goroutinectx:ignore goroutine",
49+
want: []CheckerName{Goroutine},
50+
wantOk: true,
51+
},
52+
{
53+
name: "ignore multiple checkers",
54+
text: "//goroutinectx:ignore goroutine,errgroup",
55+
want: []CheckerName{Goroutine, Errgroup},
56+
wantOk: true,
57+
},
58+
{
59+
name: "ignore with comment dash",
60+
text: "//goroutinectx:ignore - this is a reason",
61+
want: nil,
62+
wantOk: true,
63+
},
64+
{
65+
name: "ignore specific with comment",
66+
text: "//goroutinectx:ignore goroutine - this is a reason",
67+
want: []CheckerName{Goroutine},
68+
wantOk: true,
69+
},
70+
{
71+
name: "not an ignore comment",
72+
text: "// regular comment",
73+
want: nil,
74+
wantOk: false,
75+
},
76+
{
77+
name: "ignore with leading space",
78+
text: "// goroutinectx:ignore",
79+
want: nil,
80+
wantOk: true,
81+
},
82+
{
83+
name: "ignore with inline comment",
84+
text: "//goroutinectx:ignore goroutine // comment",
85+
want: []CheckerName{Goroutine},
86+
wantOk: true,
87+
},
88+
{
89+
name: "ignore dash only",
90+
text: "//goroutinectx:ignore -",
91+
want: nil,
92+
wantOk: true,
93+
},
94+
}
95+
96+
for _, tt := range tests {
97+
t.Run(tt.name, func(t *testing.T) {
98+
got, ok := parseIgnoreComment(tt.text)
99+
if ok != tt.wantOk {
100+
t.Errorf("parseIgnoreComment() ok = %v, want %v", ok, tt.wantOk)
101+
}
102+
if len(got) != len(tt.want) {
103+
t.Errorf("parseIgnoreComment() = %v, want %v", got, tt.want)
104+
}
105+
for i := range got {
106+
if got[i] != tt.want[i] {
107+
t.Errorf("parseIgnoreComment()[%d] = %v, want %v", i, got[i], tt.want[i])
108+
}
109+
}
110+
})
111+
}
112+
}
113+
114+
func TestBuild(t *testing.T) {
115+
src := `package test
116+
117+
//goroutinectx:ignore
118+
func ignored() {}
119+
120+
//goroutinectx:ignore goroutine
121+
func ignoredGoroutine() {}
122+
`
123+
fset := token.NewFileSet()
124+
file, err := parser.ParseFile(fset, "test.go", src, parser.ParseComments)
125+
if err != nil {
126+
t.Fatalf("Failed to parse: %v", err)
127+
}
128+
129+
m := Build(fset, file)
130+
131+
// Should have 2 entries
132+
if len(m) != 2 {
133+
t.Errorf("Expected 2 entries, got %d", len(m))
134+
}
135+
}
136+
137+
func TestShouldIgnore(t *testing.T) {
138+
src := `package test
139+
140+
//goroutinectx:ignore
141+
func line3() {}
142+
143+
//goroutinectx:ignore goroutine
144+
func line6() {}
145+
146+
//goroutinectx:ignore errgroup
147+
func line9() {}
148+
`
149+
fset := token.NewFileSet()
150+
file, err := parser.ParseFile(fset, "test.go", src, parser.ParseComments)
151+
if err != nil {
152+
t.Fatalf("Failed to parse: %v", err)
153+
}
154+
155+
m := Build(fset, file)
156+
157+
// Line 3: ignore all -> should ignore goroutine
158+
if !m.ShouldIgnore(3, Goroutine) && !m.ShouldIgnore(4, Goroutine) {
159+
t.Error("Expected line 3-4 to ignore goroutine")
160+
}
161+
162+
// Line 6: ignore goroutine -> should ignore goroutine
163+
if !m.ShouldIgnore(6, Goroutine) && !m.ShouldIgnore(7, Goroutine) {
164+
t.Error("Expected line 6-7 to ignore goroutine")
165+
}
166+
167+
// Line 6: ignore goroutine -> should NOT ignore errgroup
168+
if m.ShouldIgnore(6, Errgroup) || m.ShouldIgnore(7, Errgroup) {
169+
t.Error("Expected line 6-7 to NOT ignore errgroup")
170+
}
171+
172+
// Line 9: ignore errgroup -> should NOT ignore goroutine
173+
if m.ShouldIgnore(9, Goroutine) || m.ShouldIgnore(10, Goroutine) {
174+
t.Error("Expected line 9-10 to NOT ignore goroutine")
175+
}
176+
177+
// Line 100: no comment -> should NOT ignore anything
178+
if m.ShouldIgnore(100, Goroutine) {
179+
t.Error("Expected line 100 to NOT ignore goroutine")
180+
}
181+
}
182+
183+
func TestGetUnusedIgnores(t *testing.T) {
184+
src := `package test
185+
186+
//goroutinectx:ignore
187+
func unusedIgnoreAll() {}
188+
189+
//goroutinectx:ignore goroutine
190+
func unusedIgnoreSpecific() {}
191+
`
192+
fset := token.NewFileSet()
193+
file, err := parser.ParseFile(fset, "test.go", src, parser.ParseComments)
194+
if err != nil {
195+
t.Fatalf("Failed to parse: %v", err)
196+
}
197+
198+
m := Build(fset, file)
199+
200+
// All checkers enabled but not used
201+
enabled := EnabledCheckers{
202+
Goroutine: true,
203+
GoroutineDerive: true,
204+
Waitgroup: true,
205+
Errgroup: true,
206+
Spawner: true,
207+
Spawnerlabel: true,
208+
Gotask: true,
209+
}
210+
211+
unused := m.GetUnusedIgnores(enabled)
212+
213+
// Should have 2 unused ignores
214+
if len(unused) != 2 {
215+
t.Errorf("Expected 2 unused ignores, got %d", len(unused))
216+
}
217+
}
218+
219+
func TestGetUnusedIgnoresWithUsed(t *testing.T) {
220+
fset := token.NewFileSet()
221+
222+
// Create a simple file manually
223+
file := &ast.File{
224+
Comments: []*ast.CommentGroup{
225+
{
226+
List: []*ast.Comment{
227+
{Slash: token.Pos(10), Text: "//goroutinectx:ignore"},
228+
},
229+
},
230+
},
231+
}
232+
233+
// Build manually since we don't have proper position info
234+
m := Build(fset, file)
235+
236+
// Use one of the entries
237+
enabled := EnabledCheckers{Goroutine: true}
238+
line := fset.Position(token.Pos(10)).Line
239+
m.ShouldIgnore(line, Goroutine)
240+
241+
unused := m.GetUnusedIgnores(enabled)
242+
243+
// Should have 0 unused ignores (the one we have was used)
244+
if len(unused) != 0 {
245+
t.Errorf("Expected 0 unused ignores, got %d", len(unused))
246+
}
247+
}

‎testdata/metatest/tests/higherOrderGoFnCallsDeriver.json‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"title": "Higher-order go fn()() - returned func calls deriver.",
3+
"targets": [
4+
"goroutinederive"
5+
],
6+
"level": "goroutinederive",
7+
"variants": {
8+
"good": {
9+
"description": "Factory function returns a func that calls the required deriver.",
10+
"functions": {
11+
"goroutinederive": "goodHigherOrderCallsDeriver"
12+
}
13+
}
14+
}
15+
}

‎testdata/metatest/tests/higherOrderGoFnMissingDeriver.json‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"title": "Higher-order go fn()() - returned func missing deriver.",
3+
"targets": [
4+
"goroutinederive"
5+
],
6+
"level": "goroutinederive",
7+
"variants": {
8+
"bad": {
9+
"description": "Factory function returns a func that does not call the deriver.",
10+
"functions": {
11+
"goroutinederive": "badHigherOrderMissingDeriver"
12+
}
13+
}
14+
}
15+
}

‎testdata/metatest/tests/higherOrderReturnsReassignedVariable.json‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"title": "Higher-order returns reassigned variable - with ctx",
3+
"targets": [
4+
"errgroup",
5+
"goroutine",
6+
"waitgroup"
7+
],
8+
"level": "evil",
9+
"variants": {
10+
"good": {
11+
"description": "Factory function returns a reassigned variable that captures context.",
12+
"functions": {
13+
"errgroup": "goodHigherOrderReturnsReassignedVariableWithCtx",
14+
"goroutine": "goodHigherOrderReturnsReassignedVariableWithCtx",
15+
"waitgroup": "goodHigherOrderReturnsReassignedVariableWithCtx"
16+
}
17+
}
18+
}
19+
}

‎testdata/metatest/tests/higherOrderReturnsVariable.json‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"title": "Higher-order returns variable - with ctx",
3+
"targets": [
4+
"errgroup",
5+
"goroutine",
6+
"waitgroup"
7+
],
8+
"level": "evil",
9+
"variants": {
10+
"good": {
11+
"description": "Factory function returns a variable that captures context.",
12+
"functions": {
13+
"errgroup": "goodHigherOrderReturnsVariableWithCtx",
14+
"goroutine": "goodHigherOrderReturnsVariableWithCtx",
15+
"waitgroup": "goodHigherOrderReturnsVariableWithCtx"
16+
}
17+
}
18+
}
19+
}

‎testdata/metatest/tests/higherOrderReturnsVariableDeriver.json‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"title": "Higher-order returns variable - with deriver.",
3+
"targets": [
4+
"goroutinederive"
5+
],
6+
"level": "goroutinederive",
7+
"variants": {
8+
"good": {
9+
"description": "Factory function returns a variable that calls the deriver.",
10+
"functions": {
11+
"goroutinederive": "goodHigherOrderReturnsVariableWithDeriver"
12+
}
13+
}
14+
}
15+
}

‎testdata/metatest/tests/higherOrderReturnsVariableMissingDeriver.json‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"title": "Higher-order returns variable - missing deriver.",
3+
"targets": [
4+
"goroutinederive"
5+
],
6+
"level": "goroutinederive",
7+
"variants": {
8+
"bad": {
9+
"description": "Factory function returns a variable that does not call the deriver.",
10+
"functions": {
11+
"goroutinederive": "badHigherOrderReturnsVariableMissingDeriver"
12+
}
13+
}
14+
}
15+
}

‎testdata/metatest/tests/higherOrderReturnsVariableWithoutCtx.json‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"title": "Higher-order returns variable - without ctx",
3+
"targets": [
4+
"errgroup",
5+
"goroutine",
6+
"waitgroup"
7+
],
8+
"level": "evil",
9+
"variants": {
10+
"bad": {
11+
"description": "Factory function returns a variable that does not capture context.",
12+
"functions": {
13+
"errgroup": "badHigherOrderReturnsVariableWithoutCtx",
14+
"goroutine": "badHigherOrderReturnsVariableWithoutCtx",
15+
"waitgroup": "badHigherOrderReturnsVariableWithoutCtx"
16+
}
17+
}
18+
}
19+
}

‎testdata/metatest/tests/tripleHigherOrderVariable.json‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"title": "Triple higher-order returns variable - with ctx",
3+
"targets": [
4+
"goroutine"
5+
],
6+
"level": "evil",
7+
"variants": {
8+
"good": {
9+
"description": "Triple higher-order with variable returns at each level.",
10+
"functions": {
11+
"goroutine": "goodTripleHigherOrderVariableWithCtx"
12+
}
13+
}
14+
}
15+
}

0 commit comments

Comments
 (0)