Skip to content

Commit

Permalink
Fix MaxDepth
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
michaeljs1990 committed Jul 13, 2023
1 parent c79ab86 commit a186086
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 12 deletions.
6 changes: 6 additions & 0 deletions lib/console/draw_graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 6 additions & 7 deletions lib/console/draw_graph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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]
Expand All @@ -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:")
Expand All @@ -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")
Expand Down
9 changes: 8 additions & 1 deletion lib/git/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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",
Expand All @@ -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 = [],
)
11 changes: 7 additions & 4 deletions lib/git/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
73 changes: 73 additions & 0 deletions lib/git/branch_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}

0 comments on commit a186086

Please sign in to comment.