From dbc8bfd85e5065fe1b30dad008f91bc10f88eb43 Mon Sep 17 00:00:00 2001
From: Renato Costa <renato@cockroachlabs.com>
Date: Thu, 26 Jan 2023 14:16:16 -0500
Subject: [PATCH] roachtest: use teardown log when creating GitHub issue

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
---
 pkg/cmd/roachtest/github.go      | 13 ++++---------
 pkg/cmd/roachtest/github_test.go | 11 ++---------
 pkg/cmd/roachtest/test_runner.go |  6 +++---
 3 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/pkg/cmd/roachtest/github.go b/pkg/cmd/roachtest/github.go
index 88b214d55f84..f520ff85573c 100644
--- a/pkg/cmd/roachtest/github.go
+++ b/pkg/cmd/roachtest/github.go
@@ -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
@@ -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,
 	}
@@ -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
 	}
 
@@ -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),
 	)
diff --git a/pkg/cmd/roachtest/github_test.go b/pkg/cmd/roachtest/github_test.go
index d68a46bd11a5..d9ccdb298b80 100644
--- a/pkg/cmd/roachtest/github_test.go
+++ b/pkg/cmd/roachtest/github_test.go
@@ -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)
@@ -188,7 +182,6 @@ func TestCreatePostRequest(t *testing.T) {
 		github := &githubIssues{
 			vmCreateOpts: vmOpts,
 			cluster:      testClusterImpl,
-			l:            nilLogger(),
 			teamLoader:   teamLoadFn,
 		}
 
diff --git a/pkg/cmd/roachtest/test_runner.go b/pkg/cmd/roachtest/test_runner.go
index fd1a8a5f9e45..202715b619ab 100644
--- a/pkg/cmd/roachtest/test_runner.go
+++ b/pkg/cmd/roachtest/test_runner.go
@@ -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...
@@ -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 {
@@ -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 {