Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add flag to ignore missing calls (off by default) #19

Merged
merged 5 commits into from Jun 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Expand Up @@ -10,6 +10,8 @@ The Go linter `paralleltest` checks that the t.Parallel gets called for the test
paralleltest ./...
```

To ignore missing calls to `t.Parallel` and only report incorrect uses of it, pass the flag `-i`.

## Examples

### Missing `t.Parallel()` in the test method
Expand Down
2 changes: 1 addition & 1 deletion main.go
Expand Up @@ -7,5 +7,5 @@ import (
)

func main() {
singlechecker.Main(paralleltest.NewAnalyzer())
singlechecker.Main(paralleltest.Analyzer)
}
47 changes: 32 additions & 15 deletions pkg/paralleltest/paralleltest.go
@@ -1,6 +1,7 @@
package paralleltest

import (
"flag"
"go/ast"
"go/types"
"strings"
Expand All @@ -15,16 +16,28 @@ It also checks that the t.Parallel is used if multiple tests cases are run as pa
As part of ensuring parallel tests works as expected it checks for reinitialising of the range value
over the test cases.(https://tinyurl.com/y6555cy6)`

func NewAnalyzer() *analysis.Analyzer {
return &analysis.Analyzer{
Name: "paralleltest",
Doc: Doc,
Run: run,
Requires: []*analysis.Analyzer{inspect.Analyzer},
}
var Analyzer = &analysis.Analyzer{
Name: "paralleltest",
Doc: Doc,
Run: run,
Flags: flags(),
Requires: []*analysis.Analyzer{inspect.Analyzer},
}

const ignoreMissingFlag = "i"

func flags() flag.FlagSet {
options := flag.NewFlagSet("", flag.ExitOnError)
options.Bool(ignoreMissingFlag, false, "ignore missing calls to t.Parallel")
return *options
}

type boolValue bool

func run(pass *analysis.Pass) (interface{}, error) {

ignoreMissing := pass.Analyzer.Flags.Lookup(ignoreMissingFlag).Value.(flag.Getter).Get().(bool)

inspector := inspector.New(pass.Files)

nodeFilter := []ast.Node{
Expand Down Expand Up @@ -52,12 +65,12 @@ func run(pass *analysis.Pass) (interface{}, error) {

case *ast.ExprStmt:
ast.Inspect(v, func(n ast.Node) bool {
// Check if the test method is calling t.parallel
// Check if the test method is calling t.Parallel
if !funcHasParallelMethod {
funcHasParallelMethod = methodParallelIsCalledInTestFunction(n, testVar)
}

// Check if the t.Run within the test function is calling t.parallel
// Check if the t.Run within the test function is calling t.Parallel
if methodRunIsCalledInTestFunction(n, testVar) {
// n is a call to t.Run; find out the name of the subtest's *testing.T parameter.
innerTestVar := getRunCallbackParameterName(n)
Expand All @@ -77,7 +90,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
return true
})

// Check if the range over testcases is calling t.parallel
// Check if the range over testcases is calling t.Parallel
case *ast.RangeStmt:
rangeNode = v

Expand Down Expand Up @@ -114,22 +127,26 @@ func run(pass *analysis.Pass) (interface{}, error) {
}
}

if !funcHasParallelMethod {
if !ignoreMissing && !funcHasParallelMethod {
pass.Reportf(node.Pos(), "Function %s missing the call to method parallel\n", funcDecl.Name.Name)
}

if rangeStatementOverTestCasesExists && rangeNode != nil {
if !rangeStatementHasParallelMethod {
pass.Reportf(rangeNode.Pos(), "Range statement for test %s missing the call to method parallel in test Run\n", funcDecl.Name.Name)
if !ignoreMissing {
pass.Reportf(rangeNode.Pos(), "Range statement for test %s missing the call to method parallel in test Run\n", funcDecl.Name.Name)
}
} else if loopVariableUsedInRun != nil {
pass.Reportf(rangeNode.Pos(), "Range statement for test %s does not reinitialise the variable %s\n", funcDecl.Name.Name, *loopVariableUsedInRun)
}
}

// Check if the t.Run is more than one as there is no point making one test parallel
if numberOfTestRun > 1 && len(positionOfTestRunNode) > 0 {
for _, n := range positionOfTestRunNode {
pass.Reportf(n.Pos(), "Function %s has missing the call to method parallel in the test run\n", funcDecl.Name.Name)
if !ignoreMissing {
if numberOfTestRun > 1 && len(positionOfTestRunNode) > 0 {
for _, n := range positionOfTestRunNode {
pass.Reportf(n.Pos(), "Function %s has missing the call to method parallel in the test run\n", funcDecl.Name.Name)
}
}
}
})
Expand Down
24 changes: 22 additions & 2 deletions pkg/paralleltest/paralleltest_test.go
@@ -1,20 +1,40 @@
package paralleltest

import (
"flag"
"os"
"path/filepath"
"testing"

"golang.org/x/tools/go/analysis/analysistest"
)

func TestAll(t *testing.T) {
func TestMissing(t *testing.T) {
t.Parallel()

wd, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get wd: %s", err)
}

testdata := filepath.Join(filepath.Dir(wd), "paralleltest", "testdata")
analysistest.Run(t, testdata, NewAnalyzer(), "t")
analysistest.Run(t, testdata, Analyzer, "t")
}

func TestIgnoreMissing(t *testing.T) {
t.Parallel()

wd, err := os.Getwd()
if err != nil {
t.Fatalf("Failed to get wd: %s", err)
}

options := flag.NewFlagSet("", flag.ExitOnError)
options.Bool("i", true, "")

analyzer := *Analyzer
analyzer.Flags = *options

testdata := filepath.Join(filepath.Dir(wd), "paralleltest", "testdata")
analysistest.Run(t, testdata, &analyzer, "i")
}
163 changes: 163 additions & 0 deletions pkg/paralleltest/testdata/src/i/i_test.go
@@ -0,0 +1,163 @@
package t

import (
"fmt"
"testing"
)

func NoATestFunction() {}
func TestingFunctionLooksLikeATestButIsNotWithParam() {}
func TestingFunctionLooksLikeATestButIsWithParam(i int) {}
func AbcFunctionSuccessful(t *testing.T) {}

func TestFunctionSuccessfulRangeTest(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
}{{name: "foo"}}
for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(x *testing.T) {
x.Parallel()
fmt.Println(tc.name)
})
}
}

func TestFunctionSuccessfulNoRangeTest(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
}{{name: "foo"}, {name: "bar"}}

t.Run(testCases[0].name, func(t *testing.T) {
t.Parallel()
fmt.Println(testCases[0].name)
})
t.Run(testCases[1].name, func(t *testing.T) {
t.Parallel()
fmt.Println(testCases[1].name)
})

}

func TestFunctionMissingCallToParallel(t *testing.T) {}
func TestFunctionRangeMissingCallToParallel(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
}{{name: "foo"}}

// this range loop should be okay as it does not have test Run
for _, tc := range testCases {
fmt.Println(tc.name)
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
fmt.Println(tc.name)
})
}
}

