roachtest: refactor createPostRequest to include necessary params

This change refactors createPostRequest in two ways:
 - It adds two new time.Time parameters, setting up work for populating
   the testeng Grafana link.
 - Error handling by invoking t.Fatalf has been replaced with returning
   an `error`.

Author: annrpom

pkg/cmd/roachtest/github.go

@@ -14,6 +14,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/cmd/internal/issues"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
@@ -98,14 +99,18 @@ func (g *githubIssues) shouldPost(t test.Test) (bool, string) {
 }
 
 func (g *githubIssues) createPostRequest(
-	t test.Test, firstFailure failure, message string,
-) issues.PostRequest {
+	testName string,
+	start time.Time,
+	end time.Time,
+	spec *registry.TestSpec,
+	firstFailure failure,
+	message string,
+) (issues.PostRequest, error) {
 	var mention []string
 	var projColID int
 
-	spec := t.Spec().(*registry.TestSpec)
 	issueOwner := spec.Owner
-	issueName := t.Name()
+	issueName := testName
 
 	messagePrefix := ""
 	var infraFlake bool
@@ -114,15 +119,15 @@ func (g *githubIssues) createPostRequest(
 	case failureContainsError(firstFailure, errClusterProvisioningFailed):
 		issueOwner = registry.OwnerDevInf
 		issueName = "cluster_creation"
-		messagePrefix = fmt.Sprintf("test %s was skipped due to ", t.Name())
+		messagePrefix = fmt.Sprintf("test %s was skipped due to ", testName)
 		infraFlake = true
 	case failureContainsError(firstFailure, rperrors.ErrSSH255):
 		issueOwner = registry.OwnerTestEng
 		issueName = "ssh_problem"
-		messagePrefix = fmt.Sprintf("test %s failed due to ", t.Name())
+		messagePrefix = fmt.Sprintf("test %s failed due to ", testName)
 		infraFlake = true
 	case failureContainsError(firstFailure, errDuringPostAssertions):
-		messagePrefix = fmt.Sprintf("test %s failed during post test assertions (see test-post-assertions.log) due to ", t.Name())
+		messagePrefix = fmt.Sprintf("test %s failed during post test assertions (see test-post-assertions.log) due to ", testName)
 	}
 
 	// Issues posted from roachtest are identifiable as such, and they are also release blockers
@@ -134,7 +139,7 @@ func (g *githubIssues) createPostRequest(
 
 	teams, err := g.teamLoader()
 	if err != nil {
-		t.Fatalf("could not load teams: %v", err)
+		return issues.PostRequest{}, err
 	}
 
 	if sl, ok := teams.GetAliasesForPurpose(ownerToAlias(issueOwner), team.PurposeRoachtest); ok {
@@ -152,7 +157,7 @@ func (g *githubIssues) createPostRequest(
 		branch = "<unknown branch>"
 	}
 
-	artifacts := fmt.Sprintf("/%s", t.Name())
+	artifacts := fmt.Sprintf("/%s", testName)
 
 	clusterParams := map[string]string{
 		roachtestPrefix("cloud"): spec.Cluster.Cloud,
@@ -202,7 +207,7 @@ func (g *githubIssues) createPostRequest(
 				"https://cockroachlabs.atlassian.net/l/c/SSSBr8c7",
 			)(renderer)
 		},
-	}
+	}, nil
 }
 
 func (g *githubIssues) MaybePost(t *testImpl, l *logger.Logger, message string) error {
@@ -212,10 +217,15 @@ func (g *githubIssues) MaybePost(t *testImpl, l *logger.Logger, message string)
 		return nil
 	}
 
+	postRequest, err := g.createPostRequest(t.Name(), t.start, t.end, t.spec, t.firstFailure(), message)
+	if err != nil {
+		return err
+	}
+
 	return g.issuePoster(
 		context.Background(),
 		l,
 		issues.UnitTestFormatter,
-		g.createPostRequest(t, t.firstFailure(), message),
+		postRequest,
 	)
 }

pkg/cmd/roachtest/github_test.go

@@ -200,10 +200,12 @@ func TestCreatePostRequest(t *testing.T) {
 			}
 
 			if c.loadTeamsFailed {
-				// Assert that if TEAMS.yaml cannot be loaded then function panics.
-				assert.Panics(t, func() { github.createPostRequest(ti, c.failure, "message") })
+				// Assert that if TEAMS.yaml cannot be loaded then function errors.
+				_, err := github.createPostRequest("github_test", ti.start, ti.end, testSpec, c.failure, "message")
+				assert.Error(t, err, "Expected an error in createPostRequest when loading teams fails, but got nil")
 			} else {
-				req := github.createPostRequest(ti, c.failure, "message")
+				req, err := github.createPostRequest("github_test", ti.start, ti.end, testSpec, c.failure, "message")
+				assert.NoError(t, err, "Expected no error in createPostRequest")
 
 				if c.expectedParams != nil {
 					require.Equal(t, c.expectedParams, req.ExtraParams)