From 5dd71c140410e215f50a9e806b90211d83999054 Mon Sep 17 00:00:00 2001 From: Peter Mattis Date: Thu, 17 May 2018 09:56:14 -0400 Subject: [PATCH 1/2] issues: initial fork of code from cmd/github-post No changes. This is a pure copy of the code to ease subsequent diffs. Release note: None --- pkg/cmd/internal/issues/issues.go | 396 +++++++++++++++++++++++++ pkg/cmd/internal/issues/issues_test.go | 251 ++++++++++++++++ 2 files changed, 647 insertions(+) create mode 100644 pkg/cmd/internal/issues/issues.go create mode 100644 pkg/cmd/internal/issues/issues_test.go diff --git a/pkg/cmd/internal/issues/issues.go b/pkg/cmd/internal/issues/issues.go new file mode 100644 index 000000000000..8513de55eaf9 --- /dev/null +++ b/pkg/cmd/internal/issues/issues.go @@ -0,0 +1,396 @@ +// Copyright 2016 The Cockroach Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +// implied. See the License for the specific language governing +// permissions and limitations under the License. + +package main + +import ( + "bytes" + "context" + "fmt" + "io" + "log" + "net/url" + "os" + "os/exec" + "regexp" + "strings" + + "golang.org/x/oauth2" + + "github.com/google/go-github/github" + version "github.com/hashicorp/go-version" + "github.com/pkg/errors" + "github.com/tebeka/go2xunit/lib" +) + +const githubAPITokenEnv = "GITHUB_API_TOKEN" +const teamcityVCSNumberEnv = "BUILD_VCS_NUMBER" +const teamcityBuildIDEnv = "TC_BUILD_ID" +const teamcityServerURLEnv = "TC_SERVER_URL" +const githubUser = "cockroachdb" +const githubRepo = "cockroach" + +const pkgEnv = "PKG" +const tagsEnv = "TAGS" +const goFlagsEnv = "GOFLAGS" +const cockroachPkgPrefix = "github.com/cockroachdb/cockroach/pkg/" + +var issueLabels = []string{"O-robot", "C-test-failure"} + +// Based on the following observed API response: +// +// 422 Validation Failed [{Resource:Issue Field:body Code:custom Message:body is too long (maximum is 65536 characters)}] +const githubIssueBodyMaximumLength = 1<<16 - 1 + +func main() { + token, ok := os.LookupEnv(githubAPITokenEnv) + if !ok { + log.Fatalf("GitHub API token environment variable %s is not set", githubAPITokenEnv) + } + + ctx := context.Background() + + client := github.NewClient(oauth2.NewClient(ctx, oauth2.StaticTokenSource( + &oauth2.Token{AccessToken: token}, + ))) + + getLatestTag := func() (string, error) { + cmd := exec.Command("git", "describe", "--abbrev=0", "--tags") + out, err := cmd.CombinedOutput() + if err != nil { + return "", err + } + return strings.TrimSpace(string(out)), nil + } + + if err := runGH( + ctx, + os.Stdin, + client.Issues.Create, + client.Search.Issues, + client.Issues.CreateComment, + client.Repositories.ListCommits, + client.Issues.ListMilestones, + getLatestTag, + ); err != nil { + log.Fatal(err) + } +} + +var stacktraceRE = regexp.MustCompile(`(?m:^goroutine\s\d+)`) + +func trimIssueRequestBody(message string, usedCharacters int) string { + maxLength := githubIssueBodyMaximumLength - usedCharacters + + if m := stacktraceRE.FindStringIndex(message); m != nil { + // We want the top stack traces plus a few lines before. + { + startIdx := m[0] + for i := 0; i < 100; i++ { + if idx := strings.LastIndexByte(message[:startIdx], '\n'); idx != -1 { + startIdx = idx + } + } + message = message[startIdx:] + } + for len(message) > maxLength { + if idx := strings.LastIndexByte(message, '\n'); idx != -1 { + message = message[:idx] + } else { + message = message[:maxLength] + } + } + } else { + // We want the FAIL line. + for len(message) > maxLength { + if idx := strings.IndexByte(message, '\n'); idx != -1 { + message = message[idx+1:] + } else { + message = message[len(message)-maxLength:] + } + } + } + + return message +} + +// If the assignee would be the key in this map, assign to the value instead. +// Helpful to avoid pinging former employees. +var oldFriendsMap = map[string]string{ + "tamird": "tschottdorf", +} + +func getAssignee( + ctx context.Context, + packageName, testName string, + listCommits func(ctx context.Context, owner string, repo string, opts *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error), +) (string, error) { + // Search the source code for the email address of the last committer to touch + // the first line of the source code that contains testName. Then, ask GitHub + // for the GitHub username of the user with that email address by searching + // commits in cockroachdb/cockroach for commits authored by the address. + subtests := strings.Split(testName, "/") + testName = subtests[0] + packageName = strings.TrimPrefix(packageName, "github.com/cockroachdb/cockroach/") + cmd := exec.Command(`/bin/bash`, `-c`, fmt.Sprintf(`git grep -n "func %s" $(git rev-parse --show-toplevel)/%s/*_test.go`, testName, packageName)) + // This command returns output such as: + // ../ccl/storageccl/export_test.go:31:func TestExportCmd(t *testing.T) { + out, err := cmd.CombinedOutput() + if err != nil { + return "", errors.Errorf("couldn't find test %s in %s: %s %s", testName, packageName, err, string(out)) + } + re := regexp.MustCompile(`(.*):(.*):`) + // The first 2 :-delimited fields are the filename and line number. + matches := re.FindSubmatch(out) + if matches == nil { + return "", errors.Errorf("couldn't find filename/line number for test %s in %s: %s", testName, packageName, string(out)) + } + filename := matches[1] + linenum := matches[2] + + // Now run git blame. + cmd = exec.Command(`/bin/bash`, `-c`, fmt.Sprintf(`git blame --porcelain -L%s,+1 %s | grep author-mail`, linenum, filename)) + // This command returns output such as: + // author-mail + out, err = cmd.CombinedOutput() + if err != nil { + return "", errors.Errorf("couldn't find author of test %s in %s: %s %s", testName, packageName, err, string(out)) + } + re = regexp.MustCompile("author-mail <(.*)>") + matches = re.FindSubmatch(out) + if matches == nil { + return "", errors.Errorf("couldn't find author email of test %s in %s: %s", testName, packageName, string(out)) + } + email := string(matches[1]) + + commits, _, err := listCommits(ctx, githubUser, githubRepo, &github.CommitsListOptions{ + Author: email, + ListOptions: github.ListOptions{ + PerPage: 1, + }, + }) + if err != nil { + return "", err + } + if len(commits) == 0 { + return "", errors.Errorf("couldn't find GitHub commits for user email %s", email) + } + + assignee := *commits[0].Author.Login + + if newAssignee, ok := oldFriendsMap[assignee]; ok { + assignee = newAssignee + } + return assignee, nil +} + +func getProbableMilestone( + ctx context.Context, + getLatestTag func() (string, error), + listMilestones func(ctx context.Context, owner string, repo string, opt *github.MilestoneListOptions) ([]*github.Milestone, *github.Response, error), +) *int { + tag, err := getLatestTag() + if err != nil { + log.Printf("unable to get latest tag: %s", err) + log.Printf("issues will be posted without milestone") + return nil + } + + v, err := version.NewVersion(tag) + if err != nil { + log.Printf("unable to parse version from tag: %s", err) + log.Printf("issues will be posted without milestone") + return nil + } + if len(v.Segments()) < 2 { + log.Printf("version %s has less than two components; issues will be posted without milestone", tag) + return nil + } + vstring := fmt.Sprintf("%d.%d", v.Segments()[0], v.Segments()[1]) + + milestones, _, err := listMilestones(ctx, githubUser, githubRepo, &github.MilestoneListOptions{ + State: "open", + }) + if err != nil { + log.Printf("unable to list milestones: %s", err) + log.Printf("issues will be posted without milestone") + return nil + } + + for _, m := range milestones { + if m.GetTitle() == vstring { + return m.Number + } + } + return nil +} + +func runGH( + ctx context.Context, + input io.Reader, + createIssue func(ctx context.Context, owner string, repo string, issue *github.IssueRequest) (*github.Issue, *github.Response, error), + searchIssues func(ctx context.Context, query string, opt *github.SearchOptions) (*github.IssuesSearchResult, *github.Response, error), + createComment func(ctx context.Context, owner string, repo string, number int, comment *github.IssueComment) (*github.IssueComment, *github.Response, error), + listCommits func(ctx context.Context, owner string, repo string, opts *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error), + listMilestones func(ctx context.Context, owner string, repo string, opt *github.MilestoneListOptions) ([]*github.Milestone, *github.Response, error), + getLatestTag func() (string, error), +) error { + sha, ok := os.LookupEnv(teamcityVCSNumberEnv) + if !ok { + return errors.Errorf("VCS number environment variable %s is not set", teamcityVCSNumberEnv) + } + + buildID, ok := os.LookupEnv(teamcityBuildIDEnv) + if !ok { + log.Fatalf("teamcity build ID environment variable %s is not set", teamcityBuildIDEnv) + } + + serverURL, ok := os.LookupEnv(teamcityServerURLEnv) + if !ok { + log.Fatalf("teamcity server URL environment variable %s is not set", teamcityServerURLEnv) + } + + options := url.Values{} + options.Add("buildId", buildID) + options.Add("tab", "buildLog") + + u, err := url.Parse(serverURL) + if err != nil { + log.Fatal(err) + } + u.Scheme = "https" + u.Path = "viewLog.html" + u.RawQuery = options.Encode() + + var inputBuf bytes.Buffer + input = io.TeeReader(input, &inputBuf) + + var parameters []string + for _, parameter := range []string{ + tagsEnv, + goFlagsEnv, + } { + if val, ok := os.LookupEnv(parameter); ok { + parameters = append(parameters, parameter+"="+val) + } + } + parametersStr := "```\n" + strings.Join(parameters, "\n") + "\n```" + const bodyTemplate = `SHA: https://github.com/cockroachdb/cockroach/commits/%s + +Parameters: +%s + +Stress build found a failed test: %s` + + milestone := getProbableMilestone(ctx, getLatestTag, listMilestones) + + newIssueRequest := func(packageName, testName, message, assignee string) *github.IssueRequest { + title := fmt.Sprintf("%s: %s failed under stress", + strings.TrimPrefix(packageName, cockroachPkgPrefix), testName) + body := fmt.Sprintf(bodyTemplate, sha, parametersStr, u.String()) + "\n\n```\n%s\n```" + // We insert a raw "%s" above so we can figure out the length of the + // body so far, without the actual error text. We need this length so we + // can calculate the maximum amount of error text we can include in the + // issue without exceeding GitHub's limit. We replace that %s in the + // following Sprintf. + body = fmt.Sprintf(body, trimIssueRequestBody(message, len(body))) + + return &github.IssueRequest{ + Title: &title, + Body: &body, + Labels: &issueLabels, + Assignee: &assignee, + Milestone: milestone, + } + } + + newIssueComment := func(packageName, testName string) *github.IssueComment { + body := fmt.Sprintf(bodyTemplate, sha, parametersStr, u.String()) + return &github.IssueComment{Body: &body} + } + + suites, err := lib.ParseGotest(input, "") + if err != nil { + return errors.Wrap(err, "failed to parse `go test` output") + } + posted := false + for _, suite := range suites { + packageName := suite.Name + if packageName == "" { + var ok bool + packageName, ok = os.LookupEnv(pkgEnv) + if !ok { + log.Fatalf("package name environment variable %s is not set", pkgEnv) + } + } + for _, test := range suite.Tests { + switch test.Status { + case lib.Failed: + assignee, err := getAssignee(ctx, packageName, test.Name, listCommits) + if err != nil { + // if we *can't* assign anyone, sigh, feel free to hard-code me. + // -- tschottdorf, 11/3/2017 + assignee = "tschottdorf" + test.Message += fmt.Sprintf("\n\nFailed to find issue assignee: \n%s", err) + } + issueRequest := newIssueRequest(packageName, test.Name, test.Message, assignee) + searchQuery := fmt.Sprintf(`"%s" user:%s repo:%s is:open`, *issueRequest.Title, githubUser, githubRepo) + for _, label := range issueLabels { + searchQuery = searchQuery + fmt.Sprintf(` label:"%s"`, label) + } + + var foundIssue *int + + result, _, err := searchIssues(ctx, searchQuery, &github.SearchOptions{ + ListOptions: github.ListOptions{ + PerPage: 1, + }, + }) + if err != nil { + return errors.Wrapf(err, "failed to search GitHub with query %s", github.Stringify(searchQuery)) + } + if *result.Total > 0 { + foundIssue = result.Issues[0].Number + } + + if foundIssue == nil { + if _, _, err := createIssue(ctx, githubUser, githubRepo, issueRequest); err != nil { + return errors.Wrapf(err, "failed to create GitHub issue %s", github.Stringify(issueRequest)) + } + } else { + comment := newIssueComment(packageName, test.Name) + if _, _, err := createComment(ctx, githubUser, githubRepo, *foundIssue, comment); err != nil { + return errors.Wrapf(err, "failed to update issue #%d with %s", *foundIssue, github.Stringify(comment)) + } + } + posted = true + } + } + } + + const unknown = "(unknown)" + + if !posted { + packageName, ok := os.LookupEnv(pkgEnv) + if !ok { + packageName = unknown + } + issueRequest := newIssueRequest(packageName, unknown, inputBuf.String(), "") + if _, _, err := createIssue(ctx, githubUser, githubRepo, issueRequest); err != nil { + return errors.Wrapf(err, "failed to create GitHub issue %s", github.Stringify(issueRequest)) + } + } + + return nil +} diff --git a/pkg/cmd/internal/issues/issues_test.go b/pkg/cmd/internal/issues/issues_test.go new file mode 100644 index 000000000000..e629b308d66e --- /dev/null +++ b/pkg/cmd/internal/issues/issues_test.go @@ -0,0 +1,251 @@ +// Copyright 2016 The Cockroach Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +// implied. See the License for the specific language governing +// permissions and limitations under the License. + +package main + +import ( + "context" + "fmt" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" + "testing" + + "github.com/google/go-github/github" +) + +func TestRunGH(t *testing.T) { + const ( + expOwner = "cockroachdb" + expRepo = "cockroach" + expAssignee = "hodor" + expMilestone = 2 + envPkg = "storage" + envTags = "deadlock" + envGoFlags = "race" + sha = "abcd123" + serverURL = "https://teamcity.example.com" + buildID = 8008135 + issueID = 1337 + issueNumber = 30 + ) + + for key, value := range map[string]string{ + teamcityVCSNumberEnv: sha, + teamcityServerURLEnv: serverURL, + teamcityBuildIDEnv: strconv.Itoa(buildID), + + pkgEnv: cockroachPkgPrefix + envPkg, + tagsEnv: envTags, + goFlagsEnv: envGoFlags, + } { + if val, ok := os.LookupEnv(key); ok { + defer func() { + if err := os.Setenv(key, val); err != nil { + t.Error(err) + } + }() + } else { + defer func() { + if err := os.Unsetenv(key); err != nil { + t.Error(err) + } + }() + } + + if err := os.Setenv(key, value); err != nil { + t.Fatal(err) + } + } + + parameters := "```\n" + strings.Join([]string{ + tagsEnv + "=" + envTags, + goFlagsEnv + "=" + envGoFlags, + }, "\n") + "\n```" + + for fileName, expectations := range map[string]struct { + packageName string + testName string + body string + }{ + "stress-failure": { + packageName: envPkg, + testName: "TestReplicateQueueRebalance", + body: " :12: storage/replicate_queue_test.go:103, condition failed to evaluate within 45s: not balanced: [10 1 10 1 8]", + }, + "stress-fatal": { + packageName: envPkg, + testName: "TestGossipHandlesReplacedNode", + body: "F170517 07:33:43.763059 69575 storage/replica.go:1360 [n3,s3,r1/3:/M{in-ax}] on-disk and in-memory state diverged:", + }, + } { + for _, foundIssue := range []bool{true, false} { + testName := fileName + if foundIssue { + testName = testName + "-existing-issue" + } + t.Run(testName, func(t *testing.T) { + file, err := os.Open(filepath.Join("testdata", fileName)) + if err != nil { + t.Fatal(err) + } + + reString := fmt.Sprintf(`(?s)\ASHA: https://github.com/cockroachdb/cockroach/commits/%s + +Parameters: +%s + +Stress build found a failed test: %s`, + regexp.QuoteMeta(sha), + regexp.QuoteMeta(parameters), + regexp.QuoteMeta(fmt.Sprintf("%s/viewLog.html?buildId=%d&tab=buildLog", serverURL, buildID)), + ) + + issueBodyRe, err := regexp.Compile( + fmt.Sprintf(reString+` + +.* +%s +`, regexp.QuoteMeta(expectations.body)), + ) + if err != nil { + t.Fatal(err) + } + commentBodyRe, err := regexp.Compile(reString) + if err != nil { + t.Fatal(err) + } + + issueCount := 0 + commentCount := 0 + postIssue := func(_ context.Context, owner string, repo string, issue *github.IssueRequest) (*github.Issue, *github.Response, error) { + issueCount++ + if owner != expOwner { + t.Fatalf("got %s, expected %s", owner, expOwner) + } + if repo != expRepo { + t.Fatalf("got %s, expected %s", repo, expRepo) + } + if *issue.Assignee != expAssignee { + t.Fatalf("got %s, expected %s", *issue.Assignee, expAssignee) + } + if expected := fmt.Sprintf("%s: %s failed under stress", expectations.packageName, expectations.testName); *issue.Title != expected { + t.Fatalf("got %s, expected %s", *issue.Title, expected) + } + if !issueBodyRe.MatchString(*issue.Body) { + t.Fatalf("got:\n%s\nexpected:\n%s", *issue.Body, issueBodyRe) + } + if length := len(*issue.Body); length > githubIssueBodyMaximumLength { + t.Fatalf("issue length %d exceeds (undocumented) maximum %d", length, githubIssueBodyMaximumLength) + } + if *issue.Milestone != expMilestone { + t.Fatalf("expected milestone %d, but got %d", expMilestone, *issue.Milestone) + } + return &github.Issue{ID: github.Int64(issueID)}, nil, nil + } + searchIssues := func(_ context.Context, query string, opt *github.SearchOptions) (*github.IssuesSearchResult, *github.Response, error) { + total := 0 + if foundIssue { + total = 1 + } + return &github.IssuesSearchResult{ + Total: &total, + Issues: []github.Issue{ + {Number: github.Int(issueNumber)}, + }, + }, nil, nil + } + postComment := func(_ context.Context, owner string, repo string, number int, comment *github.IssueComment) (*github.IssueComment, *github.Response, error) { + if owner != expOwner { + t.Fatalf("got %s, expected %s", owner, expOwner) + } + if repo != expRepo { + t.Fatalf("got %s, expected %s", repo, expRepo) + } + if !commentBodyRe.MatchString(*comment.Body) { + t.Fatalf("got:\n%s\nexpected:\n%s", *comment.Body, issueBodyRe) + } + if length := len(*comment.Body); length > githubIssueBodyMaximumLength { + t.Fatalf("comment length %d exceeds (undocumented) maximum %d", length, githubIssueBodyMaximumLength) + } + commentCount++ + + return nil, nil, nil + } + + listCommits := func(_ context.Context, owner string, repo string, opts *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error) { + if owner != expOwner { + t.Fatalf("got %s, expected %s", owner, expOwner) + } + if repo != expRepo { + t.Fatalf("got %s, expected %s", repo, expRepo) + } + if opts.Author == "" { + t.Fatalf("found no author, but expected one") + } + assignee := expAssignee + return []*github.RepositoryCommit{ + { + Author: &github.User{ + Login: &assignee, + }, + }, + }, nil, nil + } + + listMilestones := func(_ context.Context, owner, repo string, _ *github.MilestoneListOptions) ([]*github.Milestone, *github.Response, error) { + if owner != expOwner { + t.Fatalf("got %s, expected %s", owner, expOwner) + } + if repo != expRepo { + t.Fatalf("got %s, expected %s", repo, expRepo) + } + return []*github.Milestone{ + {Title: github.String("3.3"), Number: github.Int(expMilestone)}, + {Title: github.String("3.2"), Number: github.Int(1)}, + }, nil, nil + } + + getLatestTag := func() (string, error) { return "3.3", nil } + + if err := runGH( + context.Background(), + file, + postIssue, + searchIssues, + postComment, + listCommits, + listMilestones, + getLatestTag, + ); err != nil { + t.Fatal(err) + } + expectedIssues := 1 + expectedComments := 0 + if foundIssue { + expectedIssues = 0 + expectedComments = 1 + } + if issueCount != expectedIssues { + t.Fatalf("%d issues were posted, expected %d", issueCount, expectedIssues) + } + if commentCount != expectedComments { + t.Fatalf("%d comments were posted, expected %d", commentCount, expectedComments) + } + }) + } + } +} From 50733d5939ec02ea1f1652eaf34287b75156509e Mon Sep 17 00:00:00 2001 From: Peter Mattis Date: Thu, 17 May 2018 09:56:45 -0400 Subject: [PATCH 2/2] cmd/github-post: refactor issue posting into a library See #24894 Release note: None --- pkg/cmd/github-post/main.go | 341 +----------------- pkg/cmd/github-post/main_test.go | 232 +++---------- pkg/cmd/github-post/testdata/stress-unknown | 23 ++ pkg/cmd/internal/issues/issues.go | 363 ++++++++++---------- pkg/cmd/internal/issues/issues_test.go | 86 ++--- 5 files changed, 315 insertions(+), 730 deletions(-) create mode 100644 pkg/cmd/github-post/testdata/stress-unknown diff --git a/pkg/cmd/github-post/main.go b/pkg/cmd/github-post/main.go index 8513de55eaf9..9b1420d115e0 100644 --- a/pkg/cmd/github-post/main.go +++ b/pkg/cmd/github-post/main.go @@ -17,313 +17,44 @@ package main import ( "bytes" "context" - "fmt" "io" "log" - "net/url" "os" - "os/exec" - "regexp" - "strings" - "golang.org/x/oauth2" - - "github.com/google/go-github/github" - version "github.com/hashicorp/go-version" + "github.com/cockroachdb/cockroach/pkg/cmd/internal/issues" "github.com/pkg/errors" "github.com/tebeka/go2xunit/lib" ) -const githubAPITokenEnv = "GITHUB_API_TOKEN" -const teamcityVCSNumberEnv = "BUILD_VCS_NUMBER" -const teamcityBuildIDEnv = "TC_BUILD_ID" -const teamcityServerURLEnv = "TC_SERVER_URL" -const githubUser = "cockroachdb" -const githubRepo = "cockroach" - -const pkgEnv = "PKG" -const tagsEnv = "TAGS" -const goFlagsEnv = "GOFLAGS" -const cockroachPkgPrefix = "github.com/cockroachdb/cockroach/pkg/" - -var issueLabels = []string{"O-robot", "C-test-failure"} - -// Based on the following observed API response: -// -// 422 Validation Failed [{Resource:Issue Field:body Code:custom Message:body is too long (maximum is 65536 characters)}] -const githubIssueBodyMaximumLength = 1<<16 - 1 +const ( + pkgEnv = "PKG" +) func main() { - token, ok := os.LookupEnv(githubAPITokenEnv) - if !ok { - log.Fatalf("GitHub API token environment variable %s is not set", githubAPITokenEnv) - } - ctx := context.Background() - - client := github.NewClient(oauth2.NewClient(ctx, oauth2.StaticTokenSource( - &oauth2.Token{AccessToken: token}, - ))) - - getLatestTag := func() (string, error) { - cmd := exec.Command("git", "describe", "--abbrev=0", "--tags") - out, err := cmd.CombinedOutput() - if err != nil { - return "", err - } - return strings.TrimSpace(string(out)), nil - } - - if err := runGH( - ctx, - os.Stdin, - client.Issues.Create, - client.Search.Issues, - client.Issues.CreateComment, - client.Repositories.ListCommits, - client.Issues.ListMilestones, - getLatestTag, - ); err != nil { + if err := listFailures(ctx, os.Stdin, postIssue); err != nil { log.Fatal(err) } } -var stacktraceRE = regexp.MustCompile(`(?m:^goroutine\s\d+)`) - -func trimIssueRequestBody(message string, usedCharacters int) string { - maxLength := githubIssueBodyMaximumLength - usedCharacters - - if m := stacktraceRE.FindStringIndex(message); m != nil { - // We want the top stack traces plus a few lines before. - { - startIdx := m[0] - for i := 0; i < 100; i++ { - if idx := strings.LastIndexByte(message[:startIdx], '\n'); idx != -1 { - startIdx = idx - } - } - message = message[startIdx:] - } - for len(message) > maxLength { - if idx := strings.LastIndexByte(message, '\n'); idx != -1 { - message = message[:idx] - } else { - message = message[:maxLength] - } - } - } else { - // We want the FAIL line. - for len(message) > maxLength { - if idx := strings.IndexByte(message, '\n'); idx != -1 { - message = message[idx+1:] - } else { - message = message[len(message)-maxLength:] - } - } - } - - return message -} - -// If the assignee would be the key in this map, assign to the value instead. -// Helpful to avoid pinging former employees. -var oldFriendsMap = map[string]string{ - "tamird": "tschottdorf", -} - -func getAssignee( - ctx context.Context, - packageName, testName string, - listCommits func(ctx context.Context, owner string, repo string, opts *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error), -) (string, error) { - // Search the source code for the email address of the last committer to touch - // the first line of the source code that contains testName. Then, ask GitHub - // for the GitHub username of the user with that email address by searching - // commits in cockroachdb/cockroach for commits authored by the address. - subtests := strings.Split(testName, "/") - testName = subtests[0] - packageName = strings.TrimPrefix(packageName, "github.com/cockroachdb/cockroach/") - cmd := exec.Command(`/bin/bash`, `-c`, fmt.Sprintf(`git grep -n "func %s" $(git rev-parse --show-toplevel)/%s/*_test.go`, testName, packageName)) - // This command returns output such as: - // ../ccl/storageccl/export_test.go:31:func TestExportCmd(t *testing.T) { - out, err := cmd.CombinedOutput() - if err != nil { - return "", errors.Errorf("couldn't find test %s in %s: %s %s", testName, packageName, err, string(out)) - } - re := regexp.MustCompile(`(.*):(.*):`) - // The first 2 :-delimited fields are the filename and line number. - matches := re.FindSubmatch(out) - if matches == nil { - return "", errors.Errorf("couldn't find filename/line number for test %s in %s: %s", testName, packageName, string(out)) - } - filename := matches[1] - linenum := matches[2] - - // Now run git blame. - cmd = exec.Command(`/bin/bash`, `-c`, fmt.Sprintf(`git blame --porcelain -L%s,+1 %s | grep author-mail`, linenum, filename)) - // This command returns output such as: - // author-mail - out, err = cmd.CombinedOutput() - if err != nil { - return "", errors.Errorf("couldn't find author of test %s in %s: %s %s", testName, packageName, err, string(out)) - } - re = regexp.MustCompile("author-mail <(.*)>") - matches = re.FindSubmatch(out) - if matches == nil { - return "", errors.Errorf("couldn't find author email of test %s in %s: %s", testName, packageName, string(out)) - } - email := string(matches[1]) - - commits, _, err := listCommits(ctx, githubUser, githubRepo, &github.CommitsListOptions{ - Author: email, - ListOptions: github.ListOptions{ - PerPage: 1, - }, - }) - if err != nil { - return "", err - } - if len(commits) == 0 { - return "", errors.Errorf("couldn't find GitHub commits for user email %s", email) - } - - assignee := *commits[0].Author.Login - - if newAssignee, ok := oldFriendsMap[assignee]; ok { - assignee = newAssignee - } - return assignee, nil -} - -func getProbableMilestone( - ctx context.Context, - getLatestTag func() (string, error), - listMilestones func(ctx context.Context, owner string, repo string, opt *github.MilestoneListOptions) ([]*github.Milestone, *github.Response, error), -) *int { - tag, err := getLatestTag() - if err != nil { - log.Printf("unable to get latest tag: %s", err) - log.Printf("issues will be posted without milestone") - return nil - } - - v, err := version.NewVersion(tag) - if err != nil { - log.Printf("unable to parse version from tag: %s", err) - log.Printf("issues will be posted without milestone") - return nil - } - if len(v.Segments()) < 2 { - log.Printf("version %s has less than two components; issues will be posted without milestone", tag) - return nil - } - vstring := fmt.Sprintf("%d.%d", v.Segments()[0], v.Segments()[1]) - - milestones, _, err := listMilestones(ctx, githubUser, githubRepo, &github.MilestoneListOptions{ - State: "open", - }) - if err != nil { - log.Printf("unable to list milestones: %s", err) - log.Printf("issues will be posted without milestone") - return nil - } - - for _, m := range milestones { - if m.GetTitle() == vstring { - return m.Number - } - } - return nil +func postIssue(ctx context.Context, packageName, testName, testMessage string) error { + const detail = " under stress" + return issues.Post(ctx, detail, packageName, testName, testMessage) } -func runGH( +func listFailures( ctx context.Context, input io.Reader, - createIssue func(ctx context.Context, owner string, repo string, issue *github.IssueRequest) (*github.Issue, *github.Response, error), - searchIssues func(ctx context.Context, query string, opt *github.SearchOptions) (*github.IssuesSearchResult, *github.Response, error), - createComment func(ctx context.Context, owner string, repo string, number int, comment *github.IssueComment) (*github.IssueComment, *github.Response, error), - listCommits func(ctx context.Context, owner string, repo string, opts *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error), - listMilestones func(ctx context.Context, owner string, repo string, opt *github.MilestoneListOptions) ([]*github.Milestone, *github.Response, error), - getLatestTag func() (string, error), + f func(ctx context.Context, packageName, testName, testMessage string) error, ) error { - sha, ok := os.LookupEnv(teamcityVCSNumberEnv) - if !ok { - return errors.Errorf("VCS number environment variable %s is not set", teamcityVCSNumberEnv) - } - - buildID, ok := os.LookupEnv(teamcityBuildIDEnv) - if !ok { - log.Fatalf("teamcity build ID environment variable %s is not set", teamcityBuildIDEnv) - } - - serverURL, ok := os.LookupEnv(teamcityServerURLEnv) - if !ok { - log.Fatalf("teamcity server URL environment variable %s is not set", teamcityServerURLEnv) - } - - options := url.Values{} - options.Add("buildId", buildID) - options.Add("tab", "buildLog") - - u, err := url.Parse(serverURL) - if err != nil { - log.Fatal(err) - } - u.Scheme = "https" - u.Path = "viewLog.html" - u.RawQuery = options.Encode() - var inputBuf bytes.Buffer input = io.TeeReader(input, &inputBuf) - var parameters []string - for _, parameter := range []string{ - tagsEnv, - goFlagsEnv, - } { - if val, ok := os.LookupEnv(parameter); ok { - parameters = append(parameters, parameter+"="+val) - } - } - parametersStr := "```\n" + strings.Join(parameters, "\n") + "\n```" - const bodyTemplate = `SHA: https://github.com/cockroachdb/cockroach/commits/%s - -Parameters: -%s - -Stress build found a failed test: %s` - - milestone := getProbableMilestone(ctx, getLatestTag, listMilestones) - - newIssueRequest := func(packageName, testName, message, assignee string) *github.IssueRequest { - title := fmt.Sprintf("%s: %s failed under stress", - strings.TrimPrefix(packageName, cockroachPkgPrefix), testName) - body := fmt.Sprintf(bodyTemplate, sha, parametersStr, u.String()) + "\n\n```\n%s\n```" - // We insert a raw "%s" above so we can figure out the length of the - // body so far, without the actual error text. We need this length so we - // can calculate the maximum amount of error text we can include in the - // issue without exceeding GitHub's limit. We replace that %s in the - // following Sprintf. - body = fmt.Sprintf(body, trimIssueRequestBody(message, len(body))) - - return &github.IssueRequest{ - Title: &title, - Body: &body, - Labels: &issueLabels, - Assignee: &assignee, - Milestone: milestone, - } - } - - newIssueComment := func(packageName, testName string) *github.IssueComment { - body := fmt.Sprintf(bodyTemplate, sha, parametersStr, u.String()) - return &github.IssueComment{Body: &body} - } - suites, err := lib.ParseGotest(input, "") if err != nil { return errors.Wrap(err, "failed to parse `go test` output") } + posted := false for _, suite := range suites { packageName := suite.Name @@ -337,60 +68,26 @@ Stress build found a failed test: %s` for _, test := range suite.Tests { switch test.Status { case lib.Failed: - assignee, err := getAssignee(ctx, packageName, test.Name, listCommits) - if err != nil { - // if we *can't* assign anyone, sigh, feel free to hard-code me. - // -- tschottdorf, 11/3/2017 - assignee = "tschottdorf" - test.Message += fmt.Sprintf("\n\nFailed to find issue assignee: \n%s", err) - } - issueRequest := newIssueRequest(packageName, test.Name, test.Message, assignee) - searchQuery := fmt.Sprintf(`"%s" user:%s repo:%s is:open`, *issueRequest.Title, githubUser, githubRepo) - for _, label := range issueLabels { - searchQuery = searchQuery + fmt.Sprintf(` label:"%s"`, label) - } - - var foundIssue *int - - result, _, err := searchIssues(ctx, searchQuery, &github.SearchOptions{ - ListOptions: github.ListOptions{ - PerPage: 1, - }, - }) - if err != nil { - return errors.Wrapf(err, "failed to search GitHub with query %s", github.Stringify(searchQuery)) - } - if *result.Total > 0 { - foundIssue = result.Issues[0].Number - } - - if foundIssue == nil { - if _, _, err := createIssue(ctx, githubUser, githubRepo, issueRequest); err != nil { - return errors.Wrapf(err, "failed to create GitHub issue %s", github.Stringify(issueRequest)) - } - } else { - comment := newIssueComment(packageName, test.Name) - if _, _, err := createComment(ctx, githubUser, githubRepo, *foundIssue, comment); err != nil { - return errors.Wrapf(err, "failed to update issue #%d with %s", *foundIssue, github.Stringify(comment)) - } + if err := f(ctx, packageName, test.Name, test.Message); err != nil { + return errors.Wrap(err, "failed to post issue") } posted = true } } } - const unknown = "(unknown)" - if !posted { + // We're only invoked upon failure. If we couldn't find a failing Go test, + // assume that a failure occurred before running Go and post an issue about + // that. + const unknown = "(unknown)" packageName, ok := os.LookupEnv(pkgEnv) if !ok { packageName = unknown } - issueRequest := newIssueRequest(packageName, unknown, inputBuf.String(), "") - if _, _, err := createIssue(ctx, githubUser, githubRepo, issueRequest); err != nil { - return errors.Wrapf(err, "failed to create GitHub issue %s", github.Stringify(issueRequest)) + if err := f(ctx, packageName, unknown, inputBuf.String()); err != nil { + return errors.Wrap(err, "failed to post issue") } } - return nil } diff --git a/pkg/cmd/github-post/main_test.go b/pkg/cmd/github-post/main_test.go index e629b308d66e..84a03d310bb1 100644 --- a/pkg/cmd/github-post/main_test.go +++ b/pkg/cmd/github-post/main_test.go @@ -16,41 +16,15 @@ package main import ( "context" - "fmt" "os" "path/filepath" "regexp" - "strconv" - "strings" "testing" - - "github.com/google/go-github/github" ) -func TestRunGH(t *testing.T) { - const ( - expOwner = "cockroachdb" - expRepo = "cockroach" - expAssignee = "hodor" - expMilestone = 2 - envPkg = "storage" - envTags = "deadlock" - envGoFlags = "race" - sha = "abcd123" - serverURL = "https://teamcity.example.com" - buildID = 8008135 - issueID = 1337 - issueNumber = 30 - ) - +func TestListFailures(t *testing.T) { for key, value := range map[string]string{ - teamcityVCSNumberEnv: sha, - teamcityServerURLEnv: serverURL, - teamcityBuildIDEnv: strconv.Itoa(buildID), - - pkgEnv: cockroachPkgPrefix + envPkg, - tagsEnv: envTags, - goFlagsEnv: envGoFlags, + pkgEnv: "github.com/cockroachdb/cockroach/pkg/storage", } { if val, ok := os.LookupEnv(key); ok { defer func() { @@ -65,187 +39,61 @@ func TestRunGH(t *testing.T) { } }() } - if err := os.Setenv(key, value); err != nil { t.Fatal(err) } } - parameters := "```\n" + strings.Join([]string{ - tagsEnv + "=" + envTags, - goFlagsEnv + "=" + envGoFlags, - }, "\n") + "\n```" - - for fileName, expectations := range map[string]struct { + testCases := []struct { + fileName string packageName string testName string - body string + message string }{ - "stress-failure": { - packageName: envPkg, + { + fileName: "stress-failure", + packageName: "github.com/cockroachdb/cockroach/pkg/storage", testName: "TestReplicateQueueRebalance", - body: " :12: storage/replicate_queue_test.go:103, condition failed to evaluate within 45s: not balanced: [10 1 10 1 8]", + message: " :12: storage/replicate_queue_test.go:103, condition failed to evaluate within 45s: not balanced: [10 1 10 1 8]", }, - "stress-fatal": { - packageName: envPkg, + { + fileName: "stress-fatal", + packageName: "github.com/cockroachdb/cockroach/pkg/storage", testName: "TestGossipHandlesReplacedNode", - body: "F170517 07:33:43.763059 69575 storage/replica.go:1360 [n3,s3,r1/3:/M{in-ax}] on-disk and in-memory state diverged:", + message: "F170517 07:33:43.763059 69575 storage/replica.go:1360 [n3,s3,r1/3:/M{in-ax}] on-disk and in-memory state diverged:", }, - } { - for _, foundIssue := range []bool{true, false} { - testName := fileName - if foundIssue { - testName = testName + "-existing-issue" + { + fileName: "stress-unknown", + packageName: "github.com/cockroachdb/cockroach/pkg/storage", + testName: "(unknown)", + message: "make: *** [bin/.submodules-initialized] Error 1", + }, + } + for _, c := range testCases { + t.Run(c.fileName, func(t *testing.T) { + file, err := os.Open(filepath.Join("testdata", c.fileName)) + if err != nil { + t.Fatal(err) } - t.Run(testName, func(t *testing.T) { - file, err := os.Open(filepath.Join("testdata", fileName)) - if err != nil { - t.Fatal(err) - } - - reString := fmt.Sprintf(`(?s)\ASHA: https://github.com/cockroachdb/cockroach/commits/%s - -Parameters: -%s - -Stress build found a failed test: %s`, - regexp.QuoteMeta(sha), - regexp.QuoteMeta(parameters), - regexp.QuoteMeta(fmt.Sprintf("%s/viewLog.html?buildId=%d&tab=buildLog", serverURL, buildID)), - ) + defer file.Close() - issueBodyRe, err := regexp.Compile( - fmt.Sprintf(reString+` - -.* -%s -`, regexp.QuoteMeta(expectations.body)), - ) - if err != nil { - t.Fatal(err) - } - commentBodyRe, err := regexp.Compile(reString) - if err != nil { - t.Fatal(err) + f := func(_ context.Context, packageName, testName, testMessage string) error { + if c.packageName != packageName { + t.Fatalf("expected %s, but got %s", c.packageName, packageName) } - - issueCount := 0 - commentCount := 0 - postIssue := func(_ context.Context, owner string, repo string, issue *github.IssueRequest) (*github.Issue, *github.Response, error) { - issueCount++ - if owner != expOwner { - t.Fatalf("got %s, expected %s", owner, expOwner) - } - if repo != expRepo { - t.Fatalf("got %s, expected %s", repo, expRepo) - } - if *issue.Assignee != expAssignee { - t.Fatalf("got %s, expected %s", *issue.Assignee, expAssignee) - } - if expected := fmt.Sprintf("%s: %s failed under stress", expectations.packageName, expectations.testName); *issue.Title != expected { - t.Fatalf("got %s, expected %s", *issue.Title, expected) - } - if !issueBodyRe.MatchString(*issue.Body) { - t.Fatalf("got:\n%s\nexpected:\n%s", *issue.Body, issueBodyRe) - } - if length := len(*issue.Body); length > githubIssueBodyMaximumLength { - t.Fatalf("issue length %d exceeds (undocumented) maximum %d", length, githubIssueBodyMaximumLength) - } - if *issue.Milestone != expMilestone { - t.Fatalf("expected milestone %d, but got %d", expMilestone, *issue.Milestone) - } - return &github.Issue{ID: github.Int64(issueID)}, nil, nil + if c.testName != testName { + t.Fatalf("expected %s, but got %s", c.testName, testName) } - searchIssues := func(_ context.Context, query string, opt *github.SearchOptions) (*github.IssuesSearchResult, *github.Response, error) { - total := 0 - if foundIssue { - total = 1 - } - return &github.IssuesSearchResult{ - Total: &total, - Issues: []github.Issue{ - {Number: github.Int(issueNumber)}, - }, - }, nil, nil + messageRE := regexp.MustCompile(regexp.QuoteMeta(c.message)) + if !messageRE.MatchString(testMessage) { + t.Fatalf("expected %s, but got %s", messageRE, testMessage) } - postComment := func(_ context.Context, owner string, repo string, number int, comment *github.IssueComment) (*github.IssueComment, *github.Response, error) { - if owner != expOwner { - t.Fatalf("got %s, expected %s", owner, expOwner) - } - if repo != expRepo { - t.Fatalf("got %s, expected %s", repo, expRepo) - } - if !commentBodyRe.MatchString(*comment.Body) { - t.Fatalf("got:\n%s\nexpected:\n%s", *comment.Body, issueBodyRe) - } - if length := len(*comment.Body); length > githubIssueBodyMaximumLength { - t.Fatalf("comment length %d exceeds (undocumented) maximum %d", length, githubIssueBodyMaximumLength) - } - commentCount++ - - return nil, nil, nil - } - - listCommits := func(_ context.Context, owner string, repo string, opts *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error) { - if owner != expOwner { - t.Fatalf("got %s, expected %s", owner, expOwner) - } - if repo != expRepo { - t.Fatalf("got %s, expected %s", repo, expRepo) - } - if opts.Author == "" { - t.Fatalf("found no author, but expected one") - } - assignee := expAssignee - return []*github.RepositoryCommit{ - { - Author: &github.User{ - Login: &assignee, - }, - }, - }, nil, nil - } - - listMilestones := func(_ context.Context, owner, repo string, _ *github.MilestoneListOptions) ([]*github.Milestone, *github.Response, error) { - if owner != expOwner { - t.Fatalf("got %s, expected %s", owner, expOwner) - } - if repo != expRepo { - t.Fatalf("got %s, expected %s", repo, expRepo) - } - return []*github.Milestone{ - {Title: github.String("3.3"), Number: github.Int(expMilestone)}, - {Title: github.String("3.2"), Number: github.Int(1)}, - }, nil, nil - } - - getLatestTag := func() (string, error) { return "3.3", nil } + return nil + } - if err := runGH( - context.Background(), - file, - postIssue, - searchIssues, - postComment, - listCommits, - listMilestones, - getLatestTag, - ); err != nil { - t.Fatal(err) - } - expectedIssues := 1 - expectedComments := 0 - if foundIssue { - expectedIssues = 0 - expectedComments = 1 - } - if issueCount != expectedIssues { - t.Fatalf("%d issues were posted, expected %d", issueCount, expectedIssues) - } - if commentCount != expectedComments { - t.Fatalf("%d comments were posted, expected %d", commentCount, expectedComments) - } - }) - } + if err := listFailures(context.Background(), file, f); err != nil { + t.Fatal(err) + } + }) } } diff --git a/pkg/cmd/github-post/testdata/stress-unknown b/pkg/cmd/github-post/testdata/stress-unknown new file mode 100644 index 000000000000..ff50cef1429d --- /dev/null +++ b/pkg/cmd/github-post/testdata/stress-unknown @@ -0,0 +1,23 @@ +GOPATH set to /go +Running make with -j8 +git submodule update --init +mkdir -p ./pkg/sql/parser/gen +awk -f ./pkg/sql/parser/help.awk < pkg/sql/parser/sql.y > pkg/sql/parser/help_messages.go.tmp || rm pkg/sql/parser/help_messages.go.tmp +awk -f ./pkg/sql/parser/all_keywords.awk < pkg/sql/parser/sql.y > pkg/sql/lex/keywords.go.tmp || rm pkg/sql/lex/keywords.go.tmp +set -euo pipefail; \ +TYPES=$(awk '/func.*sqlSymUnion/ {print $(NF - 1)}' ./pkg/sql/parser/sql.y | sed -e 's/[]\/$*.^|[]/\\&/g' | tr '\n' '|' | sed -E '$s/.$//'); \ +sed -E "s_(type|token) <($TYPES)>_\1 /* <\2> */_" < ./pkg/sql/parser/sql.y | \ +awk -f ./pkg/sql/parser/replace_help_rules.awk > pkg/sql/parser/gen/sql.y +awk -f ./pkg/sql/parser/reserved_keywords.awk < pkg/sql/parser/sql.y > pkg/sql/lex/reserved_keywords.go.tmp || rm pkg/sql/lex/reserved_keywords.go.tmp +mv -f pkg/sql/parser/help_messages.go.tmp pkg/sql/parser/help_messages.go +mv -f pkg/sql/lex/keywords.go.tmp pkg/sql/lex/keywords.go +mv -f pkg/sql/lex/reserved_keywords.go.tmp pkg/sql/lex/reserved_keywords.go +gofmt -s -w pkg/sql/parser/help_messages.go +gofmt -s -w pkg/sql/lex/keywords.go +gofmt -s -w pkg/sql/lex/reserved_keywords.go +mv -f pkg/sql/parser/helpmap_test.go.tmp pkg/sql/parser/helpmap_test.go +gofmt -s -w pkg/sql/parser/helpmap_test.go +fatal: Not a git repository: /home/agent/work/.go/src/github.com/cockroachdb/cockroach/.git/modules/c-deps/snappy +Unable to find current revision in submodule path 'c-deps/snappy' +Makefile:368: recipe for target 'bin/.submodules-initialized' failed +make: *** [bin/.submodules-initialized] Error 1 diff --git a/pkg/cmd/internal/issues/issues.go b/pkg/cmd/internal/issues/issues.go index 8513de55eaf9..1d214dbc9925 100644 --- a/pkg/cmd/internal/issues/issues.go +++ b/pkg/cmd/internal/issues/issues.go @@ -12,84 +12,58 @@ // implied. See the License for the specific language governing // permissions and limitations under the License. -package main +package issues import ( - "bytes" "context" "fmt" - "io" "log" "net/url" "os" "os/exec" "regexp" "strings" + "sync" "golang.org/x/oauth2" "github.com/google/go-github/github" version "github.com/hashicorp/go-version" "github.com/pkg/errors" - "github.com/tebeka/go2xunit/lib" ) -const githubAPITokenEnv = "GITHUB_API_TOKEN" -const teamcityVCSNumberEnv = "BUILD_VCS_NUMBER" -const teamcityBuildIDEnv = "TC_BUILD_ID" -const teamcityServerURLEnv = "TC_SERVER_URL" -const githubUser = "cockroachdb" -const githubRepo = "cockroach" - -const pkgEnv = "PKG" -const tagsEnv = "TAGS" -const goFlagsEnv = "GOFLAGS" -const cockroachPkgPrefix = "github.com/cockroachdb/cockroach/pkg/" +const ( + githubAPITokenEnv = "GITHUB_API_TOKEN" + teamcityVCSNumberEnv = "BUILD_VCS_NUMBER" + teamcityBuildIDEnv = "TC_BUILD_ID" + teamcityServerURLEnv = "TC_SERVER_URL" + tagsEnv = "TAGS" + goFlagsEnv = "GOFLAGS" + githubUser = "cockroachdb" + githubRepo = "cockroach" + cockroachPkgPrefix = "github.com/cockroachdb/cockroach/pkg/" +) -var issueLabels = []string{"O-robot", "C-test-failure"} +var ( + issueLabels = []string{"O-robot", "C-test-failure"} + stacktraceRE = regexp.MustCompile(`(?m:^goroutine\s\d+)`) +) // Based on the following observed API response: // -// 422 Validation Failed [{Resource:Issue Field:body Code:custom Message:body is too long (maximum is 65536 characters)}] +// 422 Validation Failed [{Resource:Issue Field:body Code:custom Message:body +// is too long (maximum is 65536 characters)}] const githubIssueBodyMaximumLength = 1<<16 - 1 -func main() { - token, ok := os.LookupEnv(githubAPITokenEnv) - if !ok { - log.Fatalf("GitHub API token environment variable %s is not set", githubAPITokenEnv) - } - - ctx := context.Background() - - client := github.NewClient(oauth2.NewClient(ctx, oauth2.StaticTokenSource( - &oauth2.Token{AccessToken: token}, - ))) - - getLatestTag := func() (string, error) { - cmd := exec.Command("git", "describe", "--abbrev=0", "--tags") - out, err := cmd.CombinedOutput() - if err != nil { - return "", err - } - return strings.TrimSpace(string(out)), nil - } - - if err := runGH( - ctx, - os.Stdin, - client.Issues.Create, - client.Search.Issues, - client.Issues.CreateComment, - client.Repositories.ListCommits, - client.Issues.ListMilestones, - getLatestTag, - ); err != nil { - log.Fatal(err) - } -} - -var stacktraceRE = regexp.MustCompile(`(?m:^goroutine\s\d+)`) - +// trimIssueRequestBody trims message such that the total size of an issue body +// is less than githubIssueBodyMaximumLength. usedCharacters specifies the +// number of characters that have already been used for the issue body (see +// newIssueRequest below). message is usually the test failure message and +// possibly includes stacktraces for all of the goroutines (which is what makes +// the message very large). +// +// TODO(peter): Rather than trimming the message like this, perhaps it can be +// added as an attachment or some other expandable comment. func trimIssueRequestBody(message string, usedCharacters int) string { maxLength := githubIssueBodyMaximumLength - usedCharacters @@ -134,7 +108,8 @@ var oldFriendsMap = map[string]string{ func getAssignee( ctx context.Context, packageName, testName string, - listCommits func(ctx context.Context, owner string, repo string, opts *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error), + listCommits func(ctx context.Context, owner string, repo string, + opts *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error), ) (string, error) { // Search the source code for the email address of the last committer to touch // the first line of the source code that contains testName. Then, ask GitHub @@ -143,34 +118,42 @@ func getAssignee( subtests := strings.Split(testName, "/") testName = subtests[0] packageName = strings.TrimPrefix(packageName, "github.com/cockroachdb/cockroach/") - cmd := exec.Command(`/bin/bash`, `-c`, fmt.Sprintf(`git grep -n "func %s" $(git rev-parse --show-toplevel)/%s/*_test.go`, testName, packageName)) + cmd := exec.Command(`/bin/bash`, `-c`, + fmt.Sprintf(`git grep -n "func %s" $(git rev-parse --show-toplevel)/%s/*_test.go`, + testName, packageName)) // This command returns output such as: // ../ccl/storageccl/export_test.go:31:func TestExportCmd(t *testing.T) { out, err := cmd.CombinedOutput() if err != nil { - return "", errors.Errorf("couldn't find test %s in %s: %s %s", testName, packageName, err, string(out)) + return "", errors.Errorf("couldn't find test %s in %s: %s %s", + testName, packageName, err, string(out)) } re := regexp.MustCompile(`(.*):(.*):`) // The first 2 :-delimited fields are the filename and line number. matches := re.FindSubmatch(out) if matches == nil { - return "", errors.Errorf("couldn't find filename/line number for test %s in %s: %s", testName, packageName, string(out)) + return "", errors.Errorf("couldn't find filename/line number for test %s in %s: %s", + testName, packageName, string(out)) } filename := matches[1] linenum := matches[2] // Now run git blame. - cmd = exec.Command(`/bin/bash`, `-c`, fmt.Sprintf(`git blame --porcelain -L%s,+1 %s | grep author-mail`, linenum, filename)) + cmd = exec.Command(`/bin/bash`, `-c`, + fmt.Sprintf(`git blame --porcelain -L%s,+1 %s | grep author-mail`, + linenum, filename)) // This command returns output such as: // author-mail out, err = cmd.CombinedOutput() if err != nil { - return "", errors.Errorf("couldn't find author of test %s in %s: %s %s", testName, packageName, err, string(out)) + return "", errors.Errorf("couldn't find author of test %s in %s: %s %s", + testName, packageName, err, string(out)) } re = regexp.MustCompile("author-mail <(.*)>") matches = re.FindSubmatch(out) if matches == nil { - return "", errors.Errorf("couldn't find author email of test %s in %s: %s", testName, packageName, string(out)) + return "", errors.Errorf("couldn't find author email of test %s in %s: %s", + testName, packageName, string(out)) } email := string(matches[1]) @@ -195,10 +178,20 @@ func getAssignee( return assignee, nil } +func getLatestTag() (string, error) { + cmd := exec.Command("git", "describe", "--abbrev=0", "--tags") + out, err := cmd.CombinedOutput() + if err != nil { + return "", err + } + return strings.TrimSpace(string(out)), nil +} + func getProbableMilestone( ctx context.Context, getLatestTag func() (string, error), - listMilestones func(ctx context.Context, owner string, repo string, opt *github.MilestoneListOptions) ([]*github.Milestone, *github.Response, error), + listMilestones func(ctx context.Context, owner string, repo string, + opt *github.MilestoneListOptions) ([]*github.Milestone, *github.Response, error), ) *int { tag, err := getLatestTag() if err != nil { @@ -236,69 +229,77 @@ func getProbableMilestone( return nil } -func runGH( - ctx context.Context, - input io.Reader, - createIssue func(ctx context.Context, owner string, repo string, issue *github.IssueRequest) (*github.Issue, *github.Response, error), - searchIssues func(ctx context.Context, query string, opt *github.SearchOptions) (*github.IssuesSearchResult, *github.Response, error), - createComment func(ctx context.Context, owner string, repo string, number int, comment *github.IssueComment) (*github.IssueComment, *github.Response, error), - listCommits func(ctx context.Context, owner string, repo string, opts *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error), - listMilestones func(ctx context.Context, owner string, repo string, opt *github.MilestoneListOptions) ([]*github.Milestone, *github.Response, error), - getLatestTag func() (string, error), -) error { - sha, ok := os.LookupEnv(teamcityVCSNumberEnv) - if !ok { - return errors.Errorf("VCS number environment variable %s is not set", teamcityVCSNumberEnv) - } +type poster struct { + sha string + buildID string + serverURL string + milestone *int + + createIssue func(ctx context.Context, owner string, repo string, + issue *github.IssueRequest) (*github.Issue, *github.Response, error) + searchIssues func(ctx context.Context, query string, + opt *github.SearchOptions) (*github.IssuesSearchResult, *github.Response, error) + createComment func(ctx context.Context, owner string, repo string, number int, + comment *github.IssueComment) (*github.IssueComment, *github.Response, error) + listCommits func(ctx context.Context, owner string, repo string, + opts *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error) + listMilestones func(ctx context.Context, owner string, repo string, + opt *github.MilestoneListOptions) ([]*github.Milestone, *github.Response, error) + getLatestTag func() (string, error) +} - buildID, ok := os.LookupEnv(teamcityBuildIDEnv) +func newPoster() *poster { + token, ok := os.LookupEnv(githubAPITokenEnv) if !ok { - log.Fatalf("teamcity build ID environment variable %s is not set", teamcityBuildIDEnv) + log.Fatalf("GitHub API token environment variable %s is not set", githubAPITokenEnv) } - serverURL, ok := os.LookupEnv(teamcityServerURLEnv) - if !ok { - log.Fatalf("teamcity server URL environment variable %s is not set", teamcityServerURLEnv) - } + ctx := context.Background() + client := github.NewClient(oauth2.NewClient(ctx, oauth2.StaticTokenSource( + &oauth2.Token{AccessToken: token}, + ))) - options := url.Values{} - options.Add("buildId", buildID) - options.Add("tab", "buildLog") + return &poster{ + createIssue: client.Issues.Create, + searchIssues: client.Search.Issues, + createComment: client.Issues.CreateComment, + listCommits: client.Repositories.ListCommits, + listMilestones: client.Issues.ListMilestones, + getLatestTag: getLatestTag, + } +} - u, err := url.Parse(serverURL) - if err != nil { - log.Fatal(err) +func (p *poster) init() { + var ok bool + if p.sha, ok = os.LookupEnv(teamcityVCSNumberEnv); !ok { + log.Fatalf("VCS number environment variable %s is not set", teamcityVCSNumberEnv) } - u.Scheme = "https" - u.Path = "viewLog.html" - u.RawQuery = options.Encode() + if p.buildID, ok = os.LookupEnv(teamcityBuildIDEnv); !ok { + log.Fatalf("teamcity build ID environment variable %s is not set", teamcityBuildIDEnv) + } + if p.serverURL, ok = os.LookupEnv(teamcityServerURLEnv); !ok { + log.Fatalf("teamcity server URL environment variable %s is not set", teamcityServerURLEnv) + } + p.milestone = getProbableMilestone(context.Background(), p.getLatestTag, p.listMilestones) +} - var inputBuf bytes.Buffer - input = io.TeeReader(input, &inputBuf) +func (p *poster) post(ctx context.Context, detail, packageName, testName, message string) error { + packageName = cockroachPkgPrefix + packageName - var parameters []string - for _, parameter := range []string{ - tagsEnv, - goFlagsEnv, - } { - if val, ok := os.LookupEnv(parameter); ok { - parameters = append(parameters, parameter+"="+val) - } - } - parametersStr := "```\n" + strings.Join(parameters, "\n") + "\n```" - const bodyTemplate = `SHA: https://github.com/cockroachdb/cockroach/commits/%s + const bodyTemplate = `SHA: https://github.com/cockroachdb/cockroach/commits/%[1]s Parameters: -%s +` + "```" + ` +%[2]s +` + "```" + ` -Stress build found a failed test: %s` - - milestone := getProbableMilestone(ctx, getLatestTag, listMilestones) +Failed test: %[3]s` + const messageTemplate = "\n\n```\n%s\n```" newIssueRequest := func(packageName, testName, message, assignee string) *github.IssueRequest { - title := fmt.Sprintf("%s: %s failed under stress", - strings.TrimPrefix(packageName, cockroachPkgPrefix), testName) - body := fmt.Sprintf(bodyTemplate, sha, parametersStr, u.String()) + "\n\n```\n%s\n```" + title := fmt.Sprintf("%s: %s failed%s", + strings.TrimPrefix(packageName, cockroachPkgPrefix), testName, detail) + body := fmt.Sprintf(bodyTemplate, p.sha, p.parameters(), p.teamcityURL()) + messageTemplate // We insert a raw "%s" above so we can figure out the length of the // body so far, without the actual error text. We need this length so we // can calculate the maximum amount of error text we can include in the @@ -311,86 +312,100 @@ Stress build found a failed test: %s` Body: &body, Labels: &issueLabels, Assignee: &assignee, - Milestone: milestone, + Milestone: p.milestone, } } newIssueComment := func(packageName, testName string) *github.IssueComment { - body := fmt.Sprintf(bodyTemplate, sha, parametersStr, u.String()) + body := fmt.Sprintf(bodyTemplate, p.sha, p.parameters(), p.teamcityURL()) return &github.IssueComment{Body: &body} } - suites, err := lib.ParseGotest(input, "") + assignee, err := getAssignee(ctx, packageName, testName, p.listCommits) if err != nil { - return errors.Wrap(err, "failed to parse `go test` output") + // if we *can't* assign anyone, sigh, feel free to hard-code me. + // -- tschottdorf, 11/3/2017 + assignee = "tschottdorf" + message += fmt.Sprintf("\n\nFailed to find issue assignee: \n%s", err) } - posted := false - for _, suite := range suites { - packageName := suite.Name - if packageName == "" { - var ok bool - packageName, ok = os.LookupEnv(pkgEnv) - if !ok { - log.Fatalf("package name environment variable %s is not set", pkgEnv) - } - } - for _, test := range suite.Tests { - switch test.Status { - case lib.Failed: - assignee, err := getAssignee(ctx, packageName, test.Name, listCommits) - if err != nil { - // if we *can't* assign anyone, sigh, feel free to hard-code me. - // -- tschottdorf, 11/3/2017 - assignee = "tschottdorf" - test.Message += fmt.Sprintf("\n\nFailed to find issue assignee: \n%s", err) - } - issueRequest := newIssueRequest(packageName, test.Name, test.Message, assignee) - searchQuery := fmt.Sprintf(`"%s" user:%s repo:%s is:open`, *issueRequest.Title, githubUser, githubRepo) - for _, label := range issueLabels { - searchQuery = searchQuery + fmt.Sprintf(` label:"%s"`, label) - } - var foundIssue *int + issueRequest := newIssueRequest(packageName, testName, message, assignee) + searchQuery := fmt.Sprintf(`"%s" user:%s repo:%s is:open`, + *issueRequest.Title, githubUser, githubRepo) + for _, label := range issueLabels { + searchQuery = searchQuery + fmt.Sprintf(` label:"%s"`, label) + } - result, _, err := searchIssues(ctx, searchQuery, &github.SearchOptions{ - ListOptions: github.ListOptions{ - PerPage: 1, - }, - }) - if err != nil { - return errors.Wrapf(err, "failed to search GitHub with query %s", github.Stringify(searchQuery)) - } - if *result.Total > 0 { - foundIssue = result.Issues[0].Number - } + var foundIssue *int + result, _, err := p.searchIssues(ctx, searchQuery, &github.SearchOptions{ + ListOptions: github.ListOptions{ + PerPage: 1, + }, + }) + if err != nil { + return errors.Wrapf(err, "failed to search GitHub with query %s", + github.Stringify(searchQuery)) + } + if *result.Total > 0 { + foundIssue = result.Issues[0].Number + } - if foundIssue == nil { - if _, _, err := createIssue(ctx, githubUser, githubRepo, issueRequest); err != nil { - return errors.Wrapf(err, "failed to create GitHub issue %s", github.Stringify(issueRequest)) - } - } else { - comment := newIssueComment(packageName, test.Name) - if _, _, err := createComment(ctx, githubUser, githubRepo, *foundIssue, comment); err != nil { - return errors.Wrapf(err, "failed to update issue #%d with %s", *foundIssue, github.Stringify(comment)) - } - } - posted = true - } + if foundIssue == nil { + if _, _, err := p.createIssue(ctx, githubUser, githubRepo, issueRequest); err != nil { + return errors.Wrapf(err, "failed to create GitHub issue %s", + github.Stringify(issueRequest)) + } + } else { + comment := newIssueComment(packageName, testName) + if _, _, err := p.createComment( + ctx, githubUser, githubRepo, *foundIssue, comment); err != nil { + return errors.Wrapf(err, "failed to update issue #%d with %s", + *foundIssue, github.Stringify(comment)) } } - const unknown = "(unknown)" + return nil +} - if !posted { - packageName, ok := os.LookupEnv(pkgEnv) - if !ok { - packageName = unknown - } - issueRequest := newIssueRequest(packageName, unknown, inputBuf.String(), "") - if _, _, err := createIssue(ctx, githubUser, githubRepo, issueRequest); err != nil { - return errors.Wrapf(err, "failed to create GitHub issue %s", github.Stringify(issueRequest)) +func (p *poster) teamcityURL() *url.URL { + options := url.Values{} + options.Add("buildId", p.buildID) + options.Add("tab", "buildLog") + + u, err := url.Parse(p.serverURL) + if err != nil { + log.Fatal(err) + } + u.Scheme = "https" + u.Path = "viewLog.html" + u.RawQuery = options.Encode() + return u +} + +func (p *poster) parameters() string { + var parameters []string + for _, parameter := range []string{ + tagsEnv, + goFlagsEnv, + } { + if val, ok := os.LookupEnv(parameter); ok { + parameters = append(parameters, parameter+"="+val) } } + return strings.Join(parameters, "\n") +} - return nil +var defaultP struct { + sync.Once + *poster +} + +// Post either creates a new issue for a failed test, or posts a comment to an +// existing open issue. +func Post(ctx context.Context, detail, packageName, testName, message string) error { + defaultP.Do(func() { + defaultP.poster = newPoster() + defaultP.init() + }) + return defaultP.post(ctx, detail, packageName, testName, message) } diff --git a/pkg/cmd/internal/issues/issues_test.go b/pkg/cmd/internal/issues/issues_test.go index e629b308d66e..d5b17a9f38ad 100644 --- a/pkg/cmd/internal/issues/issues_test.go +++ b/pkg/cmd/internal/issues/issues_test.go @@ -12,13 +12,12 @@ // implied. See the License for the specific language governing // permissions and limitations under the License. -package main +package issues import ( "context" "fmt" "os" - "path/filepath" "regexp" "strconv" "strings" @@ -27,7 +26,7 @@ import ( "github.com/google/go-github/github" ) -func TestRunGH(t *testing.T) { +func TestPost(t *testing.T) { const ( expOwner = "cockroachdb" expRepo = "cockroach" @@ -47,10 +46,8 @@ func TestRunGH(t *testing.T) { teamcityVCSNumberEnv: sha, teamcityServerURLEnv: serverURL, teamcityBuildIDEnv: strconv.Itoa(buildID), - - pkgEnv: cockroachPkgPrefix + envPkg, - tagsEnv: envTags, - goFlagsEnv: envGoFlags, + tagsEnv: envTags, + goFlagsEnv: envGoFlags, } { if val, ok := os.LookupEnv(key); ok { defer func() { @@ -76,39 +73,39 @@ func TestRunGH(t *testing.T) { goFlagsEnv + "=" + envGoFlags, }, "\n") + "\n```" - for fileName, expectations := range map[string]struct { + testCases := []struct { + name string packageName string testName string - body string + message string }{ - "stress-failure": { + { + name: "failure", packageName: envPkg, testName: "TestReplicateQueueRebalance", - body: " :12: storage/replicate_queue_test.go:103, condition failed to evaluate within 45s: not balanced: [10 1 10 1 8]", + message: " :12: storage/replicate_queue_test.go:103, condition failed to evaluate within 45s: not balanced: [10 1 10 1 8]", }, - "stress-fatal": { + { + name: "fatal", packageName: envPkg, testName: "TestGossipHandlesReplacedNode", - body: "F170517 07:33:43.763059 69575 storage/replica.go:1360 [n3,s3,r1/3:/M{in-ax}] on-disk and in-memory state diverged:", + message: "F170517 07:33:43.763059 69575 storage/replica.go:1360 [n3,s3,r1/3:/M{in-ax}] on-disk and in-memory state diverged:", }, - } { + } + + for _, c := range testCases { for _, foundIssue := range []bool{true, false} { - testName := fileName + name := c.name if foundIssue { - testName = testName + "-existing-issue" + name = name + "-existing-issue" } - t.Run(testName, func(t *testing.T) { - file, err := os.Open(filepath.Join("testdata", fileName)) - if err != nil { - t.Fatal(err) - } - + t.Run(name, func(t *testing.T) { reString := fmt.Sprintf(`(?s)\ASHA: https://github.com/cockroachdb/cockroach/commits/%s Parameters: %s -Stress build found a failed test: %s`, +Failed test: %s`, regexp.QuoteMeta(sha), regexp.QuoteMeta(parameters), regexp.QuoteMeta(fmt.Sprintf("%s/viewLog.html?buildId=%d&tab=buildLog", serverURL, buildID)), @@ -119,7 +116,7 @@ Stress build found a failed test: %s`, .* %s -`, regexp.QuoteMeta(expectations.body)), +`, regexp.QuoteMeta(c.message)), ) if err != nil { t.Fatal(err) @@ -131,7 +128,11 @@ Stress build found a failed test: %s`, issueCount := 0 commentCount := 0 - postIssue := func(_ context.Context, owner string, repo string, issue *github.IssueRequest) (*github.Issue, *github.Response, error) { + + p := &poster{} + + p.createIssue = func(_ context.Context, owner string, repo string, + issue *github.IssueRequest) (*github.Issue, *github.Response, error) { issueCount++ if owner != expOwner { t.Fatalf("got %s, expected %s", owner, expOwner) @@ -142,7 +143,7 @@ Stress build found a failed test: %s`, if *issue.Assignee != expAssignee { t.Fatalf("got %s, expected %s", *issue.Assignee, expAssignee) } - if expected := fmt.Sprintf("%s: %s failed under stress", expectations.packageName, expectations.testName); *issue.Title != expected { + if expected := fmt.Sprintf("%s: %s failed under stress", envPkg, c.testName); *issue.Title != expected { t.Fatalf("got %s, expected %s", *issue.Title, expected) } if !issueBodyRe.MatchString(*issue.Body) { @@ -156,7 +157,9 @@ Stress build found a failed test: %s`, } return &github.Issue{ID: github.Int64(issueID)}, nil, nil } - searchIssues := func(_ context.Context, query string, opt *github.SearchOptions) (*github.IssuesSearchResult, *github.Response, error) { + + p.searchIssues = func(_ context.Context, query string, + opt *github.SearchOptions) (*github.IssuesSearchResult, *github.Response, error) { total := 0 if foundIssue { total = 1 @@ -168,7 +171,9 @@ Stress build found a failed test: %s`, }, }, nil, nil } - postComment := func(_ context.Context, owner string, repo string, number int, comment *github.IssueComment) (*github.IssueComment, *github.Response, error) { + + p.createComment = func(_ context.Context, owner string, repo string, number int, + comment *github.IssueComment) (*github.IssueComment, *github.Response, error) { if owner != expOwner { t.Fatalf("got %s, expected %s", owner, expOwner) } @@ -186,7 +191,8 @@ Stress build found a failed test: %s`, return nil, nil, nil } - listCommits := func(_ context.Context, owner string, repo string, opts *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error) { + p.listCommits = func(_ context.Context, owner string, repo string, + opts *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error) { if owner != expOwner { t.Fatalf("got %s, expected %s", owner, expOwner) } @@ -206,7 +212,8 @@ Stress build found a failed test: %s`, }, nil, nil } - listMilestones := func(_ context.Context, owner, repo string, _ *github.MilestoneListOptions) ([]*github.Milestone, *github.Response, error) { + p.listMilestones = func(_ context.Context, owner, repo string, + _ *github.MilestoneListOptions) ([]*github.Milestone, *github.Response, error) { if owner != expOwner { t.Fatalf("got %s, expected %s", owner, expOwner) } @@ -219,20 +226,15 @@ Stress build found a failed test: %s`, }, nil, nil } - getLatestTag := func() (string, error) { return "3.3", nil } - - if err := runGH( - context.Background(), - file, - postIssue, - searchIssues, - postComment, - listCommits, - listMilestones, - getLatestTag, - ); err != nil { + p.getLatestTag = func() (string, error) { return "3.3", nil } + + p.init() + + ctx := context.Background() + if err := p.post(ctx, " under stress", c.packageName, c.testName, c.message); err != nil { t.Fatal(err) } + expectedIssues := 1 expectedComments := 0 if foundIssue {