From 95bbe2ece191c0446a881b6637a682c4c3f5034e Mon Sep 17 00:00:00 2001 From: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com> Date: Mon, 13 May 2024 17:46:43 +0530 Subject: [PATCH] Fix flaky tests for the parallel mutator (#1426) ## Changes Around 0.5% to 1% of the time, the tests would fail due to concurrent access to the underlying slice in the mutator. This PR makes the test thread safe preventing race conditions. Example of failed run: https://github.com/databricks/cli/actions/runs/9004657555/job/24738145829 --- bundle/parallel_test.go | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/bundle/parallel_test.go b/bundle/parallel_test.go index be1e3363..dfc7ddac 100644 --- a/bundle/parallel_test.go +++ b/bundle/parallel_test.go @@ -2,6 +2,7 @@ package bundle import ( "context" + "sync" "testing" "github.com/databricks/cli/bundle/config" @@ -10,9 +11,14 @@ import ( ) type addToContainer struct { + t *testing.T container *[]int value int err bool + + // mu is a mutex that protects container. It is used to ensure that the + // container slice is only modified by one goroutine at a time. + mu *sync.Mutex } func (m *addToContainer) Apply(ctx context.Context, b ReadOnlyBundle) diag.Diagnostics { @@ -20,9 +26,10 @@ func (m *addToContainer) Apply(ctx context.Context, b ReadOnlyBundle) diag.Diagn return diag.Errorf("error") } - c := *m.container - c = append(c, m.value) - *m.container = c + m.mu.Lock() + *m.container = append(*m.container, m.value) + m.mu.Unlock() + return nil } @@ -36,9 +43,10 @@ func TestParallelMutatorWork(t *testing.T) { } container := []int{} - m1 := &addToContainer{container: &container, value: 1} - m2 := &addToContainer{container: &container, value: 2} - m3 := &addToContainer{container: &container, value: 3} + var mu sync.Mutex + m1 := &addToContainer{t: t, container: &container, value: 1, mu: &mu} + m2 := &addToContainer{t: t, container: &container, value: 2, mu: &mu} + m3 := &addToContainer{t: t, container: &container, value: 3, mu: &mu} m := Parallel(m1, m2, m3) @@ -57,9 +65,10 @@ func TestParallelMutatorWorkWithErrors(t *testing.T) { } container := []int{} - m1 := &addToContainer{container: &container, value: 1} - m2 := &addToContainer{container: &container, err: true, value: 2} - m3 := &addToContainer{container: &container, value: 3} + var mu sync.Mutex + m1 := &addToContainer{container: &container, value: 1, mu: &mu} + m2 := &addToContainer{container: &container, err: true, value: 2, mu: &mu} + m3 := &addToContainer{container: &container, value: 3, mu: &mu} m := Parallel(m1, m2, m3)