fix: Sort reconcile events to fix flakey test

pkg/apply/applier_test.go

@@ -343,15 +343,7 @@ func TestApplier(t *testing.T) {
 						Type:      event.Started,
 					},
 				},
-				// Secrets before Deployments (see pkg/ordering)
+				// Wait events with same status sorted by Identifier (see pkg/testutil)
-				{
-					EventType: event.WaitType,
-					WaitEvent: &testutil.ExpWaitEvent{
-						GroupName:  "wait-0",
-						Status:     event.ReconcilePending,
-						Identifier: testutil.ToIdentifier(t, resources["secret"]),
-					},
-				},
 				{
 					EventType: event.WaitType,
 					WaitEvent: &testutil.ExpWaitEvent{
@@ -360,7 +352,15 @@ func TestApplier(t *testing.T) {
 						Identifier: testutil.ToIdentifier(t, resources["deployment"]),
 					},
 				},
-				// Deployment before Secret (see statusEvents)
+				{
+					EventType: event.WaitType,
+					WaitEvent: &testutil.ExpWaitEvent{
+						GroupName:  "wait-0",
+						Status:     event.ReconcilePending,
+						Identifier: testutil.ToIdentifier(t, resources["secret"]),
+					},
+				},
+				// Wait events with same status sorted by Identifier (see pkg/testutil)
 				{
 					EventType: event.WaitType,
 					WaitEvent: &testutil.ExpWaitEvent{
@@ -524,15 +524,7 @@ func TestApplier(t *testing.T) {
 						Type:      event.Started,
 					},
 				},
-				// Apply Secrets before Deployments (see ordering.SortableMetas)
+				// Wait events with same status sorted by Identifier (see pkg/testutil)
-				{
-					EventType: event.WaitType,
-					WaitEvent: &testutil.ExpWaitEvent{
-						GroupName:  "wait-0",
-						Status:     event.ReconcilePending,
-						Identifier: testutil.ToIdentifier(t, resources["secret"]),
-					},
-				},
 				{
 					EventType: event.WaitType,
 					WaitEvent: &testutil.ExpWaitEvent{
@@ -541,7 +533,15 @@ func TestApplier(t *testing.T) {
 						Identifier: testutil.ToIdentifier(t, resources["deployment"]),
 					},
 				},
-				// Wait Deployments before Secrets (see testutil.GroupedEventsByID)
+				{
+					EventType: event.WaitType,
+					WaitEvent: &testutil.ExpWaitEvent{
+						GroupName:  "wait-0",
+						Status:     event.ReconcilePending,
+						Identifier: testutil.ToIdentifier(t, resources["secret"]),
+					},
+				},
+				// Wait events with same status sorted by Identifier (see pkg/testutil)
 				{
 					EventType: event.WaitType,
 					WaitEvent: &testutil.ExpWaitEvent{
@@ -730,7 +730,7 @@ func TestApplier(t *testing.T) {
 						Type:      event.Started,
 					},
 				},
-				// Prune Deployments before Secrets (see ordering.SortableMetas)
+				// Wait events with same status sorted by Identifier (see pkg/testutil)
 				{
 					EventType: event.WaitType,
 					WaitEvent: &testutil.ExpWaitEvent{
@@ -747,7 +747,7 @@ func TestApplier(t *testing.T) {
 						Identifier: testutil.ToIdentifier(t, resources["secret"]),
 					},
 				},
-				// Wait Deployments before Secrets (see testutil.GroupedEventsByID)
+				// Wait events with same status sorted by Identifier (see pkg/testutil)
 				{
 					EventType: event.WaitType,
 					WaitEvent: &testutil.ExpWaitEvent{
@@ -1123,6 +1123,7 @@ func TestApplier(t *testing.T) {
 						Type:      event.Started,
 					},
 				},
+				// Wait events sorted Pending > Successful (see pkg/testutil)
 				{
 					EventType: event.WaitType,
 					WaitEvent: &testutil.ExpWaitEvent{
@@ -1214,15 +1215,15 @@ func TestApplier(t *testing.T) {
 						Identifiers: object.ObjMetadataSet{
 							object.UnstructuredToObjMetadata(
 								testutil.Unstructured(t, resources["deployment"], JSONPathSetter{
-									"$.kind", "",
+									"$.metadata.name", "",
 								}),
 							),
 						},
 						Error: testutil.EqualErrorString(validation.NewError(
-							field.Required(field.NewPath("kind"), "kind is required"),
+							field.Required(field.NewPath("metadata", "name"), "name is required"),
 							object.UnstructuredToObjMetadata(
 								testutil.Unstructured(t, resources["deployment"], JSONPathSetter{
-									"$.kind", "",
+									"$.metadata.name", "",
 								}),
 							),
 						).Error()),
@@ -1234,15 +1235,15 @@ func TestApplier(t *testing.T) {
 						Identifiers: object.ObjMetadataSet{
 							object.UnstructuredToObjMetadata(
 								testutil.Unstructured(t, resources["deployment"], JSONPathSetter{
-									"$.metadata.name", "",
+									"$.kind", "",
 								}),
 							),
 						},
 						Error: testutil.EqualErrorString(validation.NewError(
-							field.Required(field.NewPath("metadata", "name"), "name is required"),
+							field.Required(field.NewPath("kind"), "kind is required"),
 							object.UnstructuredToObjMetadata(
 								testutil.Unstructured(t, resources["deployment"], JSONPathSetter{
-									"$.metadata.name", "",
+									"$.kind", "",
 								}),
 							),
 						).Error()),
@@ -1302,7 +1303,7 @@ func TestApplier(t *testing.T) {
 						Type:      event.Started,
 					},
 				},
-				// Secret pending
+				// Wait events sorted Pending > Successful (see pkg/testutil)
 				{
 					EventType: event.WaitType,
 					WaitEvent: &testutil.ExpWaitEvent{
@@ -1311,7 +1312,6 @@ func TestApplier(t *testing.T) {
 						Identifier: testutil.ToIdentifier(t, resources["secret"]),
 					},
 				},
-				// Secret reconciled
 				{
 					EventType: event.WaitType,
 					WaitEvent: &testutil.ExpWaitEvent{
@@ -1802,6 +1802,7 @@ func TestApplierCancel(t *testing.T) {
 						Type:      event.Started,
 					},
 				},
+				// Wait events sorted Pending > Successful (see pkg/testutil)
 				{
 					// Deployment reconcile pending.
 					EventType: event.WaitType,
@@ -1920,6 +1921,9 @@ func TestApplierCancel(t *testing.T) {
 				}
 			}
 
+			// sort to allow comparison of multiple wait events
+			testutil.SortExpEvents(receivedEvents)
+
 			// Validate the rest of the events
 			testutil.AssertEqual(t, tc.expectedEvents, receivedEvents,
 				"Actual events (%d) do not match expected events (%d)",

pkg/apply/destroyer_test.go

@@ -259,6 +259,7 @@ func TestDestroyerCancel(t *testing.T) {
 						Type:      event.Started,
 					},
 				},
+				// Wait events sorted Pending > Successful (see pkg/testutil)
 				{
 					// Deployment reconcile pending.
 					EventType: event.WaitType,
@@ -374,6 +375,9 @@ func TestDestroyerCancel(t *testing.T) {
 				}
 			}
 
+			// sort to allow comparison of multiple wait events
+			testutil.SortExpEvents(receivedEvents)
+
 			// Validate the rest of the events
 			testutil.AssertEqual(t, tc.expectedEvents, receivedEvents,
 				"Actual events (%d) do not match expected events (%d)",

pkg/testutil/events.go

@@ -447,45 +447,41 @@ func (ape GroupedEventsByID) Less(i, j int) bool {
 		return i < j
 	}
 	switch ape[i].EventType {
+	case event.ValidationType:
+		// Validation events are predictable ordered by input object set order.
 	case event.ApplyType:
-		if ape[i].ApplyEvent.GroupName == ape[j].ApplyEvent.GroupName &&
+		// Apply events are are predictably ordered by ordering.SortableMetas.
-			ape[i].ApplyEvent.Status == ape[j].ApplyEvent.Status &&
-			ape[i].ApplyEvent.Identifier.GroupKind == ape[j].ApplyEvent.Identifier.GroupKind {
-			// apply events are predictably ordered by GroupKind, due to
-			// ordering.SortableMetas.
-			// So we only need to sort by namespace & name.
-			return ape[i].ApplyEvent.Identifier.String() < ape[j].ApplyEvent.Identifier.String()
-		}
 	case event.PruneType:
-		if ape[i].PruneEvent.GroupName == ape[j].PruneEvent.GroupName &&
+		// Prune events are predictably ordered in reverse apply order.
-			ape[i].PruneEvent.Status == ape[j].PruneEvent.Status &&
-			ape[i].PruneEvent.Identifier.GroupKind == ape[j].PruneEvent.Identifier.GroupKind {
-			// prune events are predictably ordered by GroupKind, due to
-			// ordering.SortableMetas (reversed).
-			// So we only need to sort by namespace & name.
-			return ape[i].PruneEvent.Identifier.String() < ape[j].PruneEvent.Identifier.String()
-		}
 	case event.DeleteType:
-		if ape[i].DeleteEvent.GroupName == ape[j].DeleteEvent.GroupName &&
+		// Delete events are predictably ordered in reverse apply order.
-			ape[i].DeleteEvent.Status == ape[j].DeleteEvent.Status &&
-			ape[i].DeleteEvent.Identifier.GroupKind == ape[j].DeleteEvent.Identifier.GroupKind {
-			// delete events are predictably ordered by GroupKind, due to
-			// ordering.SortableMetas (reversed).
-			// So we only need to sort by namespace & name.
-			return ape[i].DeleteEvent.Identifier.String() < ape[j].DeleteEvent.Identifier.String()
-		}
 	case event.WaitType:
-		if ape[i].WaitEvent.GroupName == ape[j].WaitEvent.GroupName &&
+		// Wait events are unpredictably ordered, because the status may
-			ape[i].WaitEvent.Status == ape[j].WaitEvent.Status &&
+		// reconcile before or after the WaitTask starts, and status event
-			ape[i].WaitEvent.Status == event.ReconcileSuccessful {
+		// order after starting is dependent on remote controller behavior.
-			// pending, skipped, and timeout operations are predictably ordered
+		// So here we sort status groups explicitly:
-			// using the order in WaitTask.Ids.
+		// Pending > Skipped > Successful > Failed > Timeout.
-			// So we only need to sort Reconciled events, which occur in the
+		// Each status group is then sorted by Identifier:
-			// order the Waitask receives StatusEvents with Current/NotFound.
+		// Group > Kind > Namespace > Name.
+		// Note that the Pending status is always optional.
+		if ape[i].WaitEvent.GroupName == ape[j].WaitEvent.GroupName {
+			if ape[i].WaitEvent.Status != ape[j].WaitEvent.Status {
+				return lessWaitStatus(ape[i].WaitEvent.Status, ape[j].WaitEvent.Status)
+			}
 			return ape[i].WaitEvent.Identifier.String() < ape[j].WaitEvent.Identifier.String()
 		}
-	case event.ValidationType:
-		return ape[i].ValidationEvent.Identifiers.Hash() < ape[j].ValidationEvent.Identifiers.Hash()
 	}
 	return i < j
 }
+
+var waitStatusWeight = map[event.WaitEventStatus]int{
+	event.ReconcilePending:    0,
+	event.ReconcileSkipped:    1,
+	event.ReconcileSuccessful: 2,
+	event.ReconcileFailed:     3,
+	event.ReconcileTimeout:    4,
+}
+
+func lessWaitStatus(x, y event.WaitEventStatus) bool {
+	return waitStatusWeight[x] < waitStatusWeight[y]
+}

test/e2e/continue_on_error_test.go

@@ -135,15 +135,6 @@ func continueOnErrorTest(ctx context.Context, c client.Client, invConfig invconf
 				Type:      event.Started,
 			},
 		},
-		{
-			// CRD reconcile Skipped.
-			EventType: event.WaitType,
-			WaitEvent: &testutil.ExpWaitEvent{
-				GroupName:  "wait-0",
-				Status:     event.ReconcileSkipped,
-				Identifier: object.UnstructuredToObjMetadata(invalidCrdObj),
-			},
-		},
 		{
 			// Pod1 reconcile Pending.
 			EventType: event.WaitType,
@@ -154,7 +145,16 @@ func continueOnErrorTest(ctx context.Context, c client.Client, invConfig invconf
 			},
 		},
 		{
-			// Pod1 confirmed Current.
+			// CRD reconcile Skipped.
+			EventType: event.WaitType,
+			WaitEvent: &testutil.ExpWaitEvent{
+				GroupName:  "wait-0",
+				Status:     event.ReconcileSkipped,
+				Identifier: object.UnstructuredToObjMetadata(invalidCrdObj),
+			},
+		},
+		{
+			// Pod1 reconcile Successful.
 			EventType: event.WaitType,
 			WaitEvent: &testutil.ExpWaitEvent{
 				GroupName:  "wait-0",

test/e2e/depends_on_test.go

@@ -382,7 +382,10 @@ func dependsOnTest(ctx context.Context, c client.Client, invConfig invconfig.Inv
 			},
 		},
 	}
-	Expect(testutil.EventsToExpEvents(applierEvents)).To(testutil.Equal(expEvents))
+	receivedEvents := testutil.EventsToExpEvents(applierEvents)
+	// sort to handle objects reconciling in random order
+	testutil.SortExpEvents(receivedEvents)
+	Expect(receivedEvents).To(testutil.Equal(expEvents))
 
 	By("verify namespace1 created")
 	e2eutil.AssertUnstructuredExists(ctx, c, namespace1Obj)
@@ -659,15 +662,6 @@ func dependsOnTest(ctx context.Context, c client.Client, invConfig invconfig.Inv
 				Type:      event.Started,
 			},
 		},
-		{
-			// Namespace2 reconcile Pending.
-			EventType: event.WaitType,
-			WaitEvent: &testutil.ExpWaitEvent{
-				GroupName:  "wait-3",
-				Status:     event.ReconcilePending,
-				Identifier: object.UnstructuredToObjMetadata(namespace2Obj),
-			},
-		},
 		{
 			// Namespace1 reconcile Pending.
 			EventType: event.WaitType,
@@ -677,6 +671,15 @@ func dependsOnTest(ctx context.Context, c client.Client, invConfig invconfig.Inv
 				Identifier: object.UnstructuredToObjMetadata(namespace1Obj),
 			},
 		},
+		{
+			// Namespace2 reconcile Pending.
+			EventType: event.WaitType,
+			WaitEvent: &testutil.ExpWaitEvent{
+				GroupName:  "wait-3",
+				Status:     event.ReconcilePending,
+				Identifier: object.UnstructuredToObjMetadata(namespace2Obj),
+			},
+		},
 		{
 			// Namespace1 confirmed NotFound.
 			EventType: event.WaitType,
@@ -723,7 +726,10 @@ func dependsOnTest(ctx context.Context, c client.Client, invConfig invconfig.Inv
 			},
 		},
 	}
-	Expect(testutil.EventsToExpEvents(destroyerEvents)).To(testutil.Equal(expEvents))
+	receivedEvents = testutil.EventsToExpEvents(destroyerEvents)
+	// sort to handle objects reconciling in random order
+	testutil.SortExpEvents(receivedEvents)
+	Expect(receivedEvents).To(testutil.Equal(expEvents))
 
 	By("verify pod1 deleted")
 	e2eutil.AssertUnstructuredDoesNotExist(ctx, c, pod1Obj)