Skip to content

Commit 2ff338a

Browse files
authored
Merge pull request #33 from spencerschrock/bug/setenv-false-positive
dont flag tests which call Setenv directly or whose subtests do
2 parents 1adbbc8 + 7bc464a commit 2ff338a

File tree

2 files changed

+98
-13
lines changed

2 files changed

+98
-13
lines changed

pkg/paralleltest/paralleltest.go

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,10 @@ func (a *parallelAnalyzer) run(pass *analysis.Pass) (interface{}, error) {
5454
inspector.Preorder(nodeFilter, func(node ast.Node) {
5555
funcDecl := node.(*ast.FuncDecl)
5656
var funcHasParallelMethod,
57+
funcCantParallelMethod,
5758
rangeStatementOverTestCasesExists,
58-
rangeStatementHasParallelMethod bool
59+
rangeStatementHasParallelMethod,
60+
rangeStatementCantParallelMethod bool
5961
var loopVariableUsedInRun *string
6062
var numberOfTestRun int
6163
var positionOfTestRunNode []ast.Node
@@ -77,20 +79,29 @@ func (a *parallelAnalyzer) run(pass *analysis.Pass) (interface{}, error) {
7779
funcHasParallelMethod = methodParallelIsCalledInTestFunction(n, testVar)
7880
}
7981

82+
// Check if the test calls t.Setenv, cannot be used in parallel tests or tests with parallel ancestors
83+
if !funcCantParallelMethod {
84+
funcCantParallelMethod = methodSetenvIsCalledInTestFunction(n, testVar)
85+
}
86+
8087
// Check if the t.Run within the test function is calling t.Parallel
8188
if methodRunIsCalledInTestFunction(n, testVar) {
8289
// n is a call to t.Run; find out the name of the subtest's *testing.T parameter.
8390
innerTestVar := getRunCallbackParameterName(n)
8491

8592
hasParallel := false
93+
cantParallel := false
8694
numberOfTestRun++
8795
ast.Inspect(v, func(p ast.Node) bool {
8896
if !hasParallel {
8997
hasParallel = methodParallelIsCalledInTestFunction(p, innerTestVar)
9098
}
99+
if !cantParallel {
100+
cantParallel = methodSetenvIsCalledInTestFunction(p, innerTestVar)
101+
}
91102
return true
92103
})
93-
if !hasParallel {
104+
if !hasParallel && !cantParallel {
94105
positionOfTestRunNode = append(positionOfTestRunNode, n)
95106
}
96107
}
@@ -122,6 +133,10 @@ func (a *parallelAnalyzer) run(pass *analysis.Pass) (interface{}, error) {
122133
rangeStatementHasParallelMethod = methodParallelIsCalledInMethodRun(r.X, innerTestVar)
123134
}
124135

136+
if !rangeStatementCantParallelMethod {
137+
rangeStatementCantParallelMethod = methodSetenvIsCalledInMethodRun(r.X, innerTestVar)
138+
}
139+
125140
if loopVariableUsedInRun == nil {
126141
if run, ok := r.X.(*ast.CallExpr); ok {
127142
loopVariableUsedInRun = loopVarReferencedInRun(run, loopVars, pass.TypesInfo)
@@ -134,12 +149,17 @@ func (a *parallelAnalyzer) run(pass *analysis.Pass) (interface{}, error) {
134149
}
135150
}
136151

137-
if !a.ignoreMissing && !funcHasParallelMethod {
152+
// Descendents which call Setenv, also prevent tests from calling Parallel
153+
if rangeStatementCantParallelMethod {
154+
funcCantParallelMethod = true
155+
}
156+
157+
if !a.ignoreMissing && !funcHasParallelMethod && !funcCantParallelMethod {
138158
pass.Reportf(node.Pos(), "Function %s missing the call to method parallel\n", funcDecl.Name.Name)
139159
}
140160

141161
if rangeStatementOverTestCasesExists && rangeNode != nil {
142-
if !rangeStatementHasParallelMethod {
162+
if !rangeStatementHasParallelMethod && !rangeStatementCantParallelMethod {
143163
if !a.ignoreMissing && !a.ignoreMissingSubtests {
144164
pass.Reportf(rangeNode.Pos(), "Range statement for test %s missing the call to method parallel in test Run\n", funcDecl.Name.Name)
145165
}
@@ -162,27 +182,31 @@ func (a *parallelAnalyzer) run(pass *analysis.Pass) (interface{}, error) {
162182
}
163183

164184
func methodParallelIsCalledInMethodRun(node ast.Node, testVar string) bool {
165-
var methodParallelCalled bool
185+
return targetMethodIsCalledInMethodRun(node, testVar, "Parallel")
186+
}
187+
188+
func methodSetenvIsCalledInMethodRun(node ast.Node, testVar string) bool {
189+
return targetMethodIsCalledInMethodRun(node, testVar, "Setenv")
190+
}
191+
192+
func targetMethodIsCalledInMethodRun(node ast.Node, testVar, targetMethod string) bool {
193+
var called bool
166194
// nolint: gocritic
167195
switch callExp := node.(type) {
168196
case *ast.CallExpr:
169197
for _, arg := range callExp.Args {
170-
if !methodParallelCalled {
198+
if !called {
171199
ast.Inspect(arg, func(n ast.Node) bool {
172-
if !methodParallelCalled {
173-
methodParallelCalled = methodParallelIsCalledInRunMethod(n, testVar)
200+
if !called {
201+
called = exprCallHasMethod(n, testVar, targetMethod)
174202
return true
175203
}
176204
return false
177205
})
178206
}
179207
}
180208
}
181-
return methodParallelCalled
182-
}
183-
184-
func methodParallelIsCalledInRunMethod(node ast.Node, testVar string) bool {
185-
return exprCallHasMethod(node, testVar, "Parallel")
209+
return called
186210
}
187211

188212
func methodParallelIsCalledInTestFunction(node ast.Node, testVar string) bool {
@@ -196,6 +220,11 @@ func methodRunIsCalledInRangeStatement(node ast.Node, testVar string) bool {
196220
func methodRunIsCalledInTestFunction(node ast.Node, testVar string) bool {
197221
return exprCallHasMethod(node, testVar, "Run")
198222
}
223+
224+
func methodSetenvIsCalledInTestFunction(node ast.Node, testVar string) bool {
225+
return exprCallHasMethod(node, testVar, "Setenv")
226+
}
227+
199228
func exprCallHasMethod(node ast.Node, receiverName, methodName string) bool {
200229
// nolint: gocritic
201230
switch n := node.(type) {

pkg/paralleltest/testdata/src/t/t_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ type notATest int
142142

143143
func (notATest) Run(args ...interface{}) {}
144144
func (notATest) Parallel() {}
145+
func (notATest) Setenv(_, _ string) {}
145146

146147
func TestFunctionWithRunLookalike(t *testing.T) {
147148
t.Parallel()
@@ -161,3 +162,58 @@ func TestFunctionWithParallelLookalike(t *testing.T) { // want "Function TestFun
161162
func TestFunctionWithOtherTestingVar(q *testing.T) {
162163
q.Parallel()
163164
}
165+
166+
func TestFunctionWithSetenv(t *testing.T) {
167+
// unable to call t.Parallel with t.Setenv
168+
t.Setenv("foo", "bar")
169+
}
170+
171+
func TestFunctionWithSetenvLookalike(t *testing.T) { // want "Function TestFunctionWithSetenvLookalike missing the call to method parallel"
172+
var other notATest
173+
other.Setenv("foo", "bar")
174+
}
175+
176+
func TestFunctionWithSetenvChild(t *testing.T) {
177+
// ancestor of setenv cant call t.Parallel
178+
t.Run("1", func(t *testing.T) {
179+
// unable to call t.Parallel with t.Setenv
180+
t.Setenv("foo", "bar")
181+
fmt.Println("1")
182+
})
183+
}
184+
185+
func TestFunctionSetenvChildrenCanBeParallel(t *testing.T) {
186+
// unable to call t.Parallel with t.Setenv
187+
t.Setenv("foo", "bar")
188+
t.Run("1", func(t *testing.T) { // want "Function TestFunctionSetenvChildrenCanBeParallel missing the call to method parallel in the test run"
189+
fmt.Println("1")
190+
})
191+
t.Run("2", func(t *testing.T) { // want "Function TestFunctionSetenvChildrenCanBeParallel missing the call to method parallel in the test run"
192+
fmt.Println("2")
193+
})
194+
}
195+
196+
func TestFunctionRunWithSetenvSibling(t *testing.T) {
197+
// ancestor of setenv cant call t.Parallel
198+
t.Run("1", func(t *testing.T) {
199+
// unable to call t.Parallel with t.Setenv
200+
t.Setenv("foo", "bar")
201+
fmt.Println("1")
202+
})
203+
t.Run("2", func(t *testing.T) { // want "Function TestFunctionRunWithSetenvSibling missing the call to method parallel in the test run"
204+
fmt.Println("2")
205+
})
206+
}
207+
208+
func TestFunctionWithSetenvRange(t *testing.T) {
209+
// ancestor of setenv cant call t.Parallel
210+
testCases := []struct {
211+
name string
212+
}{{name: "foo"}}
213+
for _, tc := range testCases {
214+
t.Run(tc.name, func(t *testing.T) {
215+
// unable to call t.Parallel with t.Setenv
216+
t.Setenv("foo", "bar")
217+
})
218+
}
219+
}

0 commit comments

Comments
 (0)