Skip to content

Instantly share code, notes, and snippets.

@posener
Last active October 29, 2024 13:01
Show Gist options
  • Save posener/92a55c4cd441fc5e5e85f27bca008721 to your computer and use it in GitHub Desktop.
Save posener/92a55c4cd441fc5e5e85f27bca008721 to your computer and use it in GitHub Desktop.
Be Careful with Table Driven Tests and t.Parallel()

Be Careful with Table Driven Tests and t.Parallel()

We Gophers, love table-driven-tests, it makes our unittesting structured, and makes it easy to add different test cases with ease.

Let’s create our table driven test, for convenience, I chose to use t.Log as the test function. Notice that we don't have any assertion in this test, it is not needed to for the demonstration.

func TestTLog(t *testing.T) {
	t.Parallel()
	tests := []struct {
		name  string
		value int
	}{
		{name: "test 1", value: 1},
		{name: "test 2", value: 2},
		{name: "test 3", value: 3},
		{name: "test 4", value: 4},
	}
	for _, tc := range tests {
		t.Run(tc.name, func(t *testing.T) {
			// Here you test tc.value against a test function.
			// Let's use t.Log as our test function :-)
			t.Log(tc.value)
		})
	}
}

The output of running such a test file will be:

 $ go test -v ./...
=== RUN   TestTLog
=== RUN   TestTLog/test_1
=== RUN   TestTLog/test_2
=== RUN   TestTLog/test_3
=== RUN   TestTLog/test_4
--- PASS: TestTLog (0.00s)
    --- PASS: TestTLog/test_1 (0.00s)
    	test_test.go:19: 1
    --- PASS: TestTLog/test_2 (0.00s)
    	test_test.go:19: 2
    --- PASS: TestTLog/test_3 (0.00s)
    	test_test.go:19: 3
    --- PASS: TestTLog/test_4 (0.00s)
    	test_test.go:19: 4
PASS
ok  	github.com/posener/testparallel	0.002s

As can be seen, go has the t.Run() function, which creates a nested test inside an existing test. This is useful:

  • We can see each test case individually in the output,
  • we can know exactly which test case failed in case of failure
  • and we can run a specific test case using the -run flag: go test ./... -v -run TestTLog/test_4.

Having this separation in a table driven test makes It very tempting to add some more parallelism to our testing. This could be done by adding t.Parallel() to the nested test created by t.Run():

	for _, tc := range tests {
		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
			// Here you test tc.value against a test function.
			// Let's use t.Log as our test function :-)
			t.Log(tc.value)
		})
	}
}

This is the result:

$ go test -v ./...
=== RUN   TestTLog
=== RUN   TestTLog/test_1
=== RUN   TestTLog/test_2
=== RUN   TestTLog/test_3
=== RUN   TestTLog/test_4
--- PASS: TestTLog (0.00s)
    --- PASS: TestTLog/test_1 (0.00s)
    	test_test.go:20: 4
    --- PASS: TestTLog/test_3 (0.00s)
    	test_test.go:20: 4
    --- PASS: TestTLog/test_2 (0.00s)
    	test_test.go:20: 4
    --- PASS: TestTLog/test_4 (0.00s)
    	test_test.go:20: 4
PASS
ok  	github.com/posener/testparallel	0.002s

Easy right? The test passed! well, yes. But we were lucky enough to run it with the -v flag which turns on the logs, and we see that the logs might be a bit surprising.

As it can be seen from the test output: in all the cases, our test function t.Log was called with the same argument: 4 in all 4 test cases. That's not what we wanted, and different than what was tested without the t.Parallel() call.

Notice that even if we did have an assertion in this test, the test run would still pass - but only one of the test cases was checked! Without logging the argument and actually checking the log output of the test we would think that all our test cases were checked and that our function is great. This is very dangerous and could lead to bugs in our code!

What Happened?

An experience Gopher will immediately sense the source for the problem. A less experience one might fight this issue for hours, or, in case that he was lucky and saw the bug when adding the t.Parallel, will just give up and remove the added line.

This is a well known Go gotcha, well hidden inside the go testing framework. This is what happen when using a closure is inside a goroutine. Actually, this bug is so common, that it is the first paragraph in the Go common mistakes guide in the github go wiki. In our case, we have the func(t *testing.T) {...} closure, that is ran in a go routine invoked inside the t.Run() function. When calling t.Parallel() the test sends a signal to its parent test to stop waiting for it, and then the loops continues.

