Fix flaky workspace notification tests and add a new slice function D…

…eepEqual

controllers/workspace_controller_notifications_test.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/google/go-cmp/cmp"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
@@ -16,6 +15,7 @@ import (
 
 	tfc "github.com/hashicorp/go-tfe"
 	appv1alpha2 "github.com/hashicorp/terraform-cloud-operator/api/v1alpha2"
+	"github.com/hashicorp/terraform-cloud-operator/internal/slice"
 )
 
 var _ = Describe("Workspace controller", Label("Notifications"), Ordered, func() {
@@ -196,50 +196,53 @@ var _ = Describe("Workspace controller", Label("Notifications"), Ordered, func()
 })
 
 func isNotificationsReconciled(instance *appv1alpha2.Workspace) {
-	Eventually(func() bool {
-		Expect(k8sClient.Get(ctx, namespacedName, instance)).Should(Succeed())
-		s := instance.Spec.Notifications
+	Expect(k8sClient.Get(ctx, namespacedName, instance)).Should(Succeed())
+	spec := instance.Spec.Notifications
+
+	m, err := tfClient.OrganizationMemberships.List(ctx, instance.Spec.Organization, &tfc.OrganizationMembershipListOptions{})
+	Expect(err).Should(Succeed())
+	Expect(m).ShouldNot(BeNil())
+	memberships := make(map[string]string, len(m.Items))
+	for _, ms := range m.Items {
+		memberships[ms.User.ID] = ms.Email
+	}
 
+	Eventually(func() bool {
 		notifications, err := tfClient.NotificationConfigurations.List(ctx, instance.Status.WorkspaceID, &tfc.NotificationConfigurationListOptions{})
 		Expect(err).Should(Succeed())
 		Expect(notifications).ShouldNot(BeNil())
 
-		members, err := tfClient.OrganizationMemberships.List(ctx, instance.Spec.Organization, &tfc.OrganizationMembershipListOptions{})
-		Expect(err).Should(Succeed())
-		Expect(members).ShouldNot(BeNil())
-
-		m := make(map[string]string)
-		for _, ms := range members.Items {
-			m[ms.User.ID] = ms.Email
-		}
-
-		if len(instance.Spec.Notifications) != len(notifications.Items) {
+		if len(spec) != len(notifications.Items) {
 			return false
 		}
 
-		var w []appv1alpha2.Notification
-		for _, n := range notifications.Items {
-			var nt []appv1alpha2.NotificationTrigger
-			for _, t := range n.Triggers {
-				nt = append(nt, appv1alpha2.NotificationTrigger(t))
+		workspace := make([]appv1alpha2.Notification, len(notifications.Items))
+		for i, n := range notifications.Items {
+			// Do not use make()
+			// t must be nil if there are no triggers
+			var t []appv1alpha2.NotificationTrigger
+			for _, v := range n.Triggers {
+				t = append(t, appv1alpha2.NotificationTrigger(v))
 			}
-			var nu []string
-			for _, u := range n.EmailUsers {
-				nu = append(nu, m[u.ID])
+			// Do not use make()
+			// nu must be nil if there are no email users
+			var eu []string
+			for _, v := range n.EmailUsers {
+				eu = append(eu, memberships[v.ID])
 			}
-			w = append(w, appv1alpha2.Notification{
+			workspace[i] = appv1alpha2.Notification{
 				Name:           n.Name,
 				Type:           n.DestinationType,
 				Enabled:        n.Enabled,
 				Token:          n.Token,
-				Triggers:       nt,
+				Triggers:       t,
 				URL:            n.URL,
 				EmailAddresses: n.EmailAddresses,
-				EmailUsers:     nu,
-			})
+				EmailUsers:     eu,
+			}
 		}
 
-		return cmp.Equal(s, w)
+		return slice.DeepEqual(spec, workspace)
 	}).Should(BeTrue())
 }
 

internal/slice/slice.go

@@ -3,6 +3,48 @@
 
 package slice
 
+import (
+	"reflect"
+
+	"github.com/google/go-cmp/cmp"
+)
+
 func RemoveFromSlice[A any](slice []A, i int) []A {
 	return append(slice[:i], slice[i+1:]...)
 }
+
+// DeepEqual compares two given slices, a and b, of an arbitrary type and checks the order of elements.
+// It returns true (indicating that the slices are equal) if all of the following conditions are true:
+// - they are both nil or both not nil
+// - they have the same length
+// - they have the same type
+// - all elements of slice a are present in slice b
+// Otherwise, it returns false.
+func DeepEqual[A any](a, b []A) bool {
+	if len(a) != len(b) {
+		return false
+	}
+
+	if a == nil || b == nil {
+		return true
+	}
+
+	if reflect.TypeOf(a) != reflect.TypeOf(b) {
+		return false
+	}
+
+	for _, va := range a {
+		found := false
+		for _, vb := range b {
+			if cmp.Equal(va, vb) {
+				found = true
+				break
+			}
+		}
+		if !found {
+			return false
+		}
+	}
+
+	return true
+}

internal/slice/slice_test.go

@@ -24,3 +24,46 @@ func TestRemoveFromSlice(t *testing.T) {
 		}
 	}
 }
+
+func TestDeepEqual(t *testing.T) {
+	input := []map[string][]any{
+		{
+			"A": {},
+			"B": {},
+		},
+		{
+			"A": nil,
+			"B": nil,
+		},
+		{
+			"A": {},
+			"B": nil,
+		},
+		{
+			"A": {1, 2, 3},
+			"B": {1, 2, 3},
+		},
+		{
+			"A": {"a", "b", "c"},
+			"B": {"b", "c"},
+		},
+		{
+			"A": {1, 2},
+			"B": {"b", "c"},
+		},
+	}
+	want := []bool{
+		true,
+		true,
+		true,
+		true,
+		false,
+		false,
+	}
+	for i, s := range input {
+		r := DeepEqual(s["A"], s["B"])
+		if r != want[i] {
+			t.Errorf("The wanted result [%t] does not match with the produced [%v] at [%d]", want[i], r, i)
+		}
+	}
+}