diff --git a/pkg/cmd/docs-issue-generation/BUILD.bazel b/pkg/cmd/docs-issue-generation/BUILD.bazel index 12b05970b424..8277e2bdcba1 100644 --- a/pkg/cmd/docs-issue-generation/BUILD.bazel +++ b/pkg/cmd/docs-issue-generation/BUILD.bazel @@ -23,11 +23,15 @@ go_binary( go_test( name = "docs-issue-generation_test", + size = "small", srcs = ["docs_issue_generation_test.go"], - args = ["-test.timeout=295s"], + args = ["-test.timeout=55s"], data = glob(["testdata/**"]), embed = [":docs-issue-generation_lib"], - deps = ["@com_github_stretchr_testify//assert"], + deps = [ + "//pkg/testutils", + "@com_github_stretchr_testify//assert", + ], ) get_x_data(name = "get_x_data") diff --git a/pkg/cmd/docs-issue-generation/docs_issue_generation.go b/pkg/cmd/docs-issue-generation/docs_issue_generation.go index 60a59ec5a7fe..95c5a3913cc6 100644 --- a/pkg/cmd/docs-issue-generation/docs_issue_generation.go +++ b/pkg/cmd/docs-issue-generation/docs_issue_generation.go @@ -205,7 +205,7 @@ var ( releaseJustificationRE = regexp.MustCompile(`(?i)release justification:.*`) prNumberRE = regexp.MustCompile(`Related PR: \[?https://github.com/cockroachdb/cockroach/pull/(\d+)\D`) commitShaRE = regexp.MustCompile(`Commit: \[?https://github.com/cockroachdb/cockroach/commit/(\w+)\W`) - exalateJiraRefRE = regexp.MustCompile(`\\n` + exalateJiraRefPart + `\\n`) + exalateJiraRefRE = regexp.MustCompile(exalateJiraRefPart) ) const ( @@ -223,7 +223,7 @@ func docsIssueGeneration(params parameters) { if err != nil { fmt.Println(err) } - docsIssues := constructDocsIssues(prs) + docsIssues := constructDocsIssues(prs, params.Token) if params.DryRun { fmt.Printf("Start time: %#v\n", params.StartTime.Format(time.RFC3339)) fmt.Printf("End time: %#v\n", params.EndTime.Format(time.RFC3339)) @@ -669,8 +669,12 @@ func containsBugFix(message string) bool { return false } +// org/repo#issue: PROJECT-NUMBER + // getJiraIssueFromGitHubIssue takes a GitHub issue and returns the appropriate Jira key from the issue body. -func getJiraIssueFromGitHubIssue(org, repo string, issue int, token string) string { +// getJiraIssueFromGitHubIssue is specified as a function closure to allow for testing +// of getJiraIssueFromGitHubIssue* methods. +var getJiraIssueFromGitHubIssue = func(org, repo string, issue int, token string) (string, error) { query := `query ($org: String!, $repo: String!, $issue: Int!) { repository(owner: $org, name: $repo) { issue(number: $issue) { @@ -687,32 +691,68 @@ func getJiraIssueFromGitHubIssue(org, repo string, issue int, token string) stri err := queryGraphQL(query, queryVariables, token, &search) if err != nil { fmt.Println(err) - return "" + return "", err } var jiraIssue string exalateRef := exalateJiraRefRE.FindString(search.Data.Repository.Issue.Body) if len(exalateRef) > 0 { jiraIssue = jiraIssuePartRE.FindString(exalateRef) } - return jiraIssue + return jiraIssue, nil +} + +func splitBySlashOrHash(r rune) bool { + return r == '/' || r == '#' } -func getJiraIssueFromRef(ref string) string { - if ghURLPartRE.MatchString(ref) || jiraURLPartRE.MatchString(ref) { +func getJiraIssueFromRef(ref, token string) string { + if jiraIssuePartRE.MatchString(ref) { return ref + } else if jiraURLPartRE.MatchString(ref) { + return strings.Replace(ref, "https://cockroachlabs.atlassian.net/browse/", "", 1) } else if ghIssueRepoPartRE.MatchString(ref) { - split := strings.Split(ref, "#") - return "https://github.com/" + split[0] + "/issues/" + split[1] + split := strings.FieldsFunc(ref, splitBySlashOrHash) + issueNumber, err := strconv.Atoi(split[2]) + if err != nil { + fmt.Println(err) + return "" + } + issueRef, err := getJiraIssueFromGitHubIssue(split[0], split[1], issueNumber, token) + if err != nil { + fmt.Println(err) + } + return issueRef } else if ghIssuePartRE.MatchString(ref) { - return "https://github.com/cockroachdb/cockroach/issues/" + strings.Replace(ref, "#", "", 1) - } else if jiraIssuePartRE.MatchString(ref) { - return "https://cockroachlabs.atlassian.net/browse/" + ref + issueNumber, err := strconv.Atoi(strings.Replace(ref, "#", "", 1)) + if err != nil { + fmt.Println(err) + return "" + } + issueRef, err := getJiraIssueFromGitHubIssue("cockroachdb", "cockroach", issueNumber, token) + if err != nil { + fmt.Println(err) + } + return issueRef + } else if ghURLPartRE.MatchString(ref) { + replace1 := strings.Replace(ref, "https://github.com/", "", 1) + replace2 := strings.Replace(replace1, "/issues", "", 1) + split := strings.FieldsFunc(replace2, splitBySlashOrHash) + issueNumber, err := strconv.Atoi(split[2]) + if err != nil { + fmt.Println(err) + return "" + } + issueRef, err := getJiraIssueFromGitHubIssue(split[0], split[1], issueNumber, token) + if err != nil { + fmt.Println(err) + } + return issueRef } else { return "Malformed epic/issue ref (" + ref + ")" } } -func extractIssueEpicRefs(prBody, commitBody string) string { +func extractIssueEpicRefs(prBody, commitBody, token string) string { refInfo := epicIssueRefInfo{ epicRefs: extractEpicIDs(commitBody + "\n" + prBody), epicNone: containsEpicNone(commitBody + "\n" + prBody), @@ -724,21 +764,21 @@ func extractIssueEpicRefs(prBody, commitBody string) string { if len(refInfo.epicRefs) > 0 { builder.WriteString("Epic:") for x := range refInfo.epicRefs { - builder.WriteString(" " + getJiraIssueFromRef(x)) + builder.WriteString(" " + getJiraIssueFromRef(x, token)) } builder.WriteString("\n") } if len(refInfo.issueCloseRefs) > 0 { builder.WriteString("Fixes:") for x := range refInfo.issueCloseRefs { - builder.WriteString(" " + getJiraIssueFromRef(x)) + builder.WriteString(" " + getJiraIssueFromRef(x, token)) } builder.WriteString("\n") } if len(refInfo.issueInformRefs) > 0 { builder.WriteString("Informs:") for x := range refInfo.issueInformRefs { - builder.WriteString(" " + getJiraIssueFromRef(x)) + builder.WriteString(" " + getJiraIssueFromRef(x, token)) } builder.WriteString("\n") } @@ -750,11 +790,11 @@ func extractIssueEpicRefs(prBody, commitBody string) string { // getIssues takes a list of commits from GitHub as well as the PR number associated with those commits and outputs a // formatted list of docs issues with valid release notes -func constructDocsIssues(prs []cockroachPR) []docsIssue { +func constructDocsIssues(prs []cockroachPR, token string) []docsIssue { var result []docsIssue for _, pr := range prs { for _, commit := range pr.Commits { - rns := formatReleaseNotes(commit.MessageBody, pr.Number, pr.Body, commit.Sha) + rns := formatReleaseNotes(commit.MessageBody, pr.Number, pr.Body, commit.Sha, token) for i, rn := range rns { x := docsIssue{ sourceCommitSha: commit.Sha, @@ -783,13 +823,13 @@ func formatTitle(title string, prNumber int, index int, totalLength int) string // formatReleaseNotes generates a list of docsIssue bodies for the docs repo based on a given CRDB sha func formatReleaseNotes( - commitMessage string, prNumber int, prBody string, crdbSha string, + commitMessage string, prNumber int, prBody, crdbSha, token string, ) []string { rnBodySlice := []string{} if releaseNoteNoneRE.MatchString(commitMessage) { return rnBodySlice } - epicIssueRefs := extractIssueEpicRefs(prBody, commitMessage) + epicIssueRefs := extractIssueEpicRefs(prBody, commitMessage, token) splitString := strings.Split(commitMessage, "\n") releaseNoteLines := []string{} var rnBody string diff --git a/pkg/cmd/docs-issue-generation/docs_issue_generation_test.go b/pkg/cmd/docs-issue-generation/docs_issue_generation_test.go index 730d5a6c8e9f..78ec9ea2bf2e 100644 --- a/pkg/cmd/docs-issue-generation/docs_issue_generation_test.go +++ b/pkg/cmd/docs-issue-generation/docs_issue_generation_test.go @@ -15,6 +15,7 @@ import ( "strconv" "testing" + "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/stretchr/testify/assert" ) @@ -103,7 +104,7 @@ func TestConstructDocsIssues(t *testing.T) { docsIssues []docsIssue }{ { - testName: "Single PR - 91345", + testName: "Single PR - 91345 - Epic: none", cockroachPRs: []cockroachPR{{ Title: "release-22.2: clusterversion: allow forcing release binary to dev version", Number: 91345, @@ -163,19 +164,19 @@ func TestConstructDocsIssues(t *testing.T) { { sourceCommitSha: "8d15073f329cf8d72e09977b34a3b339d1436000", title: "PR #91294 - ui: update filter labels", - body: "Related PR: https://github.com/cockroachdb/cockroach/pull/91294\nCommit: https://github.com/cockroachdb/cockroach/commit/8d15073f329cf8d72e09977b34a3b339d1436000\nFixes: https://github.com/cockroachdb/cockroach/issues/87960\n\n---\n\nRelease note (ui change): Update filter labels from\n\"App\" to \"Application Name\" and from \"Username\" to\n\"User Name\" on SQL Activity pages.", + body: "Related PR: https://github.com/cockroachdb/cockroach/pull/91294\nCommit: https://github.com/cockroachdb/cockroach/commit/8d15073f329cf8d72e09977b34a3b339d1436000\nFixes: CRDB-19614\n\n---\n\nRelease note (ui change): Update filter labels from\n\"App\" to \"Application Name\" and from \"Username\" to\n\"User Name\" on SQL Activity pages.", labels: []string{"C-product-change", "release-22.1"}, }, { sourceCommitSha: "1829a72664f28ddfa50324c9ff5352380029560b", title: "PR #90381 - sql/ttl: rename num_active_ranges metrics", - body: "Related PR: https://github.com/cockroachdb/cockroach/pull/90381\nCommit: https://github.com/cockroachdb/cockroach/commit/1829a72664f28ddfa50324c9ff5352380029560b\nFixes: https://github.com/cockroachdb/cockroach/issues/90094\n\n---\n\nRelease note (ops change): These TTL metrics have been renamed:\njobs.row_level_ttl.range_total_duration -> jobs.row_level_ttl.span_total_duration\njobs.row_level_ttl.num_active_ranges -> jobs.row_level_ttl.num_active_spans", + body: "Related PR: https://github.com/cockroachdb/cockroach/pull/90381\nCommit: https://github.com/cockroachdb/cockroach/commit/1829a72664f28ddfa50324c9ff5352380029560b\nFixes: CRDB-20636\n\n---\n\nRelease note (ops change): These TTL metrics have been renamed:\njobs.row_level_ttl.range_total_duration -> jobs.row_level_ttl.span_total_duration\njobs.row_level_ttl.num_active_ranges -> jobs.row_level_ttl.num_active_spans", labels: []string{"C-product-change", "release-22.2.0"}, }, { sourceCommitSha: "43de8ff30e3e6e1d9b2272ed4f62c543dc0a037c", title: "PR #89957 - opt/props: shallow-copy props.Histogram when applying selectivity", - body: "Related PR: https://github.com/cockroachdb/cockroach/pull/89957\nCommit: https://github.com/cockroachdb/cockroach/commit/43de8ff30e3e6e1d9b2272ed4f62c543dc0a037c\nFixes: https://github.com/cockroachdb/cockroach/issues/89941\n\n---\n\nRelease note (performance improvement): The optimizer now does less\ncopying of histograms while planning queries, which will reduce memory\npressure a little.", + body: "Related PR: https://github.com/cockroachdb/cockroach/pull/89957\nCommit: https://github.com/cockroachdb/cockroach/commit/43de8ff30e3e6e1d9b2272ed4f62c543dc0a037c\nFixes: CRDB-20505\n\n---\n\nRelease note (performance improvement): The optimizer now does less\ncopying of histograms while planning queries, which will reduce memory\npressure a little.", labels: []string{"C-product-change", "release-22.2"}, }, }, @@ -216,20 +217,33 @@ func TestConstructDocsIssues(t *testing.T) { docsIssues: []docsIssue{{ sourceCommitSha: "aaada3e2ac3b1b7268accfb1dcbe5464e948e9d1", title: "PR #123456 - Epic extraction PR", - body: "Related PR: https://github.com/cockroachdb/cockroach/pull/123456\nCommit: https://github.com/cockroachdb/cockroach/commit/aaada3e2ac3b1b7268accfb1dcbe5464e948e9d1\nEpic: https://cockroachlabs.atlassian.net/browse/CRDB-24680\n\n---\n\nRelease note (cli change): cli changes", + body: "Related PR: https://github.com/cockroachdb/cockroach/pull/123456\nCommit: https://github.com/cockroachdb/cockroach/commit/aaada3e2ac3b1b7268accfb1dcbe5464e948e9d1\nEpic: CRDB-24680\n\n---\n\nRelease note (cli change): cli changes", labels: []string{"C-product-change", "master"}, }}, }, } for _, tc := range testCases { t.Run(tc.testName, func(t *testing.T) { - result := constructDocsIssues(tc.cockroachPRs) + defer testutils.TestingHook(&getJiraIssueFromGitHubIssue, func(org, repo string, issue int, token string) (string, error) { + // getJiraIssueFromGitHubIssue requires a network call to the GitHub GraphQL API to calculate the Jira issue given + // a GitHub org/repo/issue. To help eliminate the need of a network call and minimize the chances of this test + // flaking, we define a pre-built map that is used to mock the network call and allow the tests to run as expected. + var ghJiraIssueMap = make(map[string]map[string]map[int]string) + ghJiraIssueMap["cockroachdb"] = make(map[string]map[int]string) + ghJiraIssueMap["cockroachdb"]["cockroach"] = make(map[int]string) + ghJiraIssueMap["cockroachdb"]["cockroach"][87960] = "CRDB-19614" + ghJiraIssueMap["cockroachdb"]["cockroach"][90094] = "CRDB-20636" + ghJiraIssueMap["cockroachdb"]["cockroach"][89941] = "CRDB-20505" + return ghJiraIssueMap[org][repo][issue], nil + })() + result := constructDocsIssues(tc.cockroachPRs, "") assert.Equal(t, tc.docsIssues, result) }) } } func TestFormatReleaseNotes(t *testing.T) { + testCases := []struct { prNum string prBody string @@ -259,7 +273,7 @@ columns, rather than resulting in an error. Any statistics in the JSON for existing columns will be injected successfully.`, rns: []string{`Related PR: https://github.com/cockroachdb/cockroach/pull/79069 Commit: https://github.com/cockroachdb/cockroach/commit/5ec9343b0e0a00bfd4603e55ca6533e2b77db2f9 -Informs: https://github.com/cockroachdb/cockroach/issues/68184 +Informs: CRDB-8919 --- @@ -349,8 +363,15 @@ Release note: None`, } for _, tc := range testCases { t.Run(tc.prNum, func(t *testing.T) { + defer testutils.TestingHook(&getJiraIssueFromGitHubIssue, func(org, repo string, issue int, token string) (string, error) { + var ghJiraIssueMap = make(map[string]map[string]map[int]string) + ghJiraIssueMap["cockroachdb"] = make(map[string]map[int]string) + ghJiraIssueMap["cockroachdb"]["cockroach"] = make(map[int]string) + ghJiraIssueMap["cockroachdb"]["cockroach"][68184] = "CRDB-8919" + return ghJiraIssueMap[org][repo][issue], nil + })() prNumInt, _ := strconv.Atoi(tc.prNum) - result := formatReleaseNotes(tc.commitMessage, prNumInt, tc.prBody, tc.sha) + result := formatReleaseNotes(tc.commitMessage, prNumInt, tc.prBody, tc.sha, "") assert.Equal(t, tc.rns, result) }) } @@ -430,40 +451,6 @@ func TestFormatTitle(t *testing.T) { } } -func TestGetUrlFromRef(t *testing.T) { - testCases := []struct { - ref string - result string - }{ - { - ref: "#12345", - result: "https://github.com/cockroachdb/cockroach/issues/12345", - }, - { - ref: "CRDB-54321", - result: "https://cockroachlabs.atlassian.net/browse/CRDB-54321", - }, - { - ref: "cockroachlabs/release-staging#23456", - result: "https://github.com/cockroachlabs/release-staging/issues/23456", - }, - { - ref: "https://github.com/cockroachdb/cockroach/issues/98765", - result: "https://github.com/cockroachdb/cockroach/issues/98765", - }, - { - ref: "https://cockroachlabs.atlassian.net/browse/CRDB-56789", - result: "https://cockroachlabs.atlassian.net/browse/CRDB-56789", - }, - } - for _, tc := range testCases { - t.Run(tc.ref, func(t *testing.T) { - result := getJiraIssueFromRef(tc.ref) - assert.Equal(t, tc.result, result) - }) - } -} - func TestExtractFixIssuesIDs(t *testing.T) { testCases := []struct { message string