Fix input revision comparison (#215)

Now that the struct has pointer type'd fields it cannot be compared with
`==` - pointers are compared literally, by comparing the addresses the
point to. So if non-nil they will never match.

Co-authored-by: Jordan Olshevski <[email protected]>

internal/controllers/watch/kind.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"math/rand"
 	"path"
+	"reflect"
 	"time"
 
 	apiv1 "github.com/Azure/eno/api/v1"
@@ -161,7 +162,7 @@ func (k *KindWatchController) Stop(ctx context.Context) {
 }
 
 func (k *KindWatchController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	logger := logr.FromContextOrDiscard(ctx)
+	logger := logr.FromContextOrDiscard(ctx).WithValues("group", k.gvk.Group, "version", k.gvk.Version, "kind", k.gvk.Kind)
 
 	meta := &metav1.PartialObjectMetadata{}
 	meta.SetGroupVersionKind(k.gvk)
@@ -241,8 +242,8 @@ func setInputRevisions(comp *apiv1.Composition, revs *apiv1.InputRevisions) bool
 		if ir.Key != revs.Key {
 			continue
 		}
-		if ir == *revs {
-			return false // TODO: Unit test for idempotence
+		if reflect.DeepEqual(ir, *revs) {
+			return false
 		}
 		comp.Status.InputRevisions[i] = *revs
 		return true

internal/controllers/watch/kind_test.go

@@ -0,0 +1,121 @@
+package watch
+
+import (
+	"testing"
+
+	apiv1 "github.com/Azure/eno/api/v1"
+	"github.com/stretchr/testify/assert"
+	"k8s.io/utils/ptr"
+)
+
+func TestSetInputRevisions(t *testing.T) {
+	tests := []struct {
+		name      string
+		comp      *apiv1.Composition
+		revs      *apiv1.InputRevisions
+		expected  bool
+		finalRevs []apiv1.InputRevisions
+	}{
+		{
+			name: "add new revision when key is not found",
+			comp: &apiv1.Composition{
+				Status: apiv1.CompositionStatus{
+					InputRevisions: []apiv1.InputRevisions{
+						{Key: "rev1", Revision: ptr.To(1)},
+					},
+				},
+			},
+			revs: &apiv1.InputRevisions{
+				Key:      "rev2",
+				Revision: ptr.To(2),
+			},
+			expected: true,
+			finalRevs: []apiv1.InputRevisions{
+				{Key: "rev1", Revision: ptr.To(1)},
+				{Key: "rev2", Revision: ptr.To(2)},
+			},
+		},
+		{
+			name: "update existing revision",
+			comp: &apiv1.Composition{
+				Status: apiv1.CompositionStatus{
+					InputRevisions: []apiv1.InputRevisions{
+						{Key: "rev1", Revision: ptr.To(1)},
+					},
+				},
+			},
+			revs: &apiv1.InputRevisions{
+				Key:      "rev1",
+				Revision: ptr.To(2),
+			},
+			expected: true,
+			finalRevs: []apiv1.InputRevisions{
+				{Key: "rev1", Revision: ptr.To(2)},
+			},
+		},
+		{
+			name: "no update if revision is identical",
+			comp: &apiv1.Composition{
+				Status: apiv1.CompositionStatus{
+					InputRevisions: []apiv1.InputRevisions{
+						{Key: "rev1", Revision: ptr.To(1)},
+					},
+				},
+			},
+			revs: &apiv1.InputRevisions{
+				Key:      "rev1",
+				Revision: ptr.To(1),
+			},
+			expected: false,
+			finalRevs: []apiv1.InputRevisions{
+				{Key: "rev1", Revision: ptr.To(1)},
+			},
+		},
+		{
+			name: "no update if revision is identical and synth generation is set",
+			comp: &apiv1.Composition{
+				Status: apiv1.CompositionStatus{
+					InputRevisions: []apiv1.InputRevisions{
+						{Key: "rev1", Revision: ptr.To(1), SynthesizerGeneration: ptr.To(int64(3))},
+					},
+				},
+			},
+			revs: &apiv1.InputRevisions{
+				Key:                   "rev1",
+				Revision:              ptr.To(1),
+				SynthesizerGeneration: ptr.To(int64(3)),
+			},
+			expected: false,
+			finalRevs: []apiv1.InputRevisions{
+				{Key: "rev1", Revision: ptr.To(1), SynthesizerGeneration: ptr.To(int64(3))},
+			},
+		},
+		{
+			name: "update if revision is identical but synth generation is not",
+			comp: &apiv1.Composition{
+				Status: apiv1.CompositionStatus{
+					InputRevisions: []apiv1.InputRevisions{
+						{Key: "rev1", Revision: ptr.To(1), SynthesizerGeneration: ptr.To(int64(3))},
+					},
+				},
+			},
+			revs: &apiv1.InputRevisions{
+				Key:                   "rev1",
+				Revision:              ptr.To(1),
+				SynthesizerGeneration: ptr.To(int64(5)),
+			},
+			expected: true,
+			finalRevs: []apiv1.InputRevisions{
+				{Key: "rev1", Revision: ptr.To(1), SynthesizerGeneration: ptr.To(int64(5))},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := setInputRevisions(tt.comp, tt.revs)
+			assert.Equal(t, tt.expected, result)
+			assert.Equal(t, tt.finalRevs, tt.comp.Status.InputRevisions)
+		})
+	}
+}