diff --git a/pkg/cmd/roachtest/BUILD.bazel b/pkg/cmd/roachtest/BUILD.bazel index 6e3c717a320e..178b79ebd2f2 100644 --- a/pkg/cmd/roachtest/BUILD.bazel +++ b/pkg/cmd/roachtest/BUILD.bazel @@ -78,6 +78,7 @@ go_test( "test_test.go", ], args = ["-test.timeout=55s"], + data = glob(["testdata/**"]), embed = [":roachtest_lib"], deps = [ "//pkg/cmd/internal/issues", @@ -91,6 +92,7 @@ go_test( "//pkg/roachprod/logger", "//pkg/roachprod/vm", "//pkg/testutils", + "//pkg/testutils/echotest", "//pkg/util/quotapool", "//pkg/util/stop", "//pkg/util/syncutil", diff --git a/pkg/cmd/roachtest/github.go b/pkg/cmd/roachtest/github.go index b7d06413cec2..038a12963854 100644 --- a/pkg/cmd/roachtest/github.go +++ b/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" @@ -46,6 +47,29 @@ func roachtestPrefix(p string) string { return "ROACHTEST_" + p } +// generateHelpCommand creates a HelpCommand for createPostRequest +func generateHelpCommand( + clusterName string, start time.Time, end time.Time, +) func(renderer *issues.Renderer) { + return func(renderer *issues.Renderer) { + issues.HelpCommandAsLink( + "roachtest README", + "https://github.com/cockroachdb/cockroach/blob/master/pkg/cmd/roachtest/README.md", + )(renderer) + issues.HelpCommandAsLink( + "How To Investigate (internal)", + "https://cockroachlabs.atlassian.net/l/c/SSSBr8c7", + )(renderer) + // An empty clusterName corresponds to a cluster creation failure + if clusterName != "" { + issues.HelpCommandAsLink( + "Grafana", + fmt.Sprintf("https://go.crdb.dev/p/roachfana/%s/%d/%d", clusterName, start.UnixMilli(), end.UnixMilli()), + )(renderer) + } + } +} + // postIssueCondition encapsulates a condition that causes issue // posting to be skipped. The `reason` field contains a textual // description as to why issue posting was skipped. @@ -98,13 +122,19 @@ 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 - issueOwner := t.Spec().(*registry.TestSpec).Owner - issueName := t.Name() + issueOwner := spec.Owner + issueName := testName + issueClusterName := "" messagePrefix := "" // Overrides to shield eng teams from potential flakes @@ -112,18 +142,17 @@ 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) 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) 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 // (this label may be removed by a human upon closer investigation). - spec := t.Spec().(*registry.TestSpec) labels := []string{"O-roachtest"} if !spec.NonReleaseBlocker { labels = append(labels, "release-blocker") @@ -131,7 +160,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 { @@ -149,7 +178,7 @@ func (g *githubIssues) createPostRequest( branch = "" } - artifacts := fmt.Sprintf("/%s", t.Name()) + artifacts := fmt.Sprintf("/%s", testName) clusterParams := map[string]string{ roachtestPrefix("cloud"): spec.Cluster.Cloud, @@ -173,6 +202,7 @@ func (g *githubIssues) createPostRequest( // N.B. when Arch is specified, it cannot differ from cluster's arch. // Hence, we only emit when arch was unspecified. clusterParams[roachtestPrefix("arch")] = string(g.cluster.arch) + issueClusterName = g.cluster.name } } @@ -185,17 +215,8 @@ func (g *githubIssues) createPostRequest( Artifacts: artifacts, ExtraLabels: labels, ExtraParams: clusterParams, - HelpCommand: func(renderer *issues.Renderer) { - issues.HelpCommandAsLink( - "roachtest README", - "https://github.com/cockroachdb/cockroach/blob/master/pkg/cmd/roachtest/README.md", - )(renderer) - issues.HelpCommandAsLink( - "How To Investigate (internal)", - "https://cockroachlabs.atlassian.net/l/c/SSSBr8c7", - )(renderer) - }, - } + HelpCommand: generateHelpCommand(issueClusterName, start, end), + }, nil } func (g *githubIssues) MaybePost(t *testImpl, l *logger.Logger, message string) error { @@ -205,10 +226,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, ) } diff --git a/pkg/cmd/roachtest/github_test.go b/pkg/cmd/roachtest/github_test.go index 4cf28d3fb85a..2dc43a26f69b 100644 --- a/pkg/cmd/roachtest/github_test.go +++ b/pkg/cmd/roachtest/github_test.go @@ -14,8 +14,10 @@ import ( "context" "errors" "fmt" + "path/filepath" "strings" "testing" + "time" "github.com/cockroachdb/cockroach/pkg/cmd/internal/issues" "github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster" @@ -25,6 +27,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/internal/team" rperrors "github.com/cockroachdb/cockroach/pkg/roachprod/errors" "github.com/cockroachdb/cockroach/pkg/roachprod/vm" + "github.com/cockroachdb/cockroach/pkg/testutils/echotest" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -103,6 +106,16 @@ func TestShouldPost(t *testing.T) { } } +func TestGenerateHelpCommand(t *testing.T) { + start := time.Date(2023, time.July, 21, 16, 34, 3, 817, time.UTC) + end := time.Date(2023, time.July, 21, 16, 42, 13, 137, time.UTC) + + r := &issues.Renderer{} + generateHelpCommand("foo-cluster", start, end)(r) + + echotest.Require(t, r.String(), filepath.Join("testdata", "help_command.txt")) +} + func TestCreatePostRequest(t *testing.T) { createFailure := func(ref error) failure { return failure{squashedErr: ref} @@ -173,11 +186,13 @@ func TestCreatePostRequest(t *testing.T) { } ti := &testImpl{ - spec: testSpec, - l: nilLogger(), + spec: testSpec, + l: nilLogger(), + start: time.Date(2023, time.July, 21, 16, 34, 3, 817, time.UTC), + end: time.Date(2023, time.July, 21, 16, 42, 13, 137, time.UTC), } - testClusterImpl := &clusterImpl{spec: clusterSpec, arch: vm.ArchAMD64} + testClusterImpl := &clusterImpl{spec: clusterSpec, arch: vm.ArchAMD64, name: "foo"} vo := vm.DefaultCreateOpts() vmOpts := &vo @@ -202,10 +217,17 @@ 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") + + r := &issues.Renderer{} + req.HelpCommand(r) + file := fmt.Sprintf("help_command_createpost_%d.txt", idx+1) + echotest.Require(t, r.String(), filepath.Join("testdata", file)) if c.expectedParams != nil { require.Equal(t, c.expectedParams, req.ExtraParams) diff --git a/pkg/cmd/roachtest/testdata/help_command.txt b/pkg/cmd/roachtest/testdata/help_command.txt new file mode 100644 index 000000000000..bce1bd5c6833 --- /dev/null +++ b/pkg/cmd/roachtest/testdata/help_command.txt @@ -0,0 +1,17 @@ +echo +---- +---- + + +See: [roachtest README](https://github.com/cockroachdb/cockroach/blob/master/pkg/cmd/roachtest/README.md) + + + +See: [How To Investigate \(internal\)](https://cockroachlabs.atlassian.net/l/c/SSSBr8c7) + + + +See: [Grafana](https://go.crdb.dev/p/roachfana/foo-cluster/1689957243000/1689957733000) + +---- +---- diff --git a/pkg/cmd/roachtest/testdata/help_command_createpost_1.txt b/pkg/cmd/roachtest/testdata/help_command_createpost_1.txt new file mode 100644 index 000000000000..d41d8bebc8db --- /dev/null +++ b/pkg/cmd/roachtest/testdata/help_command_createpost_1.txt @@ -0,0 +1,17 @@ +echo +---- +---- + + +See: [roachtest README](https://github.com/cockroachdb/cockroach/blob/master/pkg/cmd/roachtest/README.md) + + + +See: [How To Investigate \(internal\)](https://cockroachlabs.atlassian.net/l/c/SSSBr8c7) + + + +See: [Grafana](https://go.crdb.dev/p/roachfana/foo/1689957243000/1689957733000) + +---- +---- diff --git a/pkg/cmd/roachtest/testdata/help_command_createpost_2.txt b/pkg/cmd/roachtest/testdata/help_command_createpost_2.txt new file mode 100644 index 000000000000..18f5cf5d344d --- /dev/null +++ b/pkg/cmd/roachtest/testdata/help_command_createpost_2.txt @@ -0,0 +1,13 @@ +echo +---- +---- + + +See: [roachtest README](https://github.com/cockroachdb/cockroach/blob/master/pkg/cmd/roachtest/README.md) + + + +See: [How To Investigate \(internal\)](https://cockroachlabs.atlassian.net/l/c/SSSBr8c7) + +---- +---- diff --git a/pkg/cmd/roachtest/testdata/help_command_createpost_3.txt b/pkg/cmd/roachtest/testdata/help_command_createpost_3.txt new file mode 100644 index 000000000000..18f5cf5d344d --- /dev/null +++ b/pkg/cmd/roachtest/testdata/help_command_createpost_3.txt @@ -0,0 +1,13 @@ +echo +---- +---- + + +See: [roachtest README](https://github.com/cockroachdb/cockroach/blob/master/pkg/cmd/roachtest/README.md) + + + +See: [How To Investigate \(internal\)](https://cockroachlabs.atlassian.net/l/c/SSSBr8c7) + +---- +---- diff --git a/pkg/cmd/roachtest/testdata/help_command_createpost_5.txt b/pkg/cmd/roachtest/testdata/help_command_createpost_5.txt new file mode 100644 index 000000000000..d41d8bebc8db --- /dev/null +++ b/pkg/cmd/roachtest/testdata/help_command_createpost_5.txt @@ -0,0 +1,17 @@ +echo +---- +---- + + +See: [roachtest README](https://github.com/cockroachdb/cockroach/blob/master/pkg/cmd/roachtest/README.md) + + + +See: [How To Investigate \(internal\)](https://cockroachlabs.atlassian.net/l/c/SSSBr8c7) + + + +See: [Grafana](https://go.crdb.dev/p/roachfana/foo/1689957243000/1689957733000) + +---- +----