there is an common mistake in our test.
problem
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(tc.name, func(t *testing.T) {
// uncomment below line will make 4 tests pass
//t.Parallel()
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
reason
the reason is explained well in Go common mistakes guide, 2 reason:
- In Go, the loop iterator variable is a single variable that takes different values in each loop iteration
- 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{}) {
fmt.Println(val)
}(val)
}
- 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 https://github.com/kunwardeep/paralleltest
what i did
i already fix the exists of this type error in https://github.com/pingcap/tidb/pull/27939, but it is possible similiar bug continue to inject.
todo
- 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?
