Skip to content

Commit 12abfa7

Browse files
committed
asim: run no_rand tests in parallel
Each simulation is single-threaded, so it makes sense to evaluate different testfiles in parallel. A complication is that each simulation logs using our default logging library, and since that is a singleton, we can't provide each simulation with its own logger. When a subtest is a parallel test (as is the case here), it waits for the parent test to return before running. This means that the `log.Scope` in `TestDataDriven` would be cleaned up by the time the parallel subtests would actually start simulating. I'm avoiding this by creating the scope, but then releasing it via `t.Cleanup`, which only runs when all subtests have completed. Something similar is done with `leaktest.AfterTest`. ``` ./dev test pkg/kv/kvserver/asim/tests -f 'TestDataDriven' --ignore-cache --rewrite -v -- --test_env COCKROACH_RUN_ASIM_TESTS=true //pkg/kv/kvserver/asim/tests:tests_test PASSED in 202.1s # before //pkg/kv/kvserver/asim/tests:tests_test PASSED in 60.6s # after ``` Epic: CRDB-25222
1 parent cc27e28 commit 12abfa7

File tree

2 files changed

+19
-5
lines changed

2 files changed

+19
-5
lines changed

pkg/kv/kvserver/asim/tests/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ go_test(
5656
"//pkg/util/leaktest",
5757
"//pkg/util/log",
5858
"@com_github_cockroachdb_datadriven//:datadriven",
59+
"@com_github_cockroachdb_logtags//:logtags",
5960
"@com_github_stretchr_testify//require",
6061
"@org_gonum_v1_plot//:plot",
6162
"@org_gonum_v1_plot//plotter",

pkg/kv/kvserver/asim/tests/datadriven_simulation_test.go

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
3636
"github.com/cockroachdb/cockroach/pkg/util/log"
3737
"github.com/cockroachdb/datadriven"
38+
"github.com/cockroachdb/logtags"
3839
"github.com/stretchr/testify/require"
3940
"gonum.org/v1/plot"
4041
"gonum.org/v1/plot/plotter"
@@ -182,16 +183,27 @@ var runAsimTests = envutil.EnvOrDefaultBool("COCKROACH_RUN_ASIM_TESTS", false)
182183
// ..US_3
183184
// ....└── [11 12 13 14 15]
184185
func TestDataDriven(t *testing.T) {
185-
defer leaktest.AfterTest(t)()
186-
defer log.Scope(t).Close(t)
187186
skip.UnderDuressWithIssue(t, 149875)
188-
ctx := context.Background()
187+
leakTestAfter := leaktest.AfterTest(t)
189188
dir := datapathutils.TestDataPath(t, "non_rand")
189+
190+
// NB: we must use t.Cleanup instead of deferring this
191+
// cleanup in the main test due to the use of t.Parallel
192+
// below.
193+
// See https://github.com/golang/go/issues/31651.
194+
scope := log.Scope(t)
195+
t.Cleanup(func() {
196+
scope.Close(t)
197+
leakTestAfter()
198+
})
190199
datadriven.Walk(t, dir, func(t *testing.T, path string) {
191200
if filepath.Ext(path) != ".txt" {
192201
return
193202
}
194-
plotNum := 0
203+
ctx := logtags.AddTag(context.Background(), "name", filepath.Base(path))
204+
// The inline comment below is required for TestLint/TestTParallel.
205+
// We use t.Cleanup to work around the issue this lint is trying to prevent.
206+
t.Parallel() // SAFE FOR TESTING
195207
const defaultKeyspace = 10000
196208
loadGen := gen.MultiLoad{}
197209
var clusterGen gen.ClusterGen
@@ -200,7 +212,8 @@ func TestDataDriven(t *testing.T) {
200212
eventGen := gen.NewStaticEventsWithNoEvents()
201213
assertions := []assertion.SimulationAssertion{}
202214
var stateStrAcrossSamples []string
203-
runs := []history.History{}
215+
var runs []history.History
216+
plotNum := 0
204217
datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string {
205218
defer func() {
206219
require.Empty(t, d.CmdArgs, "leftover arguments for %s", d.Cmd)

0 commit comments

Comments
 (0)