This causing the tc variable to advance to the next tests value, which causes the "Go common mistake" to happen.

How to Solve This?

There are some solutions to this problem. None of them is too elegant. The easiest, maybe not nicest, is to define a new local variable inside the loop that will hide the loop variable.

To make our life easier, and maybe more confusing, we can name it with the same name of the loop variable (Thanks @a8m):

	for _, tc := range tests {
+		tc := tc
		t.Run(tc.name, func(t *testing.T) {
			t.Parallel()
			// Here you test tc.value against a test function.
			// Let's use t.Log as our test function :-)
			t.Log(tc.value)
		})
	}
}

Running the test in the above code will result in the right tests, notice that the order of the output have changed since we are not running the test serially anymore.

$ go test -v ./...
=== RUN   TestTLog
=== RUN   TestTLog/test_1
=== RUN   TestTLog/test_2
=== RUN   TestTLog/test_3
=== RUN   TestTLog/test_4
--- PASS: TestTLog (0.00s)
    --- PASS: TestTLog/test_1 (0.00s)
    	test_test.go:23: 1
    --- PASS: TestTLog/test_2 (0.00s)
    	test_test.go:23: 2
    --- PASS: TestTLog/test_4 (0.00s)
    	test_test.go:23: 4
    --- PASS: TestTLog/test_3 (0.00s)
    	test_test.go:23: 3
PASS
ok  	github.com/posener/testparallel	0.002s

Final Thoughts

I think that this issue can be very dangerous - the coverage of a function can change from full cover to only one test case without even noticing it.

This “common mistake” issue is pretty disturbing and catches often even experienced Go programmers. In my opinion, it is one of the things that we might want to eigher:

  • change in the language,
  • or have a linter for (Maybe in vet, now that is running as part of go test in v1.10).

Thanks for reading

If you find this interesting, please, ✎ comment below or ★ star above ☺ For more stuff: gist.github.com/posener.

Cheers,

Eyal

@tsyber1an
Copy link

tsyber1an commented Sep 4, 2018

you can also wrap into closure func the whole t.Run call and pass tc as a parameter.

@iwata
Copy link

iwata commented Feb 4, 2019

@boogie4eva
Copy link

valuable information .Had this issue myself .

@lkelly93
Copy link

lkelly93 commented Sep 6, 2020

As an inexperienced Gopher this was the answer I was looking for after an hour of pulling my hair out.

@alok87
Copy link

alok87 commented Sep 18, 2020

thanks

@nishantt95
Copy link

👍

@YaoC
Copy link

YaoC commented Jan 8, 2021

👍

@kunwardeep
Copy link

add the linter to avoid such pitfall https://github.com/kunwardeep/paralleltest

@sheldonhull
Copy link

sheldonhull commented Oct 23, 2021

Suggestion, after working through Should Shadowing Package Namespace With Local Variable be Avoided I'd suggest updating the text in the description.

There are some solutions to this problem. None of them is too elegant. The easiest, maybe not nicest, is to define a new local variable inside the loop that will hide the loop variable.

If I'm understanding the behavior better, I think this is actually

There are some solutions to this problem. None of them is too elegant. The easiest, maybe not nicest, is to define a new local variable inside the loop that will hide the loop variable by shadowing in the current scope. This shadows the loop variable in the new scope.

If that's wrong, I'm all ears, as this behavior is something I'm finding outside the normal Go patterns of readability and clear behavior.

edit: updated stack overflow question linked above with references to this gist.

@joseluisr-meli
Copy link

👍

@aswinkm-tc
Copy link

👍

@mgerasimchuk
Copy link

thank you, you save my day

@OJOMB
Copy link

OJOMB commented Jun 24, 2022

nice, thankyou

@mrwormhole
Copy link

mrwormhole commented Sep 20, 2022

amazing reminders 👍 we use tparallel linter rule to ensure correct usage. It is one of those nuances that people can always make mistakes

https://golangci-lint.run/usage/linters/

@hakankoklu
Copy link

Starting in 1.20, the updated go vet also reports this issue.

@mrwormhole
Copy link

mrwormhole commented Jun 25, 2023

apologies, I actually had time to test this with latest go vet, only go vet's loopclosure rules catch this mistake :) amazing, using go vet actually helps juniors with tparallel(teaches them how to use t.Parallel()) linter

@alexbozhenko
Copy link

Note that the problem will go away in Go 1.22: https://github.com/golang/go/wiki/LoopvarExperiment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment