Skip to content

Commit

Permalink
roachtest: use teardown log when creating GitHub issue
Browse files Browse the repository at this point in the history
This is a follow up of #95831. The logger passed to the `githubIssues`
struct writes to the test runner logger, which is not ideal. This
changes the logger passed to use the `teardown` logger, so log entries
related to GitHub issue creation are in the same directory as the
failing test itself.

Release note: None
  • Loading branch information
renatolabs committed Jan 26, 2023
1 parent 705d6a1 commit dbc8bfd
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 21 deletions.
13 changes: 4 additions & 9 deletions pkg/cmd/roachtest/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

type githubIssues struct {
disable bool
l *logger.Logger
cluster *clusterImpl
vmCreateOpts *vm.CreateOpts
issuePoster func(context.Context, *logger.Logger, issues.IssueFormatter, issues.PostRequest) error
Expand All @@ -41,15 +40,11 @@ const (
sshErr
)

func newGithubIssues(
disable bool, c *clusterImpl, vmCreateOpts *vm.CreateOpts, l *logger.Logger,
) *githubIssues {

func newGithubIssues(disable bool, c *clusterImpl, vmCreateOpts *vm.CreateOpts) *githubIssues {
return &githubIssues{
disable: disable,
vmCreateOpts: vmCreateOpts,
cluster: c,
l: l,
issuePoster: issues.Post,
teamLoader: team.DefaultLoadTeams,
}
Expand Down Expand Up @@ -201,10 +196,10 @@ func (g *githubIssues) createPostRequest(
}
}

func (g *githubIssues) MaybePost(t *testImpl, message string) error {
func (g *githubIssues) MaybePost(t *testImpl, l *logger.Logger, message string) error {
doPost, skipReason := g.shouldPost(t)
if !doPost {
g.l.Printf("skipping GitHub issue posting (%s)", skipReason)
l.Printf("skipping GitHub issue posting (%s)", skipReason)
return nil
}

Expand All @@ -220,7 +215,7 @@ func (g *githubIssues) MaybePost(t *testImpl, message string) error {

return g.issuePoster(
context.Background(),
g.l,
l,
issues.UnitTestFormatter,
g.createPostRequest(t, cat, message),
)
Expand Down
11 changes: 2 additions & 9 deletions pkg/cmd/roachtest/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,8 @@ func TestShouldPost(t *testing.T) {
Run: func(ctx context.Context, t test.Test, c cluster.Cluster) {},
}

ti := &testImpl{
spec: testSpec,
l: nilLogger(),
}

github := &githubIssues{
disable: c.disableIssues,
}
ti := &testImpl{spec: testSpec}
github := &githubIssues{disable: c.disableIssues}

doPost, skipReason := github.shouldPost(ti)
require.Equal(t, c.expectedPost, doPost)
Expand Down Expand Up @@ -188,7 +182,6 @@ func TestCreatePostRequest(t *testing.T) {
github := &githubIssues{
vmCreateOpts: vmOpts,
cluster: testClusterImpl,
l: nilLogger(),
teamLoader: teamLoadFn,
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/cmd/roachtest/test_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ func (r *testRunner) runWorker(
// Now run the test.
l.PrintfCtx(ctx, "starting test: %s:%d", testToRun.spec.Name, testToRun.runNum)

github := newGithubIssues(r.config.disableIssue, c, vmCreateOpts, l)
github := newGithubIssues(r.config.disableIssue, c, vmCreateOpts)

if clusterCreateErr != nil {
// N.B. cluster creation must have failed...
Expand All @@ -648,7 +648,7 @@ func (r *testRunner) runWorker(
// Generate failure reason and mark the test failed to preclude fetching (cluster) artifacts.
t.Error(clusterCreateErr)
// N.B. issue title is of the form "roachtest: ${t.spec.Name} failed" (see UnitTestFormatter).
if err := github.MaybePost(t, t.failureMsg()); err != nil {
if err := github.MaybePost(t, l, t.failureMsg()); err != nil {
shout(ctx, l, stdout, "failed to post issue: %s", err)
}
} else {
Expand Down Expand Up @@ -841,7 +841,7 @@ func (r *testRunner) runTest(

shout(ctx, l, stdout, "--- FAIL: %s (%s)\n%s", runID, durationStr, output)

if err := github.MaybePost(t, output); err != nil {
if err := github.MaybePost(t, l, output); err != nil {
shout(ctx, l, stdout, "failed to post issue: %s", err)
}
} else {
Expand Down

0 comments on commit dbc8bfd

Please sign in to comment.