fix: Make CyclicDependencyError edges stable

- Add sorting for Edges & Verticies
- Improve tests that contain CyclicDependencyError

pkg/apply/solver/solver_test.go

@@ -18,6 +18,8 @@ import (
 	"sigs.k8s.io/cli-utils/pkg/common"
 	"sigs.k8s.io/cli-utils/pkg/inventory"
 	"sigs.k8s.io/cli-utils/pkg/object"
+	"sigs.k8s.io/cli-utils/pkg/object/graph"
+	"sigs.k8s.io/cli-utils/pkg/object/validation"
 	"sigs.k8s.io/cli-utils/pkg/testutil"
 )
 
@@ -107,12 +109,11 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
 		applyObjs     []*unstructured.Unstructured
 		options       Options
 		expectedTasks []taskrunner.Task
-		isError       bool
+		expectedError error
 	}{
 		"no resources, no tasks": {
 			applyObjs:     []*unstructured.Unstructured{},
 			expectedTasks: []taskrunner.Task{},
-			isError:       false,
 		},
 		"single resource, one apply task, one wait task": {
 			applyObjs: []*unstructured.Unstructured{
@@ -134,7 +135,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
 					testutil.NewFakeRESTMapper(),
 				),
 			},
-			isError: false,
 		},
 		"multiple resource with no timeout": {
 			applyObjs: []*unstructured.Unstructured{
@@ -159,7 +159,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
 					testutil.NewFakeRESTMapper(),
 				),
 			},
-			isError: false,
 		},
 		"multiple resources with reconcile timeout": {
 			applyObjs: []*unstructured.Unstructured{
@@ -187,7 +186,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
 					testutil.NewFakeRESTMapper(),
 				),
 			},
-			isError: false,
 		},
 		"multiple resources with reconcile timeout and dryrun": {
 			applyObjs: []*unstructured.Unstructured{
@@ -208,7 +206,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
 					},
 				},
 			},
-			isError: false,
 		},
 		"multiple resources with reconcile timeout and server-dryrun": {
 			applyObjs: []*unstructured.Unstructured{
@@ -229,7 +226,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
 					},
 				},
 			},
-			isError: false,
 		},
 		"multiple resources including CRD": {
 			applyObjs: []*unstructured.Unstructured{
@@ -269,7 +265,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
 					testutil.NewFakeRESTMapper(),
 				),
 			},
-			isError: false,
 		},
 		"no wait with CRDs if it is a dryrun": {
 			applyObjs: []*unstructured.Unstructured{
@@ -296,7 +291,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
 					},
 				},
 			},
-			isError: false,
 		},
 		"resources in namespace creates multiple apply tasks": {
 			applyObjs: []*unstructured.Unstructured{
@@ -336,7 +330,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
 					testutil.NewFakeRESTMapper(),
 				),
 			},
-			isError: false,
 		},
 		"deployment depends on secret creates multiple tasks": {
 			applyObjs: []*unstructured.Unstructured{
@@ -362,7 +355,8 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
 				&task.ApplyTask{
 					TaskName: "apply-1",
 					Objects: []*unstructured.Unstructured{
-						testutil.Unstructured(t, resources["deployment"]),
+						testutil.Unstructured(t, resources["deployment"],
+							testutil.AddDependsOn(t, testutil.ToIdentifier(t, resources["secret"]))),
 					},
 				},
 				taskrunner.NewWaitTask(
@@ -374,7 +368,6 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
 					testutil.NewFakeRESTMapper(),
 				),
 			},
-			isError: false,
 		},
 		"cyclic dependency returns error": {
 			applyObjs: []*unstructured.Unstructured{
@@ -384,7 +377,22 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
 					testutil.AddDependsOn(t, testutil.ToIdentifier(t, resources["deployment"]))),
 			},
 			expectedTasks: []taskrunner.Task{},
-			isError:       true,
+			expectedError: validation.NewError(
+				graph.CyclicDependencyError{
+					Edges: []graph.Edge{
+						{
+							From: testutil.ToIdentifier(t, resources["secret"]),
+							To:   testutil.ToIdentifier(t, resources["deployment"]),
+						},
+						{
+							From: testutil.ToIdentifier(t, resources["deployment"]),
+							To:   testutil.ToIdentifier(t, resources["secret"]),
+						},
+					},
+				},
+				testutil.ToIdentifier(t, resources["secret"]),
+				testutil.ToIdentifier(t, resources["deployment"]),
+			),
 		},
 	}
 
