diff --git a/README.md b/README.md index 10aee12..38bd218 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/main.go b/main.go index 90d98e3..b212111 100644 --- a/main.go +++ b/main.go @@ -7,5 +7,5 @@ import ( ) func main() { - singlechecker.Main(paralleltest.NewAnalyzer()) + singlechecker.Main(paralleltest.Analyzer) } diff --git a/pkg/paralleltest/paralleltest.go b/pkg/paralleltest/paralleltest.go index 4783aa2..c7da52a 100644 --- a/pkg/paralleltest/paralleltest.go +++ b/pkg/paralleltest/paralleltest.go @@ -1,6 +1,7 @@ package paralleltest import ( + "flag" "go/ast" "go/types" "strings" @@ -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{ @@ -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) @@ -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 @@ -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) + } } } }) diff --git a/pkg/paralleltest/paralleltest_test.go b/pkg/paralleltest/paralleltest_test.go index cd214ed..eafd03d 100644 --- a/pkg/paralleltest/paralleltest_test.go +++ b/pkg/paralleltest/paralleltest_test.go @@ -1,6 +1,7 @@ package paralleltest import ( + "flag" "os" "path/filepath" "testing" @@ -8,13 +9,32 @@ import ( "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") } diff --git a/pkg/paralleltest/testdata/src/i/i_test.go b/pkg/paralleltest/testdata/src/i/i_test.go new file mode 100644 index 0000000..f476be3 --- /dev/null +++ b/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() +}