Skip to content
This repository was archived by the owner on Mar 11, 2021. It is now read-only.

Commit 5205666

Browse files
authored
capture range variable for parallel testing (#2346)
Before only the last range variable was being used and some tests just passed when the shouldn't have. See also https://gist.github.com/posener/92a55c4cd441fc5e5e85f27bca008721. And https://golang.org/pkg/testing/#hdr-Subtests_and_Sub_benchmarks for capturing range variables. Thanks to @michaelkleinhenz for finding this and @jarifibrahim for finding the solution which is well known but in this case it is just too easy to miss.
1 parent 829eb51 commit 5205666

File tree

4 files changed

+26
-10
lines changed

4 files changed

+26
-10
lines changed

workitem/enum_type_blackbox_test.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ func TestEnumType_GetDefaultValue(t *testing.T) {
114114
}, 222},
115115
}
116116
for _, tt := range tests {
117+
tt := tt
117118
t.Run(tt.name, func(t *testing.T) {
118119
t.Parallel()
119120
require.Equal(t, tt.expectedOutput, tt.enum.GetDefaultValue())
@@ -139,7 +140,7 @@ func TestEnumType_SetDefaultValue(t *testing.T) {
139140
Values: []interface{}{"first", "second", "third"},
140141
},
141142
"second",
142-
&w.EnumType{
143+
w.EnumType{
143144
SimpleType: w.SimpleType{Kind: w.KindEnum},
144145
BaseType: w.SimpleType{Kind: w.KindString},
145146
Values: []interface{}{"first", "second", "third"},
@@ -153,7 +154,7 @@ func TestEnumType_SetDefaultValue(t *testing.T) {
153154
Values: []interface{}{"first", "second", "third"},
154155
},
155156
nil,
156-
&w.EnumType{
157+
w.EnumType{
157158
SimpleType: w.SimpleType{Kind: w.KindEnum},
158159
BaseType: w.SimpleType{Kind: w.KindString},
159160
Values: []interface{}{"first", "second", "third"},
@@ -180,6 +181,7 @@ func TestEnumType_SetDefaultValue(t *testing.T) {
180181
true},
181182
}
182183
for _, tt := range tests {
184+
tt := tt // capture range variable
183185
t.Run(tt.name, func(t *testing.T) {
184186
t.Parallel()
185187
output, err := tt.enum.SetDefaultValue(tt.defVal)
@@ -272,6 +274,7 @@ func TestEnumType_Validate(t *testing.T) {
272274
}, true},
273275
}
274276
for _, tt := range tests {
277+
tt := tt
275278
t.Run(tt.name, func(t *testing.T) {
276279
t.Parallel()
277280
err := tt.obj.Validate()
@@ -407,8 +410,10 @@ func TestEnumType_ConvertFromModel(t *testing.T) {
407410
},
408411
}
409412
for _, test := range tests {
413+
test := test
410414
t.Run(test.name, func(t *testing.T) {
411415
for _, subtt := range test.data {
416+
subtt := subtt
412417
t.Run(subtt.subTestName, func(tt *testing.T) {
413418
val, err := test.enum.ConvertFromModel(subtt.input)
414419
if subtt.wantErr {

workitem/link/topology_blackbox_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ func TestTopology_String(t *testing.T) {
1717
link.TopologyTree: "tree",
1818
}
1919
for in, out := range inOut {
20+
in := in
21+
out := out
2022
t.Run(in.String(), func(t *testing.T) {
2123
t.Parallel()
2224
require.Equal(t, out, in.String())
@@ -62,6 +64,8 @@ func TestTopology_Scan(t *testing.T) {
6264
}
6365

6466
for i, td := range testDataArr {
67+
i := i
68+
td := td
6569
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
6670
t.Parallel()
6771
var l link.Topology
@@ -87,6 +91,8 @@ func TestTopology_CheckValid(t *testing.T) {
8791
link.Topology("foo"): true,
8892
}
8993
for topo, expectError := range expectErrorArr {
94+
topo := topo
95+
expectError := expectError
9096
t.Run(topo.String(), func(t *testing.T) {
9197
t.Parallel()
9298
err := topo.CheckValid()

workitem/list_type_blackbox_test.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ func TestListType_SetDefaultValue(t *testing.T) {
133133

134134
tests := []struct {
135135
name string
136-
enum ListType
136+
listType ListType
137137
defVal interface{}
138138
expectedOutput FieldType
139139
wantErr bool
@@ -143,11 +143,11 @@ func TestListType_SetDefaultValue(t *testing.T) {
143143
SimpleType: SimpleType{Kind: KindList},
144144
ComponentType: SimpleType{Kind: KindString},
145145
},
146-
[]interface{}{"second"},
147-
&ListType{
146+
"second",
147+
ListType{
148148
SimpleType: SimpleType{Kind: KindList},
149149
ComponentType: SimpleType{Kind: KindString},
150-
DefaultValue: []interface{}{"second"},
150+
DefaultValue: "second",
151151
},
152152
false},
153153
{"set default to nil",
@@ -156,7 +156,7 @@ func TestListType_SetDefaultValue(t *testing.T) {
156156
ComponentType: SimpleType{Kind: KindString},
157157
},
158158
nil,
159-
&ListType{
159+
ListType{
160160
SimpleType: SimpleType{Kind: KindList},
161161
ComponentType: SimpleType{Kind: KindString},
162162
DefaultValue: nil,
@@ -171,10 +171,12 @@ func TestListType_SetDefaultValue(t *testing.T) {
171171
nil,
172172
true},
173173
}
174+
174175
for _, tt := range tests {
176+
tt := tt // capture range variable
175177
t.Run(tt.name, func(t *testing.T) {
176178
t.Parallel()
177-
output, err := tt.enum.SetDefaultValue(tt.defVal)
179+
output, err := tt.listType.SetDefaultValue(tt.defVal)
178180
if tt.wantErr {
179181
require.Error(t, err)
180182
} else {

workitem/simple_type_blackbox_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ func TestSimpleType_Validate(t *testing.T) {
5252
{"invalid kind (list)", SimpleType{Kind: KindList, DefaultValue: "foo"}, true},
5353
}
5454
for _, tt := range tests {
55+
tt := tt // capture range variable
5556
t.Run(tt.name, func(t *testing.T) {
5657
t.Parallel()
5758
err := tt.obj.Validate()
@@ -77,6 +78,7 @@ func TestSimpleType_GetDefault(t *testing.T) {
7778
{"ok - string field nil default", SimpleType{Kind: KindString}, nil},
7879
}
7980
for _, tt := range tests {
81+
tt := tt // capture range variable
8082
t.Run(tt.name, func(t *testing.T) {
8183
t.Parallel()
8284
require.Equal(t, tt.output, tt.obj.GetDefaultValue())
@@ -98,12 +100,12 @@ func TestSimpleType_SetDefaultValue(t *testing.T) {
98100
{"set default to allowed value",
99101
SimpleType{Kind: KindString},
100102
"foo",
101-
&SimpleType{Kind: KindString, DefaultValue: "foo"},
103+
SimpleType{Kind: KindString, DefaultValue: "foo"},
102104
false},
103105
{"set default to nil",
104106
SimpleType{Kind: KindString},
105107
nil,
106-
&SimpleType{Kind: KindString, DefaultValue: nil},
108+
SimpleType{Kind: KindString, DefaultValue: nil},
107109
false},
108110
{"set default to not-allowed value",
109111
SimpleType{Kind: KindString},
@@ -112,6 +114,7 @@ func TestSimpleType_SetDefaultValue(t *testing.T) {
112114
true},
113115
}
114116
for _, tt := range tests {
117+
tt := tt // capture range variable
115118
t.Run(tt.name, func(t *testing.T) {
116119
t.Parallel()
117120
output, err := tt.enum.SetDefaultValue(tt.defVal)

0 commit comments

Comments
 (0)