From a18608639ac0e5dd212534460e8f4ad519f06ed9 Mon Sep 17 00:00:00 2001 From: Michael Schuett Date: Thu, 13 Jul 2023 16:10:38 -0600 Subject: [PATCH] Fix MaxDepth This fixes a bug with MaxDepth that was also causing a panic because we expected the depth to be one more than it was returning. This fixes the function to now return the depth that is expected and in the case any more bugs exist in drawing the graph we handle that with an explicit check to ensure the padding is never less than 0. --- lib/console/draw_graph.go | 6 +++ lib/console/draw_graph_test.go | 13 +++--- lib/git/BUILD.bazel | 9 ++++- lib/git/branch.go | 11 +++-- lib/git/branch_test.go | 73 ++++++++++++++++++++++++++++++++++ 5 files changed, 100 insertions(+), 12 deletions(-) create mode 100644 lib/git/branch_test.go diff --git a/lib/console/draw_graph.go b/lib/console/draw_graph.go index 9d556ad..15a36df 100644 --- a/lib/console/draw_graph.go +++ b/lib/console/draw_graph.go @@ -170,6 +170,12 @@ func drawLine(o DrawOpts, n *git.BranchNode, depth int, branchPadding = branchPadding - 1 } + // If somehow I still messed up all the calculations in some edge cases make sure + // that branchPadding is never passed as a number less than 0 or the program panics. + if branchPadding < 0 { + branchPadding = 0 + } + fmtStr := []string{ padding, "%s ", // graphLine diff --git a/lib/console/draw_graph_test.go b/lib/console/draw_graph_test.go index e83e9a2..f2f9d4f 100644 --- a/lib/console/draw_graph_test.go +++ b/lib/console/draw_graph_test.go @@ -51,9 +51,9 @@ func TestSimpleDrawGraph(t *testing.T) { out := DrawGraph(*bnw, nil) expected := strings.TrimSpace(` -main +main └ main/branch-1 0:0 [no commit message found] -master +master └ master/branch-1 0:0 [no commit message found]`) if out != expected { @@ -266,7 +266,7 @@ func TestComplexDrawGraph(t *testing.T) { out := DrawGraph(*bnw, &DrawOpts{}) expected := strings.TrimSpace(` -main +main ├ main/branch-1 0:0 [no commit message found] ├ main/branch-2 0:0 [no commit message found] │├ main/branch-2-1 0:0 [no commit message found] @@ -283,13 +283,12 @@ main │└ main/branch-5-2-1 0:0 [no commit message found] ├ main/branch-5-3 0:0 [no commit message found] └ main/branch-5-4 0:0 [no commit message found] -master +master ├ master/branch-1 0:0 [no commit message found] ├ master/branch-2 0:0 [no commit message found] │└ master/branch-2-1 0:0 [no commit message found] ├ master/branch-3 0:0 [no commit message found] └ master/branch-4 0:0 [no commit message found]`) - if out != expected { t.Error("TestComplexDrawGraph failed") t.Log("Got:") @@ -309,9 +308,9 @@ func TestDrawLines(t *testing.T) { Downstream: []*git.BranchNode{}, } - out := drawLine(DrawOpts{}, node, 3, []int{2}, true, len(node.Name)) + out := drawLine(DrawOpts{}, node, 3, []int{2}, true, len(node.Name), 25) - expected := " │└ main/branch-5-2-1 0:0 [no commit message found]" + expected := " │└ main/branch-5-2-1 0:0 [no commit message found]" if out != expected { t.Error("TestDrawLines failed") diff --git a/lib/git/BUILD.bazel b/lib/git/BUILD.bazel index ef20b2e..90e86bc 100644 --- a/lib/git/BUILD.bazel +++ b/lib/git/BUILD.bazel @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "git", @@ -24,3 +24,10 @@ go_library( "@com_github_go_git_go_git_v5//plumbing", ], ) + +go_test( + name = "git_test", + srcs = ["branch_test.go"], + embed = [":git"], + deps = [], +) diff --git a/lib/git/branch.go b/lib/git/branch.go index 70a1477..fd8155b 100644 --- a/lib/git/branch.go +++ b/lib/git/branch.go @@ -58,9 +58,10 @@ func (b BranchNode) IsRoot() bool { // in the following graph output by flow... // // master -// ├ mschuett/trash-test c9d98532 1:1 lol -// └ mschuett/off-master 539c188a 0:0 wowowowowo -// └ mschuett/off-master-2 539c188a 0:0 wowowowowo +// +// ├ mschuett/trash-test c9d98532 1:1 lol +// └ mschuett/off-master 539c188a 0:0 wowowowowo +// └ mschuett/off-master-2 539c188a 0:0 wowowowowo // // CountDownstreams(master) -> 3 // CountDownstreams(mschuett/off-master) -> 1 @@ -99,7 +100,9 @@ func (b BranchNode) maxDepth(i int) int { func (b BranchNode) MaxDepth() int { max := 0 for _, x := range b.Downstream { - depth := x.maxDepth(0) + // Max depth needs to start at 1 because it won't enter this unless + // the branch has a downstream to begin with. + depth := x.maxDepth(1) if depth > max { max = depth } diff --git a/lib/git/branch_test.go b/lib/git/branch_test.go new file mode 100644 index 0000000..b66b26a --- /dev/null +++ b/lib/git/branch_test.go @@ -0,0 +1,73 @@ +package git + +import ( + "testing" +) + +func TestMaxDepthZero(t *testing.T) { + bnw := BranchNodeWrapper{ + RootNodes: []*BranchNode{ + { + Name: "master", + }, + }, + } + + maxDepth := bnw.MaxDepth() + if maxDepth != 0 { + t.Error("maxDepth expected 0 but returned", maxDepth) + } +} + +func TestMaxDepthOne(t *testing.T) { + bnw := BranchNodeWrapper{ + RootNodes: []*BranchNode{ + { + Name: "master", + Downstream: []*BranchNode{ + { + Name: "downstream-from-master", + }, + }, + }, + }, + } + + maxDepth := bnw.MaxDepth() + if maxDepth != 1 { + t.Error("maxDepth expected 1 but returned", maxDepth) + } +} + +func TestMaxDepthMany(t *testing.T) { + bnw := BranchNodeWrapper{ + RootNodes: []*BranchNode{ + { + Name: "master", + Downstream: []*BranchNode{ + { + Name: "downstream-from-master", + Downstream: []*BranchNode{ + { + Name: "downstream-from-master-23", + }, + }, + }, + }, + }, + { + Name: "master2", + Downstream: []*BranchNode{ + { + Name: "downstream-from-master2", + }, + }, + }, + }, + } + + maxDepth := bnw.MaxDepth() + if maxDepth != 2 { + t.Error("maxDepth expected 2 but returned", maxDepth) + } +}