bazci,ci,dev: update how stress is invoked

We give both `bazci` and `dev` a subcommand to merge `test.xml` files, and update `dev` and the various `stress` invocations in CI accordingly to make use of the `-shardable-artifacts` feature. Closes #74325. Release note: None
cockroachdb · Jan 21, 2022 · fff8684 · fff8684
1 parent 28c24d6
commit fff8684
Show file tree

Hide file tree

Showing 10 changed files with 109 additions and 16 deletions.
diff --git a/build/teamcity/cockroach/nightlies/stress_impl.sh b/build/teamcity/cockroach/nightlies/stress_impl.sh
@@ -38,7 +38,7 @@ do
                                           --test_env=COCKROACH_NIGHTLY_STRESS=true \
                                           --test_env=GO_TEST_JSON_OUTPUT_FILE=$GO_TEST_JSON_OUTPUT_FILE \
                                           --test_timeout="$TESTTIMEOUTSECS" \
-                                          --run_under "@com_github_cockroachdb_stress//:stress $STRESSFLAGS" \
+                                          --run_under "@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts 'GO_TEST_JSON_OUTPUT_FILE=cat,XML_OUTPUT_FILE=$BAZEL_BIN/pkg/cmd/bazci/bazci_/bazci merge-test-xmls' $STRESSFLAGS" \
                                           --define "gotags=$TAGS" \
                                           --nocache_test_results \
                                           ${EXTRA_BAZEL_FLAGS} \

