Skip to content

Commit

Permalink
fix: cyclic deps
Browse files Browse the repository at this point in the history
  • Loading branch information
Gabriel Musat committed Dec 14, 2022
1 parent 09697f0 commit 8160042
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 9 deletions.
13 changes: 13 additions & 0 deletions internal/board/board_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,19 @@ func TestBoard(t *testing.T) {
{from: 1, to: []int{0}},
},
},
{
Name: "Cyclic deps",
Blocks: []TestBlock{
{name: "a", x: 0, y: 0},
{name: "b", x: 2, y: 1},
{name: "c", x: 4, y: 2},
},
Connections: []TestConnection{
{from: 0, to: []int{1}},
{from: 1, to: []int{2}},
{from: 2, to: []int{1}},
},
},
}

for _, tt := range tests {
Expand Down
3 changes: 3 additions & 0 deletions internal/board/board_test/Cyclic_deps.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
a
└▷b◁─┐
└▷c┘
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
aaa◁
└▷b┘
8 changes: 8 additions & 0 deletions internal/graph/graph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@ func TestMakeGraph(t *testing.T) {
{},
},
},
{
Name: "Cyclic deps",
Spec: [][]int{
{1},
{2},
{1},
},
},
}

for _, tt := range tests {
Expand Down
3 changes: 3 additions & 0 deletions internal/graph/graph_test/Cyclic deps.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
0
└▷1◁─┐
└▷2┘
30 changes: 22 additions & 8 deletions internal/graph/node/level.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,33 @@ func calculateLevel[T any](
maxLevel = newLevel
}
}

if maxLevel == unknown {
// TODO: there is a bug here, there are cases where this is reached.
msg := "This should not be reachable"
msg += fmt.Sprintf("\nhappened while calculating level for node %s", node.Id)
msg += fmt.Sprintf("\nthis node has %d parents", node.Parents.Len())
for _, parentId := range node.Parents.Keys() {
parent, _ := node.Parents.Get(parentId)

panic(msg)
} else {
return ctx, maxLevel
var newLevel int
ctx, newLevel = calculateLevel(ctx, parent, rootId, level+1, append(stack, node.Id))
if newLevel == cyclic {
continue
} else if newLevel > maxLevel {
maxLevel = newLevel
}
}
}
return ctx, maxLevel
}

// Level retrieves the longest path until going to "rootId" avoiding cyclical loops.
func (n *Node[T]) Level(ctx context.Context, rootId string) (context.Context, int) {
return calculateLevel(ctx, n, rootId, 0, []string{})
ctx, lvl := calculateLevel(ctx, n, rootId, 0, []string{})
if lvl == unknown {
// TODO: there is a bug here, there are cases where this is reached.
msg := "This should not be reachable"
msg += fmt.Sprintf("\nhappened while calculating level for node %s", n.Id)
msg += fmt.Sprintf("\nthis node has %d parents", n.Parents.Len())

panic(msg)
}
return ctx, lvl
}
21 changes: 20 additions & 1 deletion internal/graph/node/level_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ func TestNode_Level(t *testing.T) {
{
Name: "Cycle",
NumNodes: 5,

Children: map[int][]int{
0: {1, 2, 3},
1: {2, 4},
Expand All @@ -38,6 +37,26 @@ func TestNode_Level(t *testing.T) {
},
ExpectedLevels: []int{0, 1, 2, 4, 3},
},
{
Name: "Cycle 2",
NumNodes: 3,
Children: map[int][]int{
0: {1, 2},
1: {2, 0},
2: {0, 1},
},
ExpectedLevels: []int{0, 2, 1},
},
{
Name: "Cycle 3",
NumNodes: 3,
Children: map[int][]int{
0: {1},
1: {2},
2: {1},
},
ExpectedLevels: []int{0, 1, 2},
},
}

for _, tt := range tests {
Expand Down

0 comments on commit 8160042

Please sign in to comment.