func TestFunctionMissingCallToParallelAndRangeNotUsingRangeValueInTDotRun(t *testing.T) {
testCases := []struct {
name string
}{{name: "foo"}}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
fmt.Println(tc.name)
})
}
}

func TestFunctionRangeNotUsingRangeValueInTDotRun(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
}{{name: "foo"}}
for _, tc := range testCases { // want "Range statement for test TestFunctionRangeNotUsingRangeValueInTDotRun does not reinitialise the variable tc"
t.Run("tc.name", func(t *testing.T) {
t.Parallel()
fmt.Println(tc.name)
})
}
}

func TestFunctionRangeNotReInitialisingVariable(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
}{{name: "foo"}}
for _, tc := range testCases { // want "Range statement for test TestFunctionRangeNotReInitialisingVariable does not reinitialise the variable tc"
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
fmt.Println(tc.name)
})
}
}

func TestFunctionTwoTestRunMissingCallToParallel(t *testing.T) {
t.Parallel()

t.Run("1", func(t *testing.T) {
fmt.Println("1")
})
t.Run("2", func(t *testing.T) {
fmt.Println("2")
})
}

func TestFunctionFirstOneTestRunMissingCallToParallel(t *testing.T) {
t.Parallel()

t.Run("1", func(t *testing.T) {
fmt.Println("1")
})
t.Run("2", func(t *testing.T) {
t.Parallel()
fmt.Println("2")
})
}

func TestFunctionSecondOneTestRunMissingCallToParallel(t *testing.T) {
t.Parallel()

t.Run("1", func(x *testing.T) {
x.Parallel()
fmt.Println("1")
})
t.Run("2", func(t *testing.T) {
fmt.Println("2")
})
}

type notATest int

func (notATest) Run(args ...interface{}) {}
func (notATest) Parallel() {}

func TestFunctionWithRunLookalike(t *testing.T) {
t.Parallel()

var other notATest
// These aren't t.Run, so they shouldn't be flagged as Run invocations missing calls to Parallel.
other.Run(1, 1)
other.Run(2, 2)
}

func TestFunctionWithParallelLookalike(t *testing.T) {
var other notATest
// This isn't t.Parallel, so it doesn't qualify as a call to Parallel.
other.Parallel()
}

func TestFunctionWithOtherTestingVar(q *testing.T) {
q.Parallel()
}