Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Pass in Jira tickets in the body of product change issues #97789

Merged
merged 2 commits into from
Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions pkg/cmd/docs-issue-generation/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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")
137 changes: 109 additions & 28 deletions pkg/cmd/docs-issue-generation/docs_issue_generation.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,16 @@ type gqlDocsIssue struct {
} `json:"data"`
}

type gqlSingleIssue struct {
Data struct {
Repository struct {
Issue struct {
Body string `json:"body"`
} `json:"issue"`
} `json:"repository"`
} `json:"data"`
}

// gqlDocsRepoLabels contains details about the labels within the cockroach repo. In order to create issues using the
// GraphQL API, we need to use the label IDs and no
type gqlDocsRepoLabels struct {
Expand Down Expand Up @@ -160,13 +170,15 @@ type epicIssueRefInfo struct {

// Regex components for finding and validating issue and epic references in a string
var (
ghIssuePart = `(#\d+)` // e.g., #12345
ghIssueRepoPart = `([\w.-]+[/][\w.-]+#\d+)` // e.g., cockroachdb/cockroach#12345
ghURLPart = `(https://github.com/[-a-z0-9]+/[-._a-z0-9/]+/issues/\d+)` // e.g., https://github.com/cockroachdb/cockroach/issues/12345
jiraIssuePart = `([[:alpha:]]+-\d+)` // e.g., DOC-3456
jiraURLPart = "https://cockroachlabs.atlassian.net/browse/" + jiraIssuePart // e.g., https://cockroachlabs.atlassian.net/browse/DOC-3456
issueRefPart = ghIssuePart + "|" + ghIssueRepoPart + "|" + ghURLPart + "|" + jiraIssuePart + "|" + jiraURLPart
afterRefPart = `[,.;]?(?:[ \t\n\r]+|$)`
ghIssuePart = `(#\d+)` // e.g., #12345
ghIssueRepoPart = `([\w.-]+[/][\w.-]+#\d+)` // e.g., cockroachdb/cockroach#12345
ghURLPart = `(https://github.com/[-a-z0-9]+/[-._a-z0-9/]+/issues/\d+)` // e.g., https://github.com/cockroachdb/cockroach/issues/12345
jiraIssuePart = `([[:alpha:]]+-\d+)` // e.g., DOC-3456
exalateJiraRefPart = `Jira issue: ` + jiraIssuePart // e.g., Jira issue: CRDB-54321
jiraBaseUrlPart = "https://cockroachlabs.atlassian.net/browse/"
jiraURLPart = jiraBaseUrlPart + jiraIssuePart // e.g., https://cockroachlabs.atlassian.net/browse/DOC-3456
issueRefPart = ghIssuePart + "|" + ghIssueRepoPart + "|" + ghURLPart + "|" + jiraIssuePart + "|" + jiraURLPart
afterRefPart = `[,.;]?(?:[ \t\n\r]+|$)`
)

// RegExes of each issue part
Expand All @@ -193,6 +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(exalateJiraRefPart)
)

const (
Expand All @@ -210,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))
Expand Down Expand Up @@ -656,48 +669,116 @@ func containsBugFix(message string) bool {
return false
}

func getUrlFromRef(ref string) string {
if ghURLPartRE.MatchString(ref) || jiraURLPartRE.MatchString(ref) {
// org/repo#issue: PROJECT-NUMBER

// getJiraIssueFromGitHubIssue takes a GitHub issue and returns the appropriate Jira key from the issue body.
// 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) {
body
}
}
}`
var search gqlSingleIssue
queryVariables := map[string]interface{}{
"org": org,
"repo": repo,
"issue": issue,
}
err := queryGraphQL(query, queryVariables, token, &search)
if err != nil {
fmt.Println(err)
return "", err
}
var jiraIssue string
exalateRef := exalateJiraRefRE.FindString(search.Data.Repository.Issue.Body)
if len(exalateRef) > 0 {
jiraIssue = jiraIssuePartRE.FindString(exalateRef)
}
return jiraIssue, nil
}

func splitBySlashOrHash(r rune) bool {
return r == '/' || r == '#'
}

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(prBody + "\n" + commitBody),
epicNone: containsEpicNone(prBody + "\n" + commitBody),
issueCloseRefs: extractFixIssueIDs(prBody + "\n" + commitBody),
issueInformRefs: extractInformIssueIDs(prBody + "\n" + commitBody),
isBugFix: containsBugFix(prBody + "\n" + commitBody),
epicRefs: extractEpicIDs(commitBody + "\n" + prBody),
epicNone: containsEpicNone(commitBody + "\n" + prBody),
issueCloseRefs: extractFixIssueIDs(commitBody + "\n" + prBody),
issueInformRefs: extractInformIssueIDs(commitBody + "\n" + prBody),
isBugFix: containsBugFix(commitBody + "\n" + prBody),
}
var builder strings.Builder
if len(refInfo.epicRefs) > 0 {
builder.WriteString("Epic:")
for x := range refInfo.epicRefs {
builder.WriteString(" " + getUrlFromRef(x))
builder.WriteString(" " + getJiraIssueFromRef(x, token))
}
builder.WriteString("\n")
}
if len(refInfo.issueCloseRefs) > 0 {
builder.WriteString("Fixes:")
for x := range refInfo.issueCloseRefs {
builder.WriteString(" " + getUrlFromRef(x))
builder.WriteString(" " + getJiraIssueFromRef(x, token))
}
builder.WriteString("\n")
}
if len(refInfo.issueInformRefs) > 0 {
builder.WriteString("Informs:")
for x := range refInfo.issueInformRefs {
builder.WriteString(" " + getUrlFromRef(x))
builder.WriteString(" " + getJiraIssueFromRef(x, token))
}
builder.WriteString("\n")
}
Expand All @@ -709,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,
Expand Down Expand Up @@ -742,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
Expand Down
71 changes: 29 additions & 42 deletions pkg/cmd/docs-issue-generation/docs_issue_generation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"strconv"
"testing"

"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"},
},
},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

---

Expand Down Expand Up @@ -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)
})
}
Expand Down Expand Up @@ -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 := getUrlFromRef(tc.ref)
assert.Equal(t, tc.result, result)
})
}
}

func TestExtractFixIssuesIDs(t *testing.T) {
testCases := []struct {
message string
Expand Down