chore: Fix data race detection failures in application tests (#21271)

* chore: Fix race detection failures in application tests Signed-off-by: eadred <[email protected]> * Fix failing TestGetCachedAppState tests Signed-off-by: eadred <[email protected]> --------- Signed-off-by: eadred <[email protected]>
argoproj · Dec 20, 2024 · 12a4dab · 12a4dab
1 parent 8841b0d
commit 12a4dab
Showing 1 changed file with 35 additions and 49 deletions.
diff --git a/server/application/application_test.go b/server/application/application_test.go
@@ -48,7 +48,6 @@ import (
 	appinformer "github.com/argoproj/argo-cd/v2/pkg/client/informers/externalversions"
 	"github.com/argoproj/argo-cd/v2/reposerver/apiclient"
 	"github.com/argoproj/argo-cd/v2/reposerver/apiclient/mocks"
-	appmocks "github.com/argoproj/argo-cd/v2/server/application/mocks"
 	servercache "github.com/argoproj/argo-cd/v2/server/cache"
 	"github.com/argoproj/argo-cd/v2/server/rbacpolicy"
 	"github.com/argoproj/argo-cd/v2/test"
@@ -71,6 +70,35 @@ const (
 
 var testEnableEventList []string = argo.DefaultEnableEventList()
 
+type broadcasterMock struct {
+	objects []runtime.Object
+}
+
+func (b broadcasterMock) Subscribe(ch chan *appv1.ApplicationWatchEvent, filters ...func(event *appv1.ApplicationWatchEvent) bool) func() {
+	// Simulate the broadcaster notifying the subscriber of an application update.
+	// The second parameter to Subscribe is filters. For the purposes of tests, we ignore the filters. Future tests
+	// might require implementing those.
+	go func() {
+		for _, obj := range b.objects {
+			app, ok := obj.(*appsv1.Application)
+			if ok {
+				oldVersion, err := strconv.Atoi(app.ResourceVersion)
+				if err != nil {
+					oldVersion = 0
+				}
+				clonedApp := app.DeepCopy()
+				clonedApp.ResourceVersion = strconv.Itoa(oldVersion + 1)
+				ch <- &appsv1.ApplicationWatchEvent{Type: watch.Added, Application: *clonedApp}
+			}
+		}
+	}()
+	return func() {}
+}
+
+func (broadcasterMock) OnAdd(interface{}, bool)           {}
+func (broadcasterMock) OnUpdate(interface{}, interface{}) {}
+func (broadcasterMock) OnDelete(interface{})              {}
+
 func fakeRepo() *appsv1.Repository {
 	return &appsv1.Repository{
 		Repo: fakeRepoURL,
@@ -227,30 +255,9 @@ func newTestAppServerWithEnforcerConfigure(t *testing.T, f func(*rbac.Enforcer),
 		panic("Timed out waiting for caches to sync")
 	}
 
-	broadcaster := new(appmocks.Broadcaster)
-	broadcaster.On("Subscribe", mock.Anything, mock.Anything).Return(func() {}).Run(func(args mock.Arguments) {
-		// Simulate the broadcaster notifying the subscriber of an application update.
-		// The second parameter to Subscribe is filters. For the purposes of tests, we ignore the filters. Future tests
-		// might require implementing those.
-		go func() {
-			events := args.Get(0).(chan *appsv1.ApplicationWatchEvent)
-			for _, obj := range objects {
-				app, ok := obj.(*appsv1.Application)
-				if ok {
-					oldVersion, err := strconv.Atoi(app.ResourceVersion)
-					if err != nil {
-						oldVersion = 0
-					}
-					clonedApp := app.DeepCopy()
-					clonedApp.ResourceVersion = strconv.Itoa(oldVersion + 1)
-					events <- &appsv1.ApplicationWatchEvent{Type: watch.Added, Application: *clonedApp}
-				}
-			}
-		}()
-	})
-	broadcaster.On("OnAdd", mock.Anything, mock.Anything).Return()
-	broadcaster.On("OnUpdate", mock.Anything, mock.Anything).Return()
-	broadcaster.On("OnDelete", mock.Anything).Return()
+	broadcaster := broadcasterMock{
+		objects: objects,
+	}
 
 	appStateCache := appstate.NewCache(cache.NewCache(cache.NewInMemoryCache(time.Hour)), time.Hour)
 	// pre-populate the app cache
@@ -410,30 +417,9 @@ func newTestAppServerWithEnforcerConfigureWithBenchmark(b *testing.B, f func(*rb
 		panic("Timed out waiting for caches to sync")
 	}
 
-	broadcaster := new(appmocks.Broadcaster)
-	broadcaster.On("Subscribe", mock.Anything, mock.Anything).Return(func() {}).Run(func(args mock.Arguments) {
-		// Simulate the broadcaster notifying the subscriber of an application update.
-		// The second parameter to Subscribe is filters. For the purposes of tests, we ignore the filters. Future tests
-		// might require implementing those.
-		go func() {
-			events := args.Get(0).(chan *appsv1.ApplicationWatchEvent)
-			for _, obj := range objects {
-				app, ok := obj.(*appsv1.Application)
-				if ok {
-					oldVersion, err := strconv.Atoi(app.ResourceVersion)
-					if err != nil {
-						oldVersion = 0
-					}
-					clonedApp := app.DeepCopy()
-					clonedApp.ResourceVersion = strconv.Itoa(oldVersion + 1)
-					events <- &appsv1.ApplicationWatchEvent{Type: watch.Added, Application: *clonedApp}
-				}
-			}
-		}()
-	})
-	broadcaster.On("OnAdd", mock.Anything, mock.Anything).Return()
-	broadcaster.On("OnUpdate", mock.Anything, mock.Anything).Return()
-	broadcaster.On("OnDelete", mock.Anything).Return()
+	broadcaster := broadcasterMock{
+		objects: objects,
+	}
 
 	appStateCache := appstate.NewCache(cache.NewCache(cache.NewInMemoryCache(time.Hour)), time.Hour)
 	// pre-populate the app cache