NOTICE: common mistakes with table driven tests and t.Parallel()

there is an common mistake in our test.


func TestWithParallel(t *testing.T) {
	tests := []struct {
		name  string
		value int
		{name: "test 1", value: 4},
		{name: "test 2", value: 4},
		{name: "test 3", value: 4},
		{name: "test 4", value: 4},
	for i, tc := range tests {
		t.Run(, func(t *testing.T) {
                         // uncomment below line will make 4 tests pass
			fmt.Printf("%+v == %+v \n", i+1, tc.value)
			require.Equal(t, i+1, tc.value)

for above table driven tests, test1, test2, test3 should fail, test 4 should pass, it works as expect.
but if we add t.Parallel() by uncomment the line, 4 test will pass, and the output is:

4 == 4 
4 == 4 
4 == 4 
4 == 4 


the reason is explained well in Go common mistakes guide, 2 reason:

  1. In Go, the loop iterator variable is a single variable that takes different values in each loop iteration
  2. there is a very good chance that when you run this code you will see the last element printed for every iteration instead of each value in sequence, because the goroutines will probably not begin executing until after the loop

how to resolve

  • adding val as a parameter to the closure
for _, val := range values {
	go func(val interface{}) {
  • Copy iterator variable into a new variable
 for i := 0; i < 3; i++ {
+	i := i // Copy i into a new variable.
 	out = append(out, &i)

also, we may add linter to avoid such pitfall

what i did

i already fix the exists of this type error in, but it is possible similiar bug continue to inject.


  • in my branch which is 3 days ago the occurance is 11, in master branch tody the the occurance is 14.this means this is an common mistakes not realized by our community, so i post it here to let more people pay attention to it.
  • maybe we’d better add the linter to ci?

Thanks for starting this thread @feitian124 ! I believe we should think of enabling the correspodning linter.

Do you have an idea on which linter can guide this pattern? We use golangci-lint for tidb mainly. Does it already have a rule for it?

Here it seems to suggest the issue should be detected by go vet

golangci-lint has scopelint, but it is deprecated and replaced by looppointer (with some false-positives) and exportloopref (with some lints ignored).

golangci-lint includes exportloopref but not looppointer. However, exportloopref cannot find the problem here and scopelint has many false-positives.

1 Like

go vet can only detect loop variable i captured by go func() { i }(), not including

  • capture pointer out = append(out, &i)
  • parallel test

i used

it did not have much star but seems have a nice test coverage and works well.

by the way, it takes about 10 seconds linter the whole repo on my pc, 16 core + 16G + ssd,
but the first time lint used a much longer time, not sure why.

we are using Golint? seems it is deprecated and suggest use staticcheck and go vet.

Golint is deprecated and frozen. There’s no drop-in replacement for it, but tools such as Staticcheck and go vet should be used instead.

golangci/golangci-lint != golang/lint :smile:

:smile: thanks for correct me.

So do you have a plan to fix it by enabling a linter?

not yet, need research how to enable a linter, and which linter.
this must have influence on ci, who is responsible for this and i can call for help?

I think you can reach out to @disksing @tiancaiamao and @zhouqiang-cl .