dont flag tests which call Setenv directly or whose subtests do

[spencerschrock]

Setenv cannot be used in parallel tests or tests with parallel ancestors.

This attempts to fix the issue raised in #13 (comment)

[spencerschrock]

@mitar would you mind taking a first look at the added tests to see what's now being flagged/not-flagged, and if it addresses your comment?

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

Lines 166 to 219 in 7bc464a

	func TestFunctionWithSetenv(t *testing.T) {
	// unable to call t.Parallel with t.Setenv
	t.Setenv("foo", "bar")
	}
	func TestFunctionWithSetenvLookalike(t *testing.T) { // want "Function TestFunctionWithSetenvLookalike missing the call to method parallel"
	var other notATest
	other.Setenv("foo", "bar")
	}
	func TestFunctionWithSetenvChild(t *testing.T) {
	// ancestor of setenv cant call t.Parallel
	t.Run("1", func(t *testing.T) {
	// unable to call t.Parallel with t.Setenv
	t.Setenv("foo", "bar")
	fmt.Println("1")
	})
	}
	func TestFunctionSetenvChildrenCanBeParallel(t *testing.T) {
	// unable to call t.Parallel with t.Setenv
	t.Setenv("foo", "bar")
	t.Run("1", func(t *testing.T) { // want "Function TestFunctionSetenvChildrenCanBeParallel missing the call to method parallel in the test run"
	fmt.Println("1")
	})
	t.Run("2", func(t *testing.T) { // want "Function TestFunctionSetenvChildrenCanBeParallel missing the call to method parallel in the test run"
	fmt.Println("2")
	})
	}
	func TestFunctionRunWithSetenvSibling(t *testing.T) {
	// ancestor of setenv cant call t.Parallel
	t.Run("1", func(t *testing.T) {
	// unable to call t.Parallel with t.Setenv
	t.Setenv("foo", "bar")
	fmt.Println("1")
	})
	t.Run("2", func(t *testing.T) { // want "Function TestFunctionRunWithSetenvSibling missing the call to method parallel in the test run"
	fmt.Println("2")
	})
	}
	func TestFunctionWithSetenvRange(t *testing.T) {
	// ancestor of setenv cant call t.Parallel
	testCases := []struct {
	name string
	}{{name: "foo"}}
	for _, tc := range testCases {
	t.Run(tc.name, func(t *testing.T) {
	// unable to call t.Parallel with t.Setenv
	t.Setenv("foo", "bar")
	})
	}
	}

[mitar]

Thanks. This looks great.

Maybe add some test cases which use a test helper (function which calls t.Helper()) and in it it calls t.Setenv. Then test case should still not be flagged for missing t.Parallel.

[spencerschrock]

Maybe add some test cases which use a test helper (function which calls t.Helper()) and in it it calls t.Setenv. Then test case should still not be flagged for missing t.Parallel.

Ah that's a great catch, though it's a harder case to handle with how the rest of the analyzer is written. I don't think I'll try to implement it in this PR, it would involve larger changes to the analyzer. My initial thoughts involve some sort of pre-pass to identify test helpers which call Setenv, and then a call analysis for reachability.

[PhilThurston]

I like this simple approach implemented here. We are having the same issues where if we use t.Setenv() along with t.Parallel() we get a panic yet the linter is telling us we are wong.

[kunwardeep]

Sorry folks, I was away on holiday. I will take a look at this today