@@ -403,11 +411,11 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
 				[]mutator.Interface{},
 				tc.options,
 			).Build()
-			if tc.isError {
+			if tc.expectedError != nil {
-				assert.NotNil(t, err, "expected error, but received none")
+				assert.EqualError(t, err, tc.expectedError.Error())
 				return
 			}
-			assert.Nil(t, err, "unexpected error received")
+			assert.NoError(t, err)
 			assert.Equal(t, len(tc.expectedTasks), len(tq.tasks))
 			for i, expTask := range tc.expectedTasks {
 				actualTask := tq.tasks[i]
@@ -417,18 +425,11 @@ func TestTaskQueueBuilder_AppendApplyWaitTasks(t *testing.T) {
 				switch expTsk := expTask.(type) {
 				case *task.ApplyTask:
 					actApplyTask := toApplyTask(t, actualTask)
-					assert.Equal(t, len(expTsk.Objects), len(actApplyTask.Objects))
+					testutil.AssertEqual(t, expTsk.Objects, actApplyTask.Objects, "ApplyTask mismatch")
-					// Order is NOT important for objects stored within task.
-					verifyObjSets(t, expTsk.Objects, actApplyTask.Objects)
 				case *taskrunner.WaitTask:
 					actWaitTask := toWaitTask(t, actualTask)
-					assert.Equal(t, len(expTsk.Ids), len(actWaitTask.Ids))
+					testutil.AssertEqual(t, expTsk.Ids, actWaitTask.Ids)
-					// Order is NOT important for ids stored within task.
+					assert.Equal(t, taskrunner.AllCurrent, actWaitTask.Condition, "WaitTask mismatch")
-					if !expTsk.Ids.Equal(actWaitTask.Ids) {
-						t.Errorf("expected wait ids (%v), got (%v)",
-							expTsk.Ids, actWaitTask.Ids)
-					}
-					assert.Equal(t, taskrunner.AllCurrent, actWaitTask.Condition)
 				}
 			}
 		})
@@ -440,13 +441,12 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
 		pruneObjs     []*unstructured.Unstructured
 		options       Options
 		expectedTasks []taskrunner.Task
-		isError       bool
+		expectedError error
 	}{
 		"no resources, no tasks": {
 			pruneObjs:     []*unstructured.Unstructured{},
 			options:       Options{Prune: true},
 			expectedTasks: []taskrunner.Task{},
-			isError:       false,
 		},
 		"single resource, one prune task, one wait task": {
 			pruneObjs: []*unstructured.Unstructured{
@@ -469,7 +469,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
 					testutil.NewFakeRESTMapper(),
 				),
 			},
-			isError: false,
 		},
 		"multiple resources, one prune task, one wait task": {
 			pruneObjs: []*unstructured.Unstructured{
@@ -495,7 +494,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
 					testutil.NewFakeRESTMapper(),
 				),
 			},
-			isError: false,
 		},
 		"dependent resources, two prune tasks, two wait tasks": {
 			pruneObjs: []*unstructured.Unstructured{
@@ -509,7 +507,8 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
 				&task.PruneTask{
 					TaskName: "prune-0",
 					Objects: []*unstructured.Unstructured{
-						testutil.Unstructured(t, resources["pod"]),
+						testutil.Unstructured(t, resources["pod"],
+							testutil.AddDependsOn(t, testutil.ToIdentifier(t, resources["secret"]))),
 					},
 				},
 				taskrunner.NewWaitTask(
@@ -535,7 +534,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
 					testutil.NewFakeRESTMapper(),
 				),
 			},
-			isError: false,
 		},
 		"single resource with prune timeout has wait task": {
 			pruneObjs: []*unstructured.Unstructured{
@@ -562,7 +560,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
 					testutil.NewFakeRESTMapper(),
 				),
 			},
-			isError: false,
 		},
 		"multiple resources with prune timeout and server-dryrun": {
 			pruneObjs: []*unstructured.Unstructured{
@@ -584,7 +581,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
 					},
 				},
 			},
-			isError: false,
 		},
 		"multiple resources including CRD": {
 			pruneObjs: []*unstructured.Unstructured{
@@ -626,7 +622,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
 					testutil.NewFakeRESTMapper(),
 				),
 			},
