diff --git a/bots-common/associated_pr_scanner.go b/bots-common/associated_pr_scanner.go index 9034aba..6d7c097 100644 --- a/bots-common/associated_pr_scanner.go +++ b/bots-common/associated_pr_scanner.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "regexp" + "slices" "strconv" "strings" ) @@ -54,7 +55,7 @@ func parsePrLine(line string) (BasicPR, error) { return ret, nil } -func ExtractAssociatedDescriptionAndPRs(data *bufio.Scanner) (string, []BasicPR) { +func ExtractPRsFromDescription(data *bufio.Scanner) (string, []BasicPR) { prs := make([]BasicPR, 0, 1) var desc strings.Builder @@ -74,15 +75,40 @@ func ExtractAssociatedDescriptionAndPRs(data *bufio.Scanner) (string, []BasicPR) } func prToLine(writer io.Writer, pr BasicPR) { - fmt.Fprintf(writer, PrPattern, pr.org, pr.repo, pr.num) writer.Write([]byte("\n")) + fmt.Fprintf(writer, PrPattern, pr.org, pr.repo, pr.num) +} + +// returns: +// <0 for a0 for a>b +// =0 when equal +func compareBasicPRs(a BasicPR, b BasicPR) int { + if c := strings.Compare(a.org, b.org); c != 0 { + return c + } + if c := strings.Compare(a.repo, b.repo); c != 0 { + return c + } + + if a.num > b.num { + return 1 + } + if a.num < b.num { + return -1 + } + + return 0 } func AppendPRsToDescription(desc string, prs []BasicPR) string { var out strings.Builder out.WriteString(strings.TrimSpace(desc)) - out.WriteString("\n\n") + out.WriteString("\n") + + slices.SortFunc(prs, compareBasicPRs) + prs = slices.Compact(prs) for _, pr := range prs { prToLine(&out, pr) diff --git a/bots-common/associated_pr_scanner_test.go b/bots-common/associated_pr_scanner_test.go index bf977e9..ce1664d 100644 --- a/bots-common/associated_pr_scanner_test.go +++ b/bots-common/associated_pr_scanner_test.go @@ -12,181 +12,136 @@ func newStringScanner(s string) *bufio.Scanner { } func TestAssociatedPRScanner(t *testing.T) { - t.Run("No PRs", func(t *testing.T) { - if _, out := ExtractAssociatedDescriptionAndPRs(newStringScanner("")); len(out) != 0 { - t.Error("Unexpected output", out) - } - }) + testTable := []struct { + name string + input string + prs []BasicPR + desc string + }{ + { + "No PRs", + "", + []BasicPR{}, + "", + }, + { + "Single PRs", + "Some header of the issue\n\nFollowed by some description\n\nPR: test/foo#4\n", + []BasicPR{{org: "test", repo: "foo", num: 4}}, + "Some header of the issue\n\nFollowed by some description", + }, + { + "Multiple PRs", + "Some header of the issue\n\nFollowed by some description\nPR: test/foo#4\n\nPR: test/goo#5\n", + []BasicPR{ + {org: "test", repo: "foo", num: 4}, + {org: "test", repo: "goo", num: 5}, + }, + "Some header of the issue\n\nFollowed by some description", + }, + { + "Multiple PRs with whitespace", + "Some header of the issue\n\n\tPR: test/goo#5\n\n Followed by some description\n \t PR: test/foo#4\n", + []BasicPR{ + {org: "test", repo: "foo", num: 4}, + {org: "test", repo: "goo", num: 5}, + }, + "Some header of the issue\n\n\n Followed by some description", + }, + { + "Multiple PRs with missing names and other special cases to ignore", + "Some header of the issue\n\n\n\t PR: foobar#5 \n\t PR: rd/goo5 \n\t PR: test/#5 \n" + + "\t PR: /goo#5 \n\t PR: test/goo# \n\t PR: test / goo # 10 \n\tPR: test/gool# 10 \n" + + "\t PR: test/goo#5 \n\t\n Followed by some description\n\t PR: test/foo#4 \n\t\n\n", + []BasicPR{ + { + org: "test", + repo: "foo", + num: 4, + }, + { + org: "test", + repo: "goo", + num: 5, + }, + }, + "Some header of the issue\n\n\n\t PR: foobar#5 \n\t PR: rd/goo5 \n\t PR: test/#5 \n" + + "\t PR: /goo#5 \n\t PR: test/goo# \n\t PR: test / goo # 10 \n\tPR: test/gool# 10 \n" + + "\t\n Followed by some description", + }, + } - t.Run("Single PR", func(t *testing.T) { - const singlePRText = `Some header of the issue + for _, test := range testTable { + t.Run(test.name, func(t *testing.T) { + desc, prs := ExtractPRsFromDescription(newStringScanner(test.input)) + if len(prs) != len(test.prs) { + t.Error("Unexpected length:", len(prs), "expected:", len(test.prs)) + return + } -Followed by some description -PR: test/foo#4 -` - _, out := ExtractAssociatedDescriptionAndPRs(newStringScanner(singlePRText)) - if len(out) != 1 { - t.Error("Unexpected output", out) - return - } + for _, p := range test.prs { + if !slices.Contains(prs, p) { + t.Error("missing expected PR", p) + } + } - expected := BasicPR{ - org: "test", - repo: "foo", - num: 4, - } - if out[0] != expected { - t.Error("Unexpected", out, "Expected", expected) - } - }) - - t.Run("Multiple PRs", func(t *testing.T) { - const multiplePRText = `Some header of the issue - -Followed by some description -PR: test/foo#4 - -PR: test/goo#5 -` - - _, out := ExtractAssociatedDescriptionAndPRs(newStringScanner(multiplePRText)) - if len(out) != 2 { - t.Error("Unexpected output", out) - return - } - - expected1 := BasicPR{ - org: "test", - repo: "foo", - num: 4, - } - expected2 := BasicPR{ - org: "test", - repo: "goo", - num: 5, - } - - if !slices.Contains(out, expected1) { - t.Error("Unexpected", out, "Expected", expected1) - } - if !slices.Contains(out, expected2) { - t.Error("Unexpected", out, "Expected", expected2) - } - }) - - t.Run("Multiple PRs with whitespace", func(t *testing.T) { - const whitespacePRText = `Some header of the issue - - PR: test/goo#5 - - Followed by some description - PR: test/foo#4 -` - - desc, out := ExtractAssociatedDescriptionAndPRs(newStringScanner(whitespacePRText)) - if len(out) != 2 { - t.Error("Unexpected output", out) - return - } - - const expectedDesc = `Some header of the issue - - - Followed by some description` - expected1 := BasicPR{ - org: "test", - repo: "foo", - num: 4, - } - expected2 := BasicPR{ - org: "test", - repo: "goo", - num: 5, - } - - if !slices.Contains(out, expected1) { - t.Error("Unexpected", out, "Expected", expected1) - } - if !slices.Contains(out, expected2) { - t.Error("Unexpected", out, "Expected", expected2) - } - if desc != expectedDesc { - t.Error("unexpected desc", desc) - } - }) - - t.Run("Multiple PRs with missing names and other special cases to ignore", func(t *testing.T) { - const whitespacePRText = `Some header of the issue - - PR: foobar#5 - PR: rd/goo5 - PR: test/#5 - PR: /goo#5 - PR: test/goo# - PR: test / goo # 10 - PR: test/gool# 10 - PR: test/goo#5 - - Followed by some description - PR: test/foo#4 - - - ` - - desc, out := ExtractAssociatedDescriptionAndPRs(newStringScanner(whitespacePRText)) - if len(out) != 2 { - t.Error("Unexpected output", out) - return - } - - const expectedDesc = `Some header of the issue - - PR: foobar#5 - PR: rd/goo5 - PR: test/#5 - PR: /goo#5 - PR: test/goo# - PR: test / goo # 10 - PR: test/gool# 10 - - Followed by some description` - - if desc != expectedDesc { - t.Error(len(desc), "vs", len(expectedDesc)) - t.Error("description doesn't match expected. ", desc) - } - - expected1 := BasicPR{ - org: "test", - repo: "foo", - num: 4, - } - expected2 := BasicPR{ - org: "test", - repo: "goo", - num: 5, - } - - if !slices.Contains(out, expected1) { - t.Error("Unexpected", out, "Expected", expected1) - } - if !slices.Contains(out, expected2) { - t.Error("Unexpected", out, "Expected", expected2) - } - }) - - t.Run("Append PRs to end of description", func(t *testing.T) { - d := AppendPRsToDescription("something", []BasicPR{ - BasicPR{org: "a", repo: "b", num: 100}, + if desc != test.desc { + t.Error("Desc output", len(desc), "!=", len(test.desc), ":", desc) + } }) - - const expectedDesc = `something - -PR: a/b#100 -` - if d != expectedDesc { - t.Error(len(d), "vs", len(expectedDesc)) - t.Error("unpected output", d) - } - }) + } +} + +func TestAppendingPRsToDescription(t *testing.T) { + testTable := []struct { + name string + desc string + PRs []BasicPR + output string + }{ + { + "Append single PR to end of description", + "something", + []BasicPR{ + {org: "a", repo: "b", num: 100}, + }, + "something\n\nPR: a/b#100", + }, + { + "Append multiple PR to end of description", + "something", + []BasicPR{ + {org: "a1", repo: "b", num: 100}, + {org: "a1", repo: "c", num: 100}, + {org: "a1", repo: "c", num: 101}, + {org: "b", repo: "b", num: 100}, + {org: "c", repo: "b", num: 100}, + }, + "something\n\nPR: a1/b#100\nPR: a1/c#100\nPR: a1/c#101\nPR: b/b#100\nPR: c/b#100", + }, + { + "Append multiple sorted PR to end of description and remove dups", + "something", + []BasicPR{ + {org: "a1", repo: "c", num: 101}, + {org: "a1", repo: "c", num: 100}, + {org: "c", repo: "b", num: 100}, + {org: "b", repo: "b", num: 100}, + {org: "a1", repo: "c", num: 101}, + {org: "a1", repo: "c", num: 101}, + {org: "a1", repo: "b", num: 100}, + }, + "something\n\nPR: a1/b#100\nPR: a1/c#100\nPR: a1/c#101\nPR: b/b#100\nPR: c/b#100", + }, + } + + for _, test := range testTable { + t.Run(test.name, func(t *testing.T) { + d := AppendPRsToDescription(test.desc, test.PRs) + if d != test.output { + t.Error(len(d), "vs", len(test.output)) + t.Error("unpected output", d) + } + }) + } }