improve ordering cycle error message

prashantrewar · Mar 22, 2020 · 2f6543e · 2f6543e
1 parent 549e0e6
commit 2f6543e
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 8 deletions.
diff --git a/pkg/kapp/diffgraph/change_graph.go b/pkg/kapp/diffgraph/change_graph.go
@@ -94,23 +94,25 @@ func (g *ChangeGraph) printChanges(changes []*Change, indent string) string {
 
 func (g *ChangeGraph) checkCycles() error {
 	for _, change := range g.changes {
-		changeDesc := change.Change.Resource().Description()
+		changeDesc := fmt.Sprintf("[%s]", change.Change.Resource().Description())
 
 		err := g.checkCyclesInChanges(change.WaitingFor, []*Change{change}, changeDesc)
 		if err != nil {
 			return err
 		}
 	}
+
 	return nil
 }
 
 func (g *ChangeGraph) checkCyclesInChanges(changes []*Change, visitedChanges []*Change, descPath string) error {
 	for _, change := range changes {
-		changeDesc := fmt.Sprintf("%s -> %s", descPath, change.Change.Resource().Description())
+		changeDesc := fmt.Sprintf("%s -> [%s]", descPath, change.Change.Resource().Description())
 
 		for _, visitedChange := range visitedChanges {
 			if change == visitedChange {
-				return fmt.Errorf("Detected cycle in grouped changes: %s", changeDesc)
+				return fmt.Errorf("Detected cycle while ordering changes: %s (found repeated: %s)",
+					changeDesc, change.Change.Resource().Description())
 			}
 		}
 

diff --git a/pkg/kapp/diffgraph/change_graph_test.go b/pkg/kapp/diffgraph/change_graph_test.go
@@ -182,7 +182,7 @@ metadata:
 	if err == nil {
 		t.Fatalf("Expected graph to fail building")
 	}
-	if err.Error() != "Detected cycle in grouped changes: job/job1 () cluster -> job/job2 () cluster -> job/job1 () cluster" {
+	if err.Error() != "Detected cycle while ordering changes: [job/job1 () cluster] -> [job/job2 () cluster] -> [job/job1 () cluster] (found repeated: job/job1 () cluster)" {
 		t.Fatalf("Expected to detect cycle: %s", err)
 	}
 }
@@ -215,7 +215,7 @@ metadata:
 	if err == nil {
 		t.Fatalf("Expected graph to fail building")
 	}
-	if err.Error() != "Detected cycle in grouped changes: job/job1 () cluster -> job/job3 () cluster -> job/job2 () cluster -> job/job1 () cluster" {
+	if err.Error() != "Detected cycle while ordering changes: [job/job1 () cluster] -> [job/job3 () cluster] -> [job/job2 () cluster] -> [job/job1 () cluster] (found repeated: job/job1 () cluster)" {
 		t.Fatalf("Expected to detect cycle: %s", err)
 	}
 }
@@ -241,7 +241,7 @@ metadata:
 	if err == nil {
 		t.Fatalf("Expected graph to fail building")
 	}
-	if err.Error() != "Detected cycle in grouped changes: job/job1 () cluster -> job/job2 () cluster -> job/job1 () cluster" {
+	if err.Error() != "Detected cycle while ordering changes: [job/job1 () cluster] -> [job/job2 () cluster] -> [job/job1 () cluster] (found repeated: job/job1 () cluster)" {
 		t.Fatalf("Expected to detect cycle: %s", err)
 	}
 }
@@ -273,7 +273,7 @@ metadata:
 	if err == nil {
 		t.Fatalf("Expected graph to fail building")
 	}
-	if err.Error() != "Detected cycle in grouped changes: job/job3 () cluster -> job/job1 () cluster -> job/job2 () cluster -> job/job1 () cluster" {
+	if err.Error() != "Detected cycle while ordering changes: [job/job3 () cluster] -> [job/job1 () cluster] -> [job/job2 () cluster] -> [job/job1 () cluster] (found repeated: job/job1 () cluster)" {
 		t.Fatalf("Expected to detect cycle: %s", err)
 	}
 }
@@ -292,7 +292,7 @@ metadata:
 	if err == nil {
 		t.Fatalf("Expected graph to fail building")
 	}
-	if err.Error() != "Detected cycle in grouped changes: job/job1 () cluster -> job/job1 () cluster" {
+	if err.Error() != "Detected cycle while ordering changes: [job/job1 () cluster] -> [job/job1 () cluster] (found repeated: job/job1 () cluster)" {
 		t.Fatalf("Expected to detect cycle: %s", err)
 	}
 }