Skip to content

Commit fa69450

Browse files
author
Renato Costa
committed
roachtest: print issue number after test failure
This commit updates the GitHub issue poster so that information about the issue is returned when an issue is created or a comment added. The roachtest test runner uses this information in the TeamCity output so that we can easily see the issue corresponding to a test failure directly in the TC overview page. Epic: none Release note: None
1 parent bf5ffc4 commit fa69450

5 files changed

Lines changed: 67 additions & 20 deletions

File tree

‎pkg/cmd/bazci/githubpost/githubpost.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,8 @@ func DefaultIssueFilerFromFormatter(
104104
}
105105
req.ExtraParams["stress"] = "true"
106106
}
107-
return issues.Post(ctx, log.Default(), fmter, req, opts)
107+
_, err := issues.Post(ctx, log.Default(), fmter, req, opts)
108+
return err
108109
}
109110

110111
}

‎pkg/cmd/internal/issues/issues.go‎

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,34 @@ func buildIssueQueries(
337337
return existingIssueQuery, relatedIssuesQuery
338338
}
339339

340-
func (p *poster) post(origCtx context.Context, formatter IssueFormatter, req PostRequest) error {
340+
type TestFailureType string
341+
342+
const (
343+
TestFailureNewIssue = TestFailureType("new_issue")
344+
TestFailureIssueComment = TestFailureType("comment")
345+
)
346+
347+
// TestFailureIssue encapsulates data about an issue created or
348+
// changed in order to report a test failure.
349+
type TestFailureIssue struct {
350+
Type TestFailureType
351+
ID int
352+
}
353+
354+
func (tfi TestFailureIssue) String() string {
355+
switch tfi.Type {
356+
case TestFailureNewIssue:
357+
return fmt.Sprintf("created new GitHub issue #%d", tfi.ID)
358+
case TestFailureIssueComment:
359+
return fmt.Sprintf("commented on existing GitHub issue #%d", tfi.ID)
360+
default:
361+
return fmt.Sprintf("[unrecognized test failure type %q, ID=%d]", tfi.Type, tfi.ID)
362+
}
363+
}
364+
365+
func (p *poster) post(
366+
origCtx context.Context, formatter IssueFormatter, req PostRequest,
367+
) (*TestFailureIssue, error) {
341368
ctx := &postCtx{Context: origCtx}
342369
data := p.templateData(
343370
ctx,
@@ -402,6 +429,7 @@ func (p *poster) post(origCtx context.Context, formatter IssueFormatter, req Pos
402429
createLabels := []string{RobotLabel}
403430
createLabels = append(createLabels, req.labels()...)
404431
createLabels = append(createLabels, releaseLabel(p.Branch))
432+
var result TestFailureIssue
405433
if foundIssue == nil {
406434
issueRequest := github.IssueRequest{
407435
Title: &title,
@@ -411,11 +439,13 @@ func (p *poster) post(origCtx context.Context, formatter IssueFormatter, req Pos
411439
}
412440
issue, _, err := p.createIssue(ctx, p.Org, p.Repo, &issueRequest)
413441
if err != nil {
414-
return errors.Wrapf(err, "failed to create GitHub issue %s",
442+
return nil, errors.Wrapf(err, "failed to create GitHub issue %s",
415443
github.Stringify(issueRequest))
416444
}
417445

418-
p.l.Printf("created GitHub issue #%d", *issue.Number)
446+
result.Type = TestFailureNewIssue
447+
result.ID = *issue.Number
448+
p.l.Printf("%s", result)
419449
if req.ProjectColumnID != 0 {
420450
_, _, err := p.createProjectCard(ctx, int64(req.ProjectColumnID), &github.ProjectCardOptions{
421451
ContentID: *issue.ID,
@@ -433,14 +463,16 @@ func (p *poster) post(origCtx context.Context, formatter IssueFormatter, req Pos
433463
comment := github.IssueComment{Body: github.String(body)}
434464
if _, _, err := p.createComment(
435465
ctx, p.Org, p.Repo, *foundIssue, &comment); err != nil {
436-
return errors.Wrapf(err, "failed to update issue #%d with %s",
466+
return nil, errors.Wrapf(err, "failed to update issue #%d with %s",
437467
*foundIssue, github.Stringify(comment))
438468
} else {
439-
p.l.Printf("created comment on existing GitHub issue (#%d)", *foundIssue)
469+
result.Type = TestFailureIssueComment
470+
result.ID = *foundIssue
471+
p.l.Printf("%s", result)
440472
}
441473
}
442474

443-
return nil
475+
return &result, nil
444476
}
445477

446478
func (p *poster) teamcityURL(tab, fragment string) *url.URL {
@@ -559,9 +591,9 @@ type Logger interface {
559591
// will be returned.
560592
func Post(
561593
ctx context.Context, l Logger, formatter IssueFormatter, req PostRequest, opts *Options,
562-
) error {
594+
) (*TestFailureIssue, error) {
563595
if !opts.CanPost() {
564-
return errors.Newf("GITHUB_API_TOKEN env variable is not set; cannot post issue")
596+
return nil, errors.Newf("GITHUB_API_TOKEN env variable is not set; cannot post issue")
565597
}
566598

567599
client := github.NewClient(oauth2.NewClient(ctx, oauth2.StaticTokenSource(

‎pkg/cmd/internal/issues/issues_test.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -404,15 +404,19 @@ test logs left over in: /go/src/github.com/cockroachdb/cockroach/artifacts/logTe
404404
// Override the default.
405405
req.Labels = []string{}
406406
}
407-
require.NoError(t, p.post(context.Background(), UnitTestFormatter, req))
407+
issue, err := p.post(context.Background(), UnitTestFormatter, req)
408+
require.NoError(t, err)
409+
require.Equal(t, issueNumber, issue.ID)
408410

409411
switch foundIssue {
410412
case foundNoIssue, foundOnlyRelatedIssue:
411413
require.True(t, createdIssue)
412414
require.False(t, createdComment)
415+
require.Equal(t, TestFailureNewIssue, issue.Type)
413416
case foundOnlyMatchingIssue, foundMatchingAndRelatedIssue:
414417
require.False(t, createdIssue)
415418
require.True(t, createdComment)
419+
require.Equal(t, TestFailureIssueComment, issue.Type)
416420
default:
417421
t.Errorf("unhandled: %s", foundIssue)
418422
}
@@ -460,7 +464,8 @@ func TestPostEndToEnd(t *testing.T) {
460464
HelpCommand: UnitTestHelpCommand(""),
461465
}
462466

463-
require.NoError(t, Post(context.Background(), log.Default(), UnitTestFormatter, req, opts))
467+
_, err := Post(context.Background(), log.Default(), UnitTestFormatter, req, opts)
468+
require.NoError(t, err)
464469
}
465470

466471
// setEnv overrides the env variables corresponding to the input map. The

‎pkg/cmd/roachtest/github.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ type githubIssues struct {
3434
disable bool
3535
cluster *clusterImpl
3636
vmCreateOpts *vm.CreateOpts
37-
issuePoster func(context.Context, issues.Logger, issues.IssueFormatter, issues.PostRequest, *issues.Options) error
37+
issuePoster func(context.Context, issues.Logger, issues.IssueFormatter, issues.PostRequest, *issues.Options) (*issues.TestFailureIssue, error)
3838
teamLoader func() (team.Map, error)
3939
}
4040

@@ -301,11 +301,13 @@ func (g *githubIssues) createPostRequest(
301301
}, nil
302302
}
303303

304-
func (g *githubIssues) MaybePost(t *testImpl, l *logger.Logger, message string) error {
304+
func (g *githubIssues) MaybePost(
305+
t *testImpl, l *logger.Logger, message string,
306+
) (*issues.TestFailureIssue, error) {
305307
doPost, skipReason := g.shouldPost(t)
306308
if !doPost {
307309
l.Printf("skipping GitHub issue posting (%s)", skipReason)
308-
return nil
310+
return nil, nil
309311
}
310312

311313
var metamorphicBuild bool
@@ -319,7 +321,7 @@ func (g *githubIssues) MaybePost(t *testImpl, l *logger.Logger, message string)
319321
}
320322
postRequest, err := g.createPostRequest(t.Name(), t.start, t.end, t.spec, t.failures(), message, metamorphicBuild, t.goCoverEnabled)
321323
if err != nil {
322-
return err
324+
return nil, err
323325
}
324326
opts := issues.DefaultOptionsFromEnv()
325327

‎pkg/cmd/roachtest/test_runner.go‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -750,7 +750,7 @@ func (r *testRunner) runWorker(
750750
handleClusterCreationFailure := func(err error) {
751751
t.Error(errClusterProvisioningFailed(err))
752752

753-
if err := github.MaybePost(t, l, t.failureMsg()); err != nil {
753+
if _, err := github.MaybePost(t, l, t.failureMsg()); err != nil {
754754
shout(ctx, l, stdout, "failed to post issue: %s", err)
755755
}
756756
}
@@ -1016,6 +1016,17 @@ func (r *testRunner) runTest(
10161016
}
10171017
output := fmt.Sprintf("%s\ntest artifacts and logs in: %s", failureMsg, t.ArtifactsDir())
10181018

1019+
issue, err := github.MaybePost(t, l, output)
1020+
if err != nil {
1021+
shout(ctx, l, stdout, "failed to post issue: %s", err)
1022+
}
1023+
1024+
// If an issue was created (or comment added) on GitHub,
1025+
// include that information in the output so that it can be
1026+
// easily inspected on the TeamCity overview page.
1027+
if issue != nil {
1028+
output += "\n" + issue.String()
1029+
}
10191030
if roachtestflags.TeamCity {
10201031
// If `##teamcity[testFailed ...]` is not present before `##teamCity[testFinished ...]`,
10211032
// TeamCity regards the test as successful.
@@ -1024,10 +1035,6 @@ func (r *testRunner) runTest(
10241035
}
10251036

10261037
shout(ctx, l, stdout, "--- FAIL: %s (%s)\n%s", testRunID, durationStr, output)
1027-
1028-
if err := github.MaybePost(t, l, output); err != nil {
1029-
shout(ctx, l, stdout, "failed to post issue: %s", err)
1030-
}
10311038
} else {
10321039
shout(ctx, l, stdout, "--- PASS: %s (%s)", testRunID, durationStr)
10331040
}

0 commit comments

Comments
 (0)