Skip to content

Commit

Permalink
test: enforce skip.UnderStress[Race] locally
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@HonoreDB
HonoreDB committed Jul 14, 2022
1 parent a21c8b9 commit 393120d
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 12 deletions.
4 changes: 2 additions & 2 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/dev/test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/dev/testdata/datadriven/test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions pkg/cmd/dev/testdata/datadriven/testlogic
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
4 changes: 2 additions & 2 deletions pkg/testutils/skip/skip.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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...))
}
}
Expand All @@ -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...))
}
}
Expand Down
12 changes: 10 additions & 2 deletions pkg/testutils/skip/stress.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit 393120d

Please sign in to comment.