-			isError: false,
 		},
 		"no wait with CRDs if it is a dryrun": {
 			pruneObjs: []*unstructured.Unstructured{
@@ -654,7 +649,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
 					},
 				},
 			},
-			isError: false,
 		},
 		"resources in namespace creates multiple apply tasks": {
 			pruneObjs: []*unstructured.Unstructured{
@@ -695,7 +689,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
 					testutil.NewFakeRESTMapper(),
 				),
 			},
-			isError: false,
 		},
 		"cyclic dependency returns error": {
 			pruneObjs: []*unstructured.Unstructured{
@@ -706,7 +699,22 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
 			},
 			options:       Options{Prune: true},
 			expectedTasks: []taskrunner.Task{},
-			isError:       true,
+			expectedError: validation.NewError(
+				graph.CyclicDependencyError{
+					Edges: []graph.Edge{
+						{
+							From: testutil.ToIdentifier(t, resources["secret"]),
+							To:   testutil.ToIdentifier(t, resources["deployment"]),
+						},
+						{
+							From: testutil.ToIdentifier(t, resources["deployment"]),
+							To:   testutil.ToIdentifier(t, resources["secret"]),
+						},
+					},
+				},
+				testutil.ToIdentifier(t, resources["secret"]),
+				testutil.ToIdentifier(t, resources["deployment"]),
+			),
 		},
 	}
 
@@ -721,11 +729,11 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
 			}
 			emptyPruneFilters := []filter.ValidationFilter{}
 			tq, err := tqb.AppendPruneWaitTasks(tc.pruneObjs, emptyPruneFilters, tc.options).Build()
-			if tc.isError {
+			if tc.expectedError != nil {
-				assert.NotNil(t, err, "expected error, but received none")
+				assert.EqualError(t, err, tc.expectedError.Error())
 				return
 			}
-			assert.Nil(t, err, "unexpected error received")
+			assert.NoError(t, err)
 			assert.Equal(t, len(tc.expectedTasks), len(tq.tasks))
 			for i, expTask := range tc.expectedTasks {
 				actualTask := tq.tasks[i]
@@ -735,16 +743,11 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
 				switch expTsk := expTask.(type) {
 				case *task.PruneTask:
 					actPruneTask := toPruneTask(t, actualTask)
-					assert.Equal(t, len(expTsk.Objects), len(actPruneTask.Objects))
+					testutil.AssertEqual(t, expTsk.Objects, actPruneTask.Objects, "PruneTask mismatch")
-					verifyObjSets(t, expTsk.Objects, actPruneTask.Objects)
 				case *taskrunner.WaitTask:
 					actWaitTask := toWaitTask(t, actualTask)
-					assert.Equal(t, len(expTsk.Ids), len(actWaitTask.Ids))
+					testutil.AssertEqual(t, expTsk.Ids, actWaitTask.Ids)
-					if !expTsk.Ids.Equal(actWaitTask.Ids) {
+					assert.Equal(t, taskrunner.AllNotFound, actWaitTask.Condition, "WaitTask mismatch")
-						t.Errorf("expected wait ids (%v), got (%v)",
-							expTsk.Ids, actWaitTask.Ids)
-					}
-					assert.Equal(t, taskrunner.AllNotFound, actWaitTask.Condition)
 					// Validate the prune wait timeout.
 					assert.Equal(t, tc.options.PruneTimeout, actualTask.(*taskrunner.WaitTask).Timeout)
 				}
@@ -753,32 +756,6 @@ func TestTaskQueueBuilder_AppendPruneWaitTasks(t *testing.T) {
 	}
 }
 
-// verifyObjSets ensures the slice of expected objects is the same as
-// the actual slice of objects. Order is NOT important.
-func verifyObjSets(t *testing.T, expected []*unstructured.Unstructured, actual []*unstructured.Unstructured) {
-	if len(expected) != len(actual) {
-		t.Fatalf("expected set size (%d), got (%d)", len(expected), len(actual))
-	}
-	for _, obj := range expected {
-		if !containsObj(actual, obj) {
-			t.Fatalf("expected object (%v) not in found", obj)
-		}
-	}
-}
-
-// containsObj returns true if the passed object is within the passed
-// slice of objects; false otherwise.
-func containsObj(objs []*unstructured.Unstructured, obj *unstructured.Unstructured) bool {
-	ids := object.UnstructuredSetToObjMetadataSet(objs)
-	id := object.UnstructuredToObjMetadata(obj)
-	for _, i := range ids {
-		if i == id {
-			return true
-		}
-	}
-	return false
-}
-
 func toWaitTask(t *testing.T, task taskrunner.Task) *taskrunner.WaitTask {
 	switch tsk := task.(type) {
 	case *taskrunner.WaitTask:

pkg/object/graph/graph.go

@@ -9,10 +9,12 @@ package graph
 import (
 	"bytes"
 	"fmt"
+	"sort"
 
 	"sigs.k8s.io/cli-utils/pkg/multierror"
 	"sigs.k8s.io/cli-utils/pkg/object"
 	"sigs.k8s.io/cli-utils/pkg/object/validation"
+	"sigs.k8s.io/cli-utils/pkg/ordering"
 )
 
 // Graph is contains a directed set of edges, implemented as
@@ -45,7 +47,7 @@ func (g *Graph) AddVertex(v object.ObjMetadata) {
 	}
 }
 
-// GetVertices returns an unsorted set of unique vertices in the graph.
+// GetVertices returns a sorted set of unique vertices in the graph.
 func (g *Graph) GetVertices() object.ObjMetadataSet {
 	keys := make(object.ObjMetadataSet, len(g.edges))
 	i := 0
@@ -53,6 +55,7 @@ func (g *Graph) GetVertices() object.ObjMetadataSet {
 		keys[i] = k
 		i++
 	}
+	sort.Sort(ordering.SortableMetas(keys))
 	return keys
 }
 
@@ -74,8 +77,7 @@ func (g *Graph) AddEdge(from object.ObjMetadata, to object.ObjMetadata) {
 	}
 }
 
-// GetEdges returns the slice of vertex pairs which are
+// GetEdges returns a sorted slice of directed graph edges (vertex pairs).
-// the directed edges of the graph.
 func (g *Graph) GetEdges() []Edge {
 	edges := []Edge{}
 	for from, toList := range g.edges {
@@ -84,6 +86,7 @@ func (g *Graph) GetEdges() []Edge {
 			edges = append(edges, edge)
 		}
 	}
+	sort.Sort(SortableEdges(edges))
 	return edges
 }
 
@@ -164,3 +167,30 @@ func (cde CyclicDependencyError) Error() string {
 	}
 	return errorBuf.String()
 }
