Skip to content

Commit

Permalink
tests: fix potential races with t.Parallel and loops ➿
Browse files Browse the repository at this point in the history
Following the CommonMistake[1] in Go, for loop and go routines can be
a big racey problem. This happens quite easily when using `t.Parallel`
in for loop with tests.

This fixes possible races by adding a "shadow" var that is evaluated
at each iteration and placed on the stack for the goroutine, so each
slice element is available to the goroutine when it is eventually
executed.

[1]: https://github.com/golang/go/wiki/CommonMistakes#using-goroutines-on-loop-iterator-variables

Signed-off-by: Vincent Demeester <[email protected]>
  • Loading branch information
vdemeester committed Nov 4, 2020
1 parent 7b5b2fa commit 3016975
Show file tree
Hide file tree
Showing 8 changed files with 12 additions and 0 deletions.
1 change: 1 addition & 0 deletions pkg/reconciler/pipelinerun/resources/apply_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ func TestApplyParameters(t *testing.T) {
}},
},
}} {
tt := tt // capture range variable
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
run := &v1beta1.PipelineRun{
Expand Down
1 change: 1 addition & 0 deletions test/cancel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func TestTaskRunPipelineRunCancel(t *testing.T) {
// on failure, to ensure that cancelling the PipelineRun doesn't cause
// the retrying TaskRun to retry.
for _, numRetries := range []int{0, 1} {
numRetries := numRetries // capture range variable
t.Run(fmt.Sprintf("retries=%d", numRetries), func(t *testing.T) {
ctx := context.Background()
ctx, cancel := context.WithCancel(ctx)
Expand Down
1 change: 1 addition & 0 deletions test/examples_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ func TestExamples(t *testing.T) {

t.Parallel()
for _, path := range getExamplePaths(t, baseDir) {
path := path // capture range variable
testName := extractTestName(baseDir, path)
waitValidateFunc := waitValidatePipelineRunDone
kind := "pipelinerun"
Expand Down
2 changes: 2 additions & 0 deletions test/git_checkout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ func TestGitPipelineRun(t *testing.T) {
repo: "https://github.com/spring-projects/spring-petclinic",
revision: "main",
}} {
tc := tc // capture range variable
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
ctx := context.Background()
Expand Down Expand Up @@ -182,6 +183,7 @@ func TestGitPipelineRunFail(t *testing.T) {
name: "invalid httpsproxy",
httpsproxy: "invalid.https.proxy.example.com",
}} {
tc := tc // capture range variable
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
ctx := context.Background()
Expand Down
2 changes: 2 additions & 0 deletions test/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ func TestPipelineRun(t *testing.T) {
}}

for i, td := range tds {
i := i // capture range variable
td := td // capture range variable
t.Run(td.name, func(t *testing.T) {
td := td

Expand Down
1 change: 1 addition & 0 deletions test/v1alpha1/cancel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func TestTaskRunPipelineRunCancel(t *testing.T) {
// on failure, to ensure that cancelling the PipelineRun doesn't cause
// the retrying TaskRun to retry.
for _, numRetries := range []int{0, 1} {
numRetries := numRetries // capture range variable
t.Run(fmt.Sprintf("retries=%d", numRetries), func(t *testing.T) {
ctx := context.Background()
ctx, cancel := context.WithCancel(ctx)
Expand Down
2 changes: 2 additions & 0 deletions test/v1alpha1/git_checkout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ func TestGitPipelineRun(t *testing.T) {
repo: "https://github.com/spring-projects/spring-petclinic",
revision: "main",
}} {
tc := tc // capture range variable
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
ctx := context.Background()
Expand Down Expand Up @@ -180,6 +181,7 @@ func TestGitPipelineRunFail(t *testing.T) {
name: "invalid httpsproxy",
httpsproxy: "invalid.https.proxy.example.com",
}} {
tc := tc // capture range variable
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
ctx := context.Background()
Expand Down
2 changes: 2 additions & 0 deletions test/v1alpha1/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ func TestPipelineRun(t *testing.T) {
}}

for i, td := range tds {
i := i // capture range variable
td := td // capture range variable
t.Run(td.name, func(t *testing.T) {
td := td
t.Parallel()
Expand Down

0 comments on commit 3016975

Please sign in to comment.