diff --git a/pkg/cmd/roachtest/election.go b/pkg/cmd/roachtest/election.go index e243e89d1383..1d649371f96d 100644 --- a/pkg/cmd/roachtest/election.go +++ b/pkg/cmd/roachtest/election.go @@ -14,6 +14,7 @@ import ( "context" "time" + "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/util/timeutil" ) @@ -21,9 +22,10 @@ func registerElectionAfterRestart(r *testRegistry) { r.Add(testSpec{ Name: "election-after-restart", Owner: OwnerKV, - Skip: "https://github.com/cockroachdb/cockroach/issues/35047", Cluster: makeClusterSpec(3), Run: func(ctx context.Context, t *test, c *cluster) { + skip.UnderRace(t, "race builds make this test exceed its timeout") + t.Status("starting up") c.Put(ctx, cockroach, "./cockroach") c.Start(ctx, t) diff --git a/pkg/cmd/roachtest/kv.go b/pkg/cmd/roachtest/kv.go index 3a50bd3a078b..3d756c9748e2 100644 --- a/pkg/cmd/roachtest/kv.go +++ b/pkg/cmd/roachtest/kv.go @@ -23,6 +23,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/kv" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/ts/tspb" "github.com/cockroachdb/cockroach/pkg/util/httputil" "github.com/cockroachdb/cockroach/pkg/util/timeutil" @@ -349,11 +350,12 @@ func registerKVQuiescenceDead(r *testRegistry) { func registerKVGracefulDraining(r *testRegistry) { r.Add(testSpec{ - Skip: "https://github.com/cockroachdb/cockroach/issues/33501", Name: "kv/gracefuldraining/nodes=3", Owner: OwnerKV, Cluster: makeClusterSpec(4), Run: func(ctx context.Context, t *test, c *cluster) { + skip.UnderRace(t, "race builds make this test exceed its timeout") + nodes := c.spec.NodeCount - 1 c.Put(ctx, cockroach, "./cockroach", c.Range(1, nodes)) c.Put(ctx, workload, "./workload", c.Node(nodes+1)) diff --git a/pkg/cmd/roachtest/test.go b/pkg/cmd/roachtest/test.go index 98e317b5bfe0..8e6992176172 100644 --- a/pkg/cmd/roachtest/test.go +++ b/pkg/cmd/roachtest/test.go @@ -21,6 +21,7 @@ import ( "strings" "time" + "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/util/syncutil" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/cockroach/pkg/util/version" @@ -232,11 +233,24 @@ func (t *test) WorkerProgress(frac float64) { t.progress(goid.Get(), frac) } -// Skip records msg into t.spec.Skip and calls panic(errTestFatal) - thus -// interrupting the running of the test. -func (t *test) Skip(msg string, details string) { - t.spec.Skip = msg - t.spec.SkipDetails = details +var _ skip.SkippableTest = (*test)(nil) + +// Skip skips the test. The first argument if any is the main message. +// The remaining argument, if any, form the details. +// This implements the skip.SkippableTest interface. +func (t *test) Skip(args ...interface{}) { + if len(args) > 0 { + t.spec.Skip = fmt.Sprint(args[0]) + args = args[1:] + } + t.spec.SkipDetails = fmt.Sprint(args...) + panic(errTestFatal) +} + +// Skipf skips the test. The formatted message becomes the skip reason. +// This implements the skip.SkippableTest interface. +func (t *test) Skipf(format string, args ...interface{}) { + t.spec.Skip = fmt.Sprintf(format, args...) panic(errTestFatal) } diff --git a/pkg/testutils/skip/skip.go b/pkg/testutils/skip/skip.go index 2d50520a5733..9f1648f4a1ac 100644 --- a/pkg/testutils/skip/skip.go +++ b/pkg/testutils/skip/skip.go @@ -17,8 +17,14 @@ import ( "github.com/cockroachdb/cockroach/pkg/util" ) +// SkippableTest is a testing.TB with Skip methods. +type SkippableTest interface { + Skip(...interface{}) + Skipf(string, ...interface{}) +} + // WithIssue skips this test, logging the given issue ID as the reason. -func WithIssue(t testing.TB, githubIssueID int, args ...interface{}) { +func WithIssue(t SkippableTest, githubIssueID int, args ...interface{}) { t.Skip(append([]interface{}{ fmt.Sprintf("https://github.com/cockroachdb/cockroach/issue/%d", githubIssueID)}, args...)) @@ -27,31 +33,31 @@ func WithIssue(t testing.TB, githubIssueID int, args ...interface{}) { // IgnoreLint skips this test, explicitly marking it as not a test that // should be tracked as a "skipped test" by external tools. You should use this // if, for example, your test should only be run in Race mode. -func IgnoreLint(t testing.TB, args ...interface{}) { +func IgnoreLint(t SkippableTest, args ...interface{}) { t.Skip(args...) } // IgnoreLintf is like IgnoreLint, and it also takes a format string. -func IgnoreLintf(t testing.TB, format string, args ...interface{}) { +func IgnoreLintf(t SkippableTest, format string, args ...interface{}) { t.Skipf(format, args...) } // UnderRace skips this test if the race detector is enabled. -func UnderRace(t testing.TB, args ...interface{}) { +func UnderRace(t SkippableTest, args ...interface{}) { if util.RaceEnabled { t.Skip(append([]interface{}{"disabled under race"}, args...)) } } // UnderShort skips this test if the -short flag is specified. -func UnderShort(t testing.TB, args ...interface{}) { +func UnderShort(t SkippableTest, args ...interface{}) { if testing.Short() { t.Skip(append([]interface{}{"disabled under -short"}, args...)) } } // UnderStress skips this test when running under stress. -func UnderStress(t testing.TB, args ...interface{}) { +func UnderStress(t SkippableTest, args ...interface{}) { if NightlyStress() { t.Skip(append([]interface{}{"disabled under stress"}, args...)) } @@ -59,7 +65,7 @@ func UnderStress(t testing.TB, args ...interface{}) { // UnderStressRace skips this test during stressrace runs, which are tests // run under stress with the -race flag. -func UnderStressRace(t testing.TB, args ...interface{}) { +func UnderStressRace(t SkippableTest, args ...interface{}) { if NightlyStress() && util.RaceEnabled { t.Skip(append([]interface{}{"disabled under stressrace"}, args...)) }