+
+// SortableEdges sorts a list of edges alphanumerically by From and then To.
+type SortableEdges []Edge
+
+var _ sort.Interface = SortableEdges{}
+
+func (a SortableEdges) Len() int      { return len(a) }
+func (a SortableEdges) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
+func (a SortableEdges) Less(i, j int) bool {
+	if a[i].From != a[j].From {
+		return metaIsLessThan(a[i].From, a[j].From)
+	}
+	return metaIsLessThan(a[i].To, a[j].To)
+}
+
+func metaIsLessThan(i, j object.ObjMetadata) bool {
+	if i.GroupKind.Group != j.GroupKind.Group {
+		return i.GroupKind.Group < j.GroupKind.Group
+	}
+	if i.GroupKind.Kind != j.GroupKind.Kind {
+		return i.GroupKind.Kind < j.GroupKind.Kind
+	}
+	if i.Namespace != j.Namespace {
+		return i.Namespace < j.Namespace
+	}
+	return i.Name < j.Name
+}

pkg/object/graph/graph_test.go

@@ -6,10 +6,14 @@
 package graph
 
 import (
+	"sort"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/cli-utils/pkg/object"
+	"sigs.k8s.io/cli-utils/pkg/object/validation"
+	"sigs.k8s.io/cli-utils/pkg/testutil"
 )
 
 var (
@@ -36,61 +40,86 @@ func TestObjectGraphSort(t *testing.T) {
 		vertices      object.ObjMetadataSet
 		edges         []Edge
 		expected      []object.ObjMetadataSet
-		isError  bool
+		expectedError error
 	}{
 		"one edge": {
 			vertices: object.ObjMetadataSet{o1, o2},
 			edges:    []Edge{e1},
 			expected: []object.ObjMetadataSet{{o2}, {o1}},
-			isError:  false,
 		},
 		"two edges": {
 			vertices: object.ObjMetadataSet{o1, o2, o3},
 			edges:    []Edge{e1, e2},
 			expected: []object.ObjMetadataSet{{o3}, {o2}, {o1}},
-			isError:  false,
 		},
 		"three edges": {
 			vertices: object.ObjMetadataSet{o1, o2, o3},
 			edges:    []Edge{e1, e3, e2},
 			expected: []object.ObjMetadataSet{{o3}, {o2}, {o1}},
-			isError:  false,
 		},
 		"four edges": {
 			vertices: object.ObjMetadataSet{o1, o2, o3, o4},
 			edges:    []Edge{e1, e2, e4, e5},
 			expected: []object.ObjMetadataSet{{o4}, {o3}, {o2}, {o1}},
-			isError:  false,
 		},
 		"five edges": {
 			vertices: object.ObjMetadataSet{o1, o2, o3, o4},
 			edges:    []Edge{e5, e1, e3, e2, e4},
 			expected: []object.ObjMetadataSet{{o4}, {o3}, {o2}, {o1}},
-			isError:  false,
 		},
 		"no edges means all in the same first set": {
 			vertices: object.ObjMetadataSet{o1, o2, o3, o4},
 			edges:    []Edge{},
 			expected: []object.ObjMetadataSet{{o4, o3, o2, o1}},
-			isError:  false,
 		},
 		"multiple objects in first set": {
 			vertices: object.ObjMetadataSet{o1, o2, o3, o4, o5},
 			edges:    []Edge{e1, e2, e5, e8},
 			expected: []object.ObjMetadataSet{{o5, o3}, {o4}, {o2}, {o1}},
-			isError:  false,
 		},
 		"simple cycle in graph is an error": {
 			vertices: object.ObjMetadataSet{o1, o2},
 			edges:    []Edge{e1, e6},
 			expected: []object.ObjMetadataSet{},
-			isError:  true,
+			expectedError: validation.NewError(
+				CyclicDependencyError{
+					Edges: []Edge{
+						{
+							From: o1,
+							To:   o2,
+						},
+						{
+							From: o2,
+							To:   o1,
+						},
+					},
+				},
+				o1, o2,
+			),
 		},
 		"multi-edge cycle in graph is an error": {
 			vertices: object.ObjMetadataSet{o1, o2, o3},
 			edges:    []Edge{e1, e2, e7},
 			expected: []object.ObjMetadataSet{},
-			isError:  true,
+			expectedError: validation.NewError(
+				CyclicDependencyError{
+					Edges: []Edge{
+						{
+							From: o1,
+							To:   o2,
+						},
+						{
+							From: o2,
+							To:   o3,
+						},
+						{
+							From: o3,
+							To:   o1,
+						},
+					},
+				},
+				o1, o2, o3,
+			),
 		},
 	}
 
@@ -104,23 +133,266 @@ func TestObjectGraphSort(t *testing.T) {
 				g.AddEdge(edge.From, edge.To)
 			}
 			actual, err := g.Sort()
-			if err == nil && tc.isError {
+			if tc.expectedError != nil {
-				t.Fatalf("expected error, but received none")
+				assert.EqualError(t, tc.expectedError, err.Error())
-			}
+				return
-			if err != nil && !tc.isError {
-				t.Errorf("unexpected error: %s", err)
-			}
-			if !tc.isError {
-				if len(actual) != len(tc.expected) {
-					t.Errorf("expected (%s), got (%s)", tc.expected, actual)
-				}
-				for i, actualSet := range actual {
-					expectedSet := tc.expected[i]
-					if !expectedSet.Equal(actualSet) {
-						t.Errorf("expected sorted objects (%s), got (%s)", tc.expected, actual)
-					}
-				}
 			}
+			assert.NoError(t, err)
+			testutil.AssertEqual(t, tc.expected, actual)
+		})
+	}
+}
+
+func TestEdgeSort(t *testing.T) {
+	testCases := map[string]struct {
+		edges    []Edge
+		expected []Edge
+	}{
+		"one edge": {
+			edges: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj1"},
+					To:   object.ObjMetadata{Name: "obj2"},
+				},
+			},
+			expected: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj1"},
+					To:   object.ObjMetadata{Name: "obj2"},
+				},
+			},
+		},
+		"two edges no change": {
+			edges: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj1"},
+					To:   object.ObjMetadata{Name: "obj2"},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj2"},
+					To:   object.ObjMetadata{Name: "obj3"},
+				},
+			},
+			expected: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj1"},
+					To:   object.ObjMetadata{Name: "obj2"},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj2"},
+					To:   object.ObjMetadata{Name: "obj3"},
+				},
+			},
+		},
+		"two edges same from": {
+			edges: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj1"},
+					To:   object.ObjMetadata{Name: "obj3"},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj1"},
+					To:   object.ObjMetadata{Name: "obj2"},
+				},
+			},
+			expected: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj1"},
+					To:   object.ObjMetadata{Name: "obj2"},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj1"},
+					To:   object.ObjMetadata{Name: "obj3"},
+				},
+			},
+		},
+		"two edges": {
+			edges: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj2"},
+					To:   object.ObjMetadata{Name: "obj3"},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj1"},
+					To:   object.ObjMetadata{Name: "obj2"},
+				},
+			},
+			expected: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj1"},
+					To:   object.ObjMetadata{Name: "obj2"},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj2"},
+					To:   object.ObjMetadata{Name: "obj3"},
+				},
+			},
+		},
+		"two edges by name": {
+			edges: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+					To:   object.ObjMetadata{Name: "obj3", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj1", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+					To:   object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+				},
+			},
+			expected: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj1", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+					To:   object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+					To:   object.ObjMetadata{Name: "obj3", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+				},
+			},
+		},
+		"three edges": {
+			edges: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj3"},
+					To:   object.ObjMetadata{Name: "obj4"},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj2"},
+					To:   object.ObjMetadata{Name: "obj3"},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj1"},
+					To:   object.ObjMetadata{Name: "obj2"},
+				},
+			},
+			expected: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj1"},
+					To:   object.ObjMetadata{Name: "obj2"},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj2"},
+					To:   object.ObjMetadata{Name: "obj3"},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj3"},
+					To:   object.ObjMetadata{Name: "obj4"},
+				},
+			},
+		},
+		"two edges cycle": {
+			edges: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+					To:   object.ObjMetadata{Name: "obj1", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj1", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+					To:   object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+				},
+			},
+			expected: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj1", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+					To:   object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj2", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+					To:   object.ObjMetadata{Name: "obj1", Namespace: "ns1", GroupKind: schema.GroupKind{Kind: "Pod"}},
+				},
+			},
+		},
+		"three edges cycle": {
+			edges: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj3"},
+					To:   object.ObjMetadata{Name: "obj1"},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj2"},
+					To:   object.ObjMetadata{Name: "obj3"},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj1"},
+					To:   object.ObjMetadata{Name: "obj2"},
+				},
+			},
+			expected: []Edge{
+				{
+					From: object.ObjMetadata{Name: "obj1"},
+					To:   object.ObjMetadata{Name: "obj2"},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj2"},
+					To:   object.ObjMetadata{Name: "obj3"},
+				},
+				{
+					From: object.ObjMetadata{Name: "obj3"},
+					To:   object.ObjMetadata{Name: "obj1"},
+				},
+			},
+		},
+	}
+
+	for tn, tc := range testCases {
+		t.Run(tn, func(t *testing.T) {
+			sort.Sort(SortableEdges(tc.edges))
+			assert.Equal(t, tc.expected, tc.edges)
+		})
+	}
+}
+
+func TestCyclicDependencyErrorString(t *testing.T) {
+	testCases := map[string]struct {
+		cycle          CyclicDependencyError
+		expectedString string
+	}{
+		"two object cycle": {
+			cycle: CyclicDependencyError{
+				Edges: []Edge{
+					{
+						From: o1,
+						To:   o2,
+					},
+					{
+						From: o2,
+						To:   o1,
+					},
+				},
+			},
+			expectedString: `cyclic dependency:
+- /obj1 -> /obj2
+- /obj2 -> /obj1
+`,
+		},
+		"three object cycle": {
+			cycle: CyclicDependencyError{
+				Edges: []Edge{
+					{
+						From: o1,
+						To:   o2,
+					},
+					{
+						From: o2,
+						To:   o3,
+					},
+					{
+						From: o3,
+						To:   o1,
+					},
+				},
+			},
+			expectedString: `cyclic dependency:
+- /obj1 -> /obj2
+- /obj2 -> /obj3
+- /obj3 -> /obj1
+`,
+		},
+	}
+
+	for tn, tc := range testCases {
+		t.Run(tn, func(t *testing.T) {
+			assert.Equal(t, tc.expectedString, tc.cycle.Error())
 		})
 	}
 }