From 0b4c0d923705e7f635879b80739b4bfd3d134def Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Wed, 4 Nov 2020 16:52:00 +0100 Subject: [PATCH] =?UTF-8?q?tests:=20fix=20potential=20races=20with=20t.Par?= =?UTF-8?q?allel=20and=20loops=20=E2=9E=BF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- pkg/reconciler/pipelinerun/resources/apply_test.go | 1 + test/cancel_test.go | 1 + test/examples_test.go | 1 + test/git_checkout_test.go | 2 ++ test/pipelinerun_test.go | 4 ++-- test/v1alpha1/cancel_test.go | 1 + test/v1alpha1/git_checkout_test.go | 2 ++ test/v1alpha1/pipelinerun_test.go | 3 ++- 8 files changed, 12 insertions(+), 3 deletions(-) diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index 283f0ab6bb0..c27cae71fce 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -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{ diff --git a/test/cancel_test.go b/test/cancel_test.go index 331aef2afdb..6b0b189704a 100644 --- a/test/cancel_test.go +++ b/test/cancel_test.go @@ -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) diff --git a/test/examples_test.go b/test/examples_test.go index 41fbfbb457d..e2549e29fcd 100644 --- a/test/examples_test.go +++ b/test/examples_test.go @@ -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" diff --git a/test/git_checkout_test.go b/test/git_checkout_test.go index 57de9993be3..47037fabc08 100644 --- a/test/git_checkout_test.go +++ b/test/git_checkout_test.go @@ -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() @@ -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() diff --git a/test/pipelinerun_test.go b/test/pipelinerun_test.go index a684432ac86..bd25433163c 100644 --- a/test/pipelinerun_test.go +++ b/test/pipelinerun_test.go @@ -207,9 +207,9 @@ 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() ctx := context.Background() ctx, cancel := context.WithCancel(ctx) diff --git a/test/v1alpha1/cancel_test.go b/test/v1alpha1/cancel_test.go index 69ae54270a5..00b9bb03a81 100644 --- a/test/v1alpha1/cancel_test.go +++ b/test/v1alpha1/cancel_test.go @@ -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) diff --git a/test/v1alpha1/git_checkout_test.go b/test/v1alpha1/git_checkout_test.go index 13952dddf97..631d89da1c1 100644 --- a/test/v1alpha1/git_checkout_test.go +++ b/test/v1alpha1/git_checkout_test.go @@ -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() @@ -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() diff --git a/test/v1alpha1/pipelinerun_test.go b/test/v1alpha1/pipelinerun_test.go index 3eee0dbaf08..7a19822c5e7 100644 --- a/test/v1alpha1/pipelinerun_test.go +++ b/test/v1alpha1/pipelinerun_test.go @@ -145,8 +145,9 @@ 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() ctx := context.Background() ctx, cancel := context.WithCancel(ctx)