From 393120d8b227590c5fd3a997d1bf20b17b8a5a6f Mon Sep 17 00:00:00 2001 From: Aaron Zinger Date: Wed, 13 Jul 2022 12:52:03 -0400 Subject: [PATCH] test: enforce skip.UnderStress[Race] locally Previously, skip.UnderStress only checked NightlyStress(), which is only set during nightly stress test runs, not by ./dev test --stress or make stress. This lead to developers checking in tests that flake or reliably fail under stress or stressrace. This PR modifies dev and make to set a COCKROACH_STRESS environment variable, and modifies skip to honor both that flag and the one set by nightly stress. A follow-up might be to default to not skipping when a filter matches only one test, as there the intent was almost certainly to actually run it. Release note: None --- Makefile | 4 ++-- pkg/cmd/dev/test.go | 1 + pkg/cmd/dev/testdata/datadriven/test | 2 +- pkg/cmd/dev/testdata/datadriven/testlogic | 10 +++++----- pkg/testutils/skip/skip.go | 4 ++-- pkg/testutils/skip/stress.go | 12 ++++++++++-- 6 files changed, 21 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 4577fec542a1..434298c816ca 100644 --- a/Makefile +++ b/Makefile @@ -1154,10 +1154,10 @@ check test testshort testrace testdeadlock bench benchshort: .PHONY: stress stressrace stress: ## Run tests under stress. $(info $(yellow)[WARNING] Use `dev test --stress $(subst ./,,$(PKG))$(if $(filter . -,$(TESTS)),, --filter $(TESTS))$(if $(TESTFLAGS), --test-args "$(TESTFLAGS)")$(if $(findstring 60m,$(TESTTIMEOUT)),, --timeout $(TESTTIMEOUT))` instead.$(term-reset)) - $(xgo) test $(GOTESTFLAGS) $(GOFLAGS) $(GOMODVENDORFLAGS) -exec 'stress $(STRESSFLAGS)' -tags '$(TAGS)' -ldflags '$(LINKFLAGS)' -run "$(TESTS)" -timeout 0 $(PKG) $(filter-out -v,$(TESTFLAGS)) -v -args -test.timeout $(TESTTIMEOUT) + COCKROACH_STRESS=true $(xgo) test $(GOTESTFLAGS) $(GOFLAGS) $(GOMODVENDORFLAGS) -exec 'stress $(STRESSFLAGS)' -tags '$(TAGS)' -ldflags '$(LINKFLAGS)' -run "$(TESTS)" -timeout 0 $(PKG) $(filter-out -v,$(TESTFLAGS)) -v -args -test.timeout $(TESTTIMEOUT) stressrace: ## Run tests under stress with the race detector enabled. $(info $(yellow)[WARNING] Use `dev test --stress --race $(subst ./,,$(PKG))$(if $(filter . -,$(TESTS)),, --filter $(TESTS))$(if $(TESTFLAGS), --test-args "$(TESTFLAGS)")$(if $(findstring 60m,$(TESTTIMEOUT)),, --timeout $(TESTTIMEOUT))` instead.$(term-reset)) - $(xgo) test $(GOTESTFLAGS) $(GOFLAGS) $(GOMODVENDORFLAGS) -exec 'stress $(STRESSFLAGS)' -tags '$(TAGS)' -ldflags '$(LINKFLAGS)' -run "$(TESTS)" -timeout 0 $(PKG) $(filter-out -v,$(TESTFLAGS)) -v -args -test.timeout $(TESTTIMEOUT) + COCKROACH_STRESS=true $(xgo) test $(GOTESTFLAGS) $(GOFLAGS) $(GOMODVENDORFLAGS) -exec 'stress $(STRESSFLAGS)' -tags '$(TAGS)' -ldflags '$(LINKFLAGS)' -run "$(TESTS)" -timeout 0 $(PKG) $(filter-out -v,$(TESTFLAGS)) -v -args -test.timeout $(TESTTIMEOUT) .PHONY: roachprod-stress roachprod-stressrace roachprod-stress roachprod-stressrace: bin/roachprod-stress diff --git a/pkg/cmd/dev/test.go b/pkg/cmd/dev/test.go index 8320ca086c17..998198d0d3d6 100644 --- a/pkg/cmd/dev/test.go +++ b/pkg/cmd/dev/test.go @@ -343,6 +343,7 @@ func (d *dev) getStressArgs(stressCmdArg string, timeout time.Duration) []string if stressCmdArg != "" { stressCmdArgs = append(stressCmdArgs, stressCmdArg) } + stressArgs = append(stressArgs, "--test_env=COCKROACH_STRESS=true") stressArgs = append(stressArgs, "--run_under", // NB: Run with -bazel, which propagates `TEST_TMPDIR` to `TMPDIR`, // and -shardable-artifacts set such that we can merge the XML output diff --git a/pkg/cmd/dev/testdata/datadriven/test b/pkg/cmd/dev/testdata/datadriven/test index d6b85f800507..ad141df3b317 100644 --- a/pkg/cmd/dev/testdata/datadriven/test +++ b/pkg/cmd/dev/testdata/datadriven/test @@ -26,7 +26,7 @@ bazel test pkg/util/tracing:all --nocache_test_results --test_env=GOTRACEBACK=al exec dev test --stress pkg/util/tracing --filter TestStartChild* --cpus=12 --timeout=25s ---- -bazel test --local_cpu_resources=12 pkg/util/tracing:all --test_env=GOTRACEBACK=all --test_timeout=85 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' -maxtime=25s -p=12' '--test_filter=TestStartChild*' --test_sharding_strategy=disabled --test_output streamed +bazel test --local_cpu_resources=12 pkg/util/tracing:all --test_env=GOTRACEBACK=all --test_timeout=85 --test_env=COCKROACH_STRESS=true --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' -maxtime=25s -p=12' '--test_filter=TestStartChild*' --test_sharding_strategy=disabled --test_output streamed exec dev test //pkg/testutils --timeout=10s diff --git a/pkg/cmd/dev/testdata/datadriven/testlogic b/pkg/cmd/dev/testdata/datadriven/testlogic index eec452ebdfae..fee15e2a633c 100644 --- a/pkg/cmd/dev/testdata/datadriven/testlogic +++ b/pkg/cmd/dev/testdata/datadriven/testlogic @@ -50,12 +50,12 @@ bazel test --test_env=GOTRACEBACK=all --test_arg -test.count=5 --test_arg -show- exec dev testlogic base --files=auto_span_config_reconciliation --stress ---- -bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed +bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --test_env=COCKROACH_STRESS=true --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed exec dev testlogic base --files=auto_span_config_reconciliation --stress --timeout 1m --cpus 8 ---- -bazel test --test_env=GOTRACEBACK=all --local_cpu_resources=8 --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=120 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' -maxtime=1m0s -p=8' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed +bazel test --test_env=GOTRACEBACK=all --local_cpu_resources=8 --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=120 --test_env=COCKROACH_STRESS=true --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' -maxtime=1m0s -p=8' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed exec dev testlogic ccl --rewrite --show-logs -v --files distsql_automatic_stats --config 3node-tenant @@ -66,14 +66,14 @@ bazel test --test_env=GOTRACEBACK=all --nocache_test_results --test_arg -test.v exec dev testlogic base --files=auto_span_config_reconciliation --stress --stream-output ---- -bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed +bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --test_env=COCKROACH_STRESS=true --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed exec dev testlogic base --files=auto_span_config_reconciliation --stress --stream-output ---- -bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed +bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --test_env=COCKROACH_STRESS=true --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed exec dev testlogic base --files=auto_span_config_reconciliation --stress --stream-output ---- -bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed +bazel test --test_env=GOTRACEBACK=all --test_arg -show-sql --test_sharding_strategy=disabled --test_timeout=86400 --test_env=COCKROACH_STRESS=true --run_under '@com_github_cockroachdb_stress//:stress -bazel -shardable-artifacts '"'"'XML_OUTPUT_FILE=dev merge-test-xmls'"'"' ' //pkg/sql/logictest:logictest_test --test_filter 'TestLogic//^auto_span_config_reconciliation$/' --test_output streamed diff --git a/pkg/testutils/skip/skip.go b/pkg/testutils/skip/skip.go index 6be83ff2bb5f..3b1bf65f9193 100644 --- a/pkg/testutils/skip/skip.go +++ b/pkg/testutils/skip/skip.go @@ -114,7 +114,7 @@ func UnderShort(t SkippableTest, args ...interface{}) { // UnderStress skips this test when running under stress. func UnderStress(t SkippableTest, args ...interface{}) { t.Helper() - if NightlyStress() { + if Stress() { t.Skip(append([]interface{}{"disabled under stress"}, args...)) } } @@ -123,7 +123,7 @@ func UnderStress(t SkippableTest, args ...interface{}) { // run under stress with the -race flag. func UnderStressRace(t SkippableTest, args ...interface{}) { t.Helper() - if NightlyStress() && util.RaceEnabled { + if Stress() && util.RaceEnabled { t.Skip(append([]interface{}{"disabled under stressrace"}, args...)) } } diff --git a/pkg/testutils/skip/stress.go b/pkg/testutils/skip/stress.go index 668d3efde668..4368746ca431 100644 --- a/pkg/testutils/skip/stress.go +++ b/pkg/testutils/skip/stress.go @@ -12,10 +12,18 @@ package skip import "github.com/cockroachdb/cockroach/pkg/util/envutil" -var stress = envutil.EnvOrDefaultBool("COCKROACH_NIGHTLY_STRESS", false) +var nightlyStress = envutil.EnvOrDefaultBool("COCKROACH_NIGHTLY_STRESS", false) + +var stress = envutil.EnvOrDefaultBool("COCKROACH_STRESS", false) // NightlyStress returns true iff the process is running as part of CockroachDB's // nightly stress tests. func NightlyStress() bool { - return stress + return nightlyStress +} + +// Stress returns true iff the process is running under any instance of the stress +// harness, including the nightly one. +func Stress() bool { + return stress || nightlyStress }