diff --git a/pkg/cmd/bazci/bazci.go b/pkg/cmd/bazci/bazci.go
@@ -11,6 +11,7 @@ package main
 
 import (
 	"bytes"
+	"encoding/xml"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -24,9 +25,10 @@ import (
 )
 
 const (
-	buildSubcmd        = "build"
-	testSubcmd         = "test"
-	mungeTestXMLSubcmd = "munge-test-xml"
+	buildSubcmd         = "build"
+	testSubcmd          = "test"
+	mergeTestXMLsSubcmd = "merge-test-xmls"
+	mungeTestXMLSubcmd  = "munge-test-xml"
 )
 
 var (
@@ -98,8 +100,8 @@ func parseArgs(args []string, argsLenAtDash int) (*parsedArgs, error) {
 	if len(args) < 2 {
 		return nil, errUsage
 	}
-	if args[0] != buildSubcmd && args[0] != testSubcmd && args[0] != mungeTestXMLSubcmd {
-		return nil, errors.Newf("First argument must be `build`, `test`, or `munge-test-xml`; got %v", args[0])
+	if args[0] != buildSubcmd && args[0] != testSubcmd && args[0] != mungeTestXMLSubcmd && args[0] != mergeTestXMLsSubcmd {
+		return nil, errors.Newf("First argument must be `build`, `test`, `merge-test-xmls`, or `munge-test-xml`; got %v", args[0])
 	}
 	var splitLoc int
 	if argsLenAtDash < 0 {
@@ -213,11 +215,14 @@ func bazciImpl(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	// Special case: munge-test-xml doesn't require running Bazel at all.
+	// Special case: munge-test-xml/merge-test-xmls don't require running Bazel at all.
 	// Perform the munge then exit immediately.
 	if parsedArgs.subcmd == mungeTestXMLSubcmd {
 		return mungeTestXMLs(*parsedArgs)
 	}
+	if parsedArgs.subcmd == mergeTestXMLsSubcmd {
+		return mergeTestXMLs(*parsedArgs)
+	}
 
 	info, err := getBuildInfo(*parsedArgs)
 	if err != nil {
@@ -267,6 +272,23 @@ func mungeTestXMLs(args parsedArgs) error {
 	return nil
 }
 
+func mergeTestXMLs(args parsedArgs) error {
+	var xmlsToMerge []bazelutil.TestSuites
+	for _, file := range args.targets {
+		contents, err := ioutil.ReadFile(file)
+		if err != nil {
+			return err
+		}
+		var testSuites bazelutil.TestSuites
+		err = xml.Unmarshal(contents, &testSuites)
+		if err != nil {
+			return err
+		}
+		xmlsToMerge = append(xmlsToMerge, testSuites)
+	}
+	return bazelutil.MergeTestXMLs(xmlsToMerge, os.Stdout)
+}
+
 func configArgList() []string {
 	ret := []string{}
 	for _, config := range configs {

diff --git a/pkg/cmd/dev/BUILD.bazel b/pkg/cmd/dev/BUILD.bazel
@@ -13,6 +13,7 @@ go_library(
         "lint.go",
         "logic.go",
         "main.go",
+        "merge_test_xmls.go",
         "test.go",
         "util.go",
     ],

diff --git a/pkg/cmd/dev/dev.go b/pkg/cmd/dev/dev.go
@@ -114,6 +114,7 @@ Typical usage:
 		makeDoctorCmd(ret.doctor),
 		makeGenerateCmd(ret.generate),
 		makeGoCmd(ret.gocmd),
+		makeMergeTestXMLsCmd(ret.mergeTestXMLs),
 		makeTestLogicCmd(ret.testlogic),
 		makeLintCmd(ret.lint),
 		makeTestCmd(ret.test),

diff --git a/pkg/cmd/dev/merge_test_xmls.go b/pkg/cmd/dev/merge_test_xmls.go
@@ -0,0 +1,49 @@
+// Copyright 2022 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package main
+
+import (
+	"encoding/xml"
+	"io/ioutil"
+	"os"
+
+	bazelutil "github.com/cockroachdb/cockroach/pkg/build/util"
+	"github.com/spf13/cobra"
+)
+
+func makeMergeTestXMLsCmd(runE func(cmd *cobra.Command, args []string) error) *cobra.Command {
+	mergeTestXMLsCommand := &cobra.Command{
+		Use:   "merge-test-xmls XML1 [XML2...]",
+		Short: "Merge the given test XML's (utility command)",
+		Long:  "Merge the given test XML's (utility command)",
+		Args:  cobra.MinimumNArgs(1),
+		RunE:  runE,
+	}
+	mergeTestXMLsCommand.Hidden = true
+	return mergeTestXMLsCommand
+}
+
+func (d *dev) mergeTestXMLs(cmd *cobra.Command, xmls []string) error {
+	var suites []bazelutil.TestSuites
+	for _, file := range xmls {
+		suitesToAdd := bazelutil.TestSuites{}
+		input, err := ioutil.ReadFile(file)
+		if err != nil {
+			return err
+		}
+		err = xml.Unmarshal(input, &suitesToAdd)
+		if err != nil {
+			return err
+		}
+		suites = append(suites, suitesToAdd)
+	}
+	return bazelutil.MergeTestXMLs(suites, os.Stdout)
+}
diff --git a/pkg/cmd/dev/test.go b/pkg/cmd/dev/test.go
@@ -219,7 +219,10 @@ func (d *dev) test(cmd *cobra.Command, commandLine []string) error {
 		}
 		stressCmdArgs = append(stressCmdArgs, stressArgs)
 		args = append(args, "--run_under",
-			fmt.Sprintf("%s %s", stressTarget, strings.Join(stressCmdArgs, " ")))
+			// NB: Run with -bazel, which propagates `TEST_TMPDIR` to `TMPDIR`,
+			// and -shardable-artifacts set such that we can merge the XML output
+			// files.
+			fmt.Sprintf("%s -bazel -shardable-artifacts 'XML_OUTPUT_FILE=%s merge-test-xmls' %s", stressTarget, getDevBin(), strings.Join(stressCmdArgs, " ")))
 	}
 
 	if filter != "" {

diff --git a/pkg/cmd/dev/testdata/recording/test.txt b/pkg/cmd/dev/testdata/recording/test.txt
@@ -84,7 +84,7 @@ bazel query 'kind(go_test, //pkg/util/tracing:all)'
 ----
 //pkg/util/tracing:tracing_test
 
-bazel test --test_sharding_strategy=disabled //pkg/util/tracing:tracing_test --test_env=GOTRACEBACK=all --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress ' '--test_filter=TestStartChild*' --test_output streamed
+bazel test --test_sharding_strategy=disabled //pkg/util/tracing:tracing_test --test_env=GOTRACEBACK=all --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' '--test_filter=TestStartChild*' --test_output streamed
 ----
 ----
 //pkg/util/tracing:tracing_test                                          [0m[32mPASSED[0m in 12.3s
@@ -97,7 +97,7 @@ bazel query 'kind(go_test, //pkg/util/tracing:all)'
 ----
 //pkg/util/tracing:tracing_test
 
-bazel test --local_cpu_resources=12 --test_sharding_strategy=disabled //pkg/util/tracing:tracing_test --test_env=GOTRACEBACK=all --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress -p=12 ' '--test_filter=TestStartChild*' --test_output streamed
+bazel test --local_cpu_resources=12 --test_sharding_strategy=disabled //pkg/util/tracing:tracing_test --test_env=GOTRACEBACK=all --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' -p=12 ' '--test_filter=TestStartChild*' --test_output streamed
 ----
 ----
 //pkg/util/tracing:tracing_test                                          [0m[32mPASSED[0m in 12.3s
@@ -110,7 +110,7 @@ bazel query 'kind(go_test, //pkg/util/tracing:all)'
 ----
 //pkg/util/tracing:tracing_test
 
-bazel test --test_sharding_strategy=disabled //pkg/util/tracing:tracing_test --test_env=GOTRACEBACK=all --test_timeout=70 --run_under '@com_github_cockroachdb_stress//:stress -maxtime=10s ' '--test_filter=TestStartChild*' --test_arg -test.v --test_output streamed
+bazel test --test_sharding_strategy=disabled //pkg/util/tracing:tracing_test --test_env=GOTRACEBACK=all --test_timeout=70 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' -maxtime=10s ' '--test_filter=TestStartChild*' --test_arg -test.v --test_output streamed
 ----
 ----
 ==================== Test output for //pkg/util/tracing:tracing_test:

diff --git a/pkg/cmd/dev/testdata/test.txt b/pkg/cmd/dev/testdata/test.txt
@@ -31,17 +31,17 @@ bazel test //pkg/util/tracing:tracing_test --nocache_test_results --test_env=GOT
 dev test --stress pkg/util/tracing --filter TestStartChild*
 ----
 bazel query 'kind(go_test, //pkg/util/tracing:all)'
-bazel test --test_sharding_strategy=disabled //pkg/util/tracing:tracing_test --test_env=GOTRACEBACK=all --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress ' '--test_filter=TestStartChild*' --test_output streamed
+bazel test --test_sharding_strategy=disabled //pkg/util/tracing:tracing_test --test_env=GOTRACEBACK=all --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' '--test_filter=TestStartChild*' --test_output streamed
 
 dev test --stress pkg/util/tracing --filter TestStartChild* --cpus=12
 ----
 bazel query 'kind(go_test, //pkg/util/tracing:all)'
-bazel test --local_cpu_resources=12 --test_sharding_strategy=disabled //pkg/util/tracing:tracing_test --test_env=GOTRACEBACK=all --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress -p=12 ' '--test_filter=TestStartChild*' --test_output streamed
+bazel test --local_cpu_resources=12 --test_sharding_strategy=disabled //pkg/util/tracing:tracing_test --test_env=GOTRACEBACK=all --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' -p=12 ' '--test_filter=TestStartChild*' --test_output streamed
 
 dev test --stress pkg/util/tracing --filter TestStartChild* --timeout=10s -v
 ----
 bazel query 'kind(go_test, //pkg/util/tracing:all)'
-bazel test --test_sharding_strategy=disabled //pkg/util/tracing:tracing_test --test_env=GOTRACEBACK=all --test_timeout=70 --run_under '@com_github_cockroachdb_stress//:stress -maxtime=10s ' '--test_filter=TestStartChild*' --test_arg -test.v --test_output streamed
+bazel test --test_sharding_strategy=disabled //pkg/util/tracing:tracing_test --test_env=GOTRACEBACK=all --test_timeout=70 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' -maxtime=10s ' '--test_filter=TestStartChild*' --test_arg -test.v --test_output streamed
 
 dev test //pkg/testutils --timeout=10s
 ----

diff --git a/pkg/cmd/dev/util.go b/pkg/cmd/dev/util.go
@@ -227,3 +227,11 @@ func logCommand(cmd string, args ...string) {
 	fullArgs = append(fullArgs, args...)
 	log.Printf("$ %s", shellescape.QuoteCommand(fullArgs))
 }
+
+// getDevBin returns the path to the running dev executable.
+func getDevBin() string {
+	if isTesting {
+		return "dev"
+	}
+	return os.Args[0]
+}
diff --git a/pkg/cmd/github-pull-request-make/main.go b/pkg/cmd/github-pull-request-make/main.go
@@ -223,6 +223,15 @@ func main() {
 	if forceBazelStr, ok := os.LookupEnv(forceBazelEnv); ok {
 		forceBazel, _ = strconv.ParseBool(forceBazelStr)
 	}
+	var bazciPath string
+	if bazel.BuiltWithBazel() || forceBazel {
+		// NB: bazci is expected to be put in `PATH` by the caller.
+		var err error
+		bazciPath, err = exec.LookPath("bazci")
+		if err != nil {
+			log.Fatal(err)
+		}
+	}
 
 	crdb, err := os.Getwd()
 	if err != nil {
@@ -308,8 +317,8 @@ func main() {
 				args = append(args, "--test_arg=-test.timeout", fmt.Sprintf("--test_arg=%s", timeout))
 				// Give the entire test 1 more minute than the duration to wrap up.
 				args = append(args, fmt.Sprintf("--test_timeout=%d", int((duration+1*time.Minute).Seconds())))
-				args = append(args, "--run_under", fmt.Sprintf("%s -stderr -maxfails 1 -maxtime %s -p %d", bazelStressTarget, duration, parallelism))
-				// NB: bazci is expected to be put in `PATH` by the caller.
+
+				args = append(args, "--run_under", fmt.Sprintf("%s -bazel -shardable-artifacts 'XML_OUTPUT_FILE=%s merge-test-xmls' -stderr -maxfails 1 -maxtime %s -p %d", bazelStressTarget, bazciPath, duration, parallelism))
 				cmd := exec.Command("bazci", args...)
 				cmd.Stdout = os.Stdout
 				cmd.Stderr = os.Stderr