test(*): remove all t.Parallel() calls in the tests

CONTRIBUTING.md

@@ -102,6 +102,14 @@ mage test:integration
 
 Run `mage` or `mage -l` for a full list of test suites.
 
+#### Using t.Parallel
+
+`t.Parallel` is often recommended as a way of speeding up test execution. Our experience in
+this repo is that because of the number of test packages and the fact that `go test`
+already parallelizes over test packages, `t.Parallel` mostly increases peak memory usage
+without meaningfully speeding up test execution. If you're going to use `t.Parallel`,
+ensure that it actually speeds up the test in question.
+
 ### Linting
 
 SpiceDB uses several linters to maintain code and docs quality.

cmd/spicedb/memoryprotection/memory_protection_integration_test.go

@@ -29,8 +29,6 @@ func init() {
 }
 
 func TestServeWithMemoryProtectionMiddleware(t *testing.T) {
-	t.Parallel()
-
 	pool, err := dockertest.NewPool("")
 	require.NoError(t, err)
 

internal/datastore/common/changes_test.go

@@ -971,8 +971,6 @@ func canonicalize(in []datastore.RevisionChanges) []datastore.RevisionChanges {
 }
 
 func TestThreadSafe(t *testing.T) {
-	t.Parallel()
-
 	ch := NewChanges(revisions.TransactionIDKeyFunc, datastore.WatchRelationships|datastore.WatchSchema, math.MaxInt64)
 
 	var wg errgroup.Group

internal/datastore/common/transaction_metadata_test.go

@@ -7,17 +7,13 @@ import (
 )
 
 func TestMustStruct(t *testing.T) {
-	t.Parallel()
-
 	cc := TransactionMetadata{"key": "value"}
 
 	ss := cc.MustStruct()
 	require.NotNil(t, ss)
 }
 
 func TestScan(t *testing.T) {
-	t.Parallel()
-
 	tests := []struct {
 		name     string
 		initial  TransactionMetadata
@@ -50,8 +46,6 @@ func TestScan(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
 			cc := tt.initial
 			err := cc.Scan(tt.val)
 

internal/datastore/crdb/crdb_test.go

@@ -69,7 +69,6 @@ func crdbTestVersion() string {
 }
 
 func TestCRDBDatastoreWithoutIntegrity(t *testing.T) {
-	t.Parallel()
 	b := testdatastore.RunCRDBForTesting(t, "", crdbTestVersion())
 	test.All(t, crdbFactory.NewTester(test.DatastoreTesterFunc(func(_ testing.TB, revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) {
 		ctx := context.Background()
@@ -130,7 +129,6 @@ func createDatastoreTest(b testdatastore.RunningEngineForTest, tf datastoreTestF
 }
 
 func TestCRDBDatastoreWithFollowerReads(t *testing.T) {
-	t.Parallel()
 	followerReadDelay := time.Duration(4.8 * float64(time.Second))
 	gcWindow := 100 * time.Second
 
@@ -142,7 +140,6 @@ func TestCRDBDatastoreWithFollowerReads(t *testing.T) {
 	}
 	for _, quantization := range quantizationDurations {
 		t.Run(fmt.Sprintf("Quantization%s", quantization), func(t *testing.T) {
-			t.Parallel()
 			ctx := context.Background()
 
 			ds := engine.NewDatastore(t, func(engine, uri string) datastore.Datastore {
@@ -196,7 +193,6 @@ var defaultKeyForTesting = proxy.KeyConfig{
 }
 
 func TestCRDBDatastoreWithIntegrity(t *testing.T) { //nolint:tparallel
-	t.Parallel()
 	b := testdatastore.RunCRDBForTesting(t, "", crdbTestVersion())
 
 	test.All(t, crdbFactory.NewTester(test.DatastoreTesterFunc(func(_ testing.TB, revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) {
@@ -256,7 +252,6 @@ func TestCRDBDatastoreWithIntegrity(t *testing.T) { //nolint:tparallel
 }
 
 func TestWatchFeatureDetection(t *testing.T) {
-	t.Parallel()
 	pool, err := dockertest.NewPool("")
 	require.NoError(t, err)
 	cases := []struct {
@@ -301,7 +296,6 @@ func TestWatchFeatureDetection(t *testing.T) {
 	for _, tt := range cases {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
 			ctx, cancel := context.WithCancel(context.Background())
 			t.Cleanup(cancel)
 			adminConn, connStrings := newCRDBWithUser(t, pool)

internal/datastore/crdb/pool/pool_test.go

@@ -81,7 +81,6 @@ func createTestRetryPool(testPool *TestPool) *RetryPool {
 }
 
 func TestContextCancelledDuringBlockingAcquire(t *testing.T) {
-	t.Parallel()
 	synctest.Test(t, func(t *testing.T) {
 		testPool := NewTestPool()
 		testPool.acquireFunc = func(ctx context.Context) (*pgxpool.Conn, error) {
@@ -112,7 +111,6 @@ func TestContextCancelledDuringBlockingAcquire(t *testing.T) {
 }
 
 func TestAcquireTimeoutReturnsErrAcquire(t *testing.T) {
-	t.Parallel()
 	synctest.Test(t, func(t *testing.T) {
 		testPool := NewTestPool()
 		testPool.acquireFunc = func(ctx context.Context) (*pgxpool.Conn, error) {
@@ -143,7 +141,6 @@ func TestAcquireTimeoutReturnsErrAcquire(t *testing.T) {
 }
 
 func TestAcquireSucceedsButTopLevelContextCancelled(t *testing.T) {
-	t.Parallel()
 	testPool := NewTestPool()
 
 	retryPool := createTestRetryPool(testPool)
@@ -160,7 +157,6 @@ func TestAcquireSucceedsButTopLevelContextCancelled(t *testing.T) {
 }
 
 func TestAcquireErrorWithConnectionReturned(t *testing.T) {
-	t.Parallel()
 	synctest.Test(t, func(t *testing.T) {
 		testPool := NewTestPool()
 		testPool.acquireFunc = func(ctx context.Context) (*pgxpool.Conn, error) {
@@ -185,7 +181,6 @@ func TestAcquireErrorWithConnectionReturned(t *testing.T) {
 }
 
 func TestAcquireSucceedsWithinTimeout(t *testing.T) {
-	t.Parallel()
 	synctest.Test(t, func(t *testing.T) {
 		testPool := NewTestPool()
 		testPool.acquireFunc = func(ctx context.Context) (*pgxpool.Conn, error) {
@@ -216,7 +211,6 @@ func TestAcquireSucceedsWithinTimeout(t *testing.T) {
 }
 
 func TestNoAcquireTimeoutUsesOriginalContext(t *testing.T) {
-	t.Parallel()
 	var acquireContext context.Context
 
 	testPool := NewTestPool()
@@ -237,7 +231,6 @@ func TestNoAcquireTimeoutUsesOriginalContext(t *testing.T) {
 }
 
 func TestAcquireTimeoutCreatesSeparateContext(t *testing.T) {
-	t.Parallel()
 	synctest.Test(t, func(t *testing.T) {
 		var acquireContext context.Context
 
@@ -268,7 +261,6 @@ func TestAcquireTimeoutCreatesSeparateContext(t *testing.T) {
 }
 
 func TestAcquireTimeoutContextCausePreserved(t *testing.T) {
-	t.Parallel()
 	synctest.Test(t, func(t *testing.T) {
 		testPool := NewTestPool()
 		testPool.acquireFunc = func(ctx context.Context) (*pgxpool.Conn, error) {
@@ -295,7 +287,6 @@ func TestAcquireTimeoutContextCausePreserved(t *testing.T) {
 }
 
 func TestSuccessfulFunctionExecution(t *testing.T) {
-	t.Parallel()
 	testPool := NewTestPool()
 	testPool.acquireFunc = func(ctx context.Context) (*pgxpool.Conn, error) {
 		return &pgxpool.Conn{}, nil

internal/datastore/memdb/memdb_test.go

@@ -28,12 +28,10 @@ func (memDBTest) New(_ testing.TB, revisionQuantization, _, gcWindow time.Durati
 }
 
 func TestMemdbDatastore(t *testing.T) {
-	t.Parallel()
 	test.All(t, memdbFactory.NewTester(memDBTest{}), true)
 }
 
 func TestConcurrentWritePanic(t *testing.T) {
-	t.Parallel()
 	require := require.New(t)
 
 	ds, err := NewMemdbDatastore(0, 1*time.Hour, 1*time.Hour)
@@ -85,7 +83,6 @@ func TestConcurrentWritePanic(t *testing.T) {
 }
 
 func TestConcurrentWriteRelsError(t *testing.T) {
-	t.Parallel()
 	require := require.New(t)
 
 	ds, err := NewMemdbDatastore(0, 1*time.Hour, 1*time.Hour)
@@ -116,7 +113,6 @@ func TestConcurrentWriteRelsError(t *testing.T) {
 }
 
 func TestAnythingAfterCloseDoesNotPanic(t *testing.T) {
-	t.Parallel()
 	require := require.New(t)
 
 	ds, err := NewMemdbDatastore(0, 1*time.Hour, 1*time.Hour)

internal/datastore/postgres/pgbouncer_test.go

@@ -21,11 +21,9 @@ func pgbouncerTestVersion() string {
 var pgbouncerConfig = postgresTestConfig{"head", "", pgbouncerTestVersion(), true}
 
 func TestPostgresWithPgBouncerDatastore(t *testing.T) {
-	t.Parallel()
 	testPostgresDatastore(t, pgbouncerConfig)
 }
 
 func TestPostgresDatastoreWithPgBouncerWithoutCommitTimestamps(t *testing.T) {
-	t.Parallel()
 	testPostgresDatastoreWithoutCommitTimestamps(t, pgbouncerConfig)
 }

internal/datastore/postgres/postgres_shared_test.go

@@ -110,7 +110,6 @@ func testPostgresDatastore(t *testing.T, config postgresTestConfig) {
 	})
 
 	t.Run(fmt.Sprintf("%spostgres-%s-%s-%s", pgbouncerStr, config.pgVersion, config.targetMigration, config.migrationPhase), func(t *testing.T) {
-		t.Parallel()
 		b := testdatastore.RunPostgresForTesting(t, "", config.targetMigration, config.pgVersion, config.pgbouncer)
 		ctx := context.Background()
 
@@ -306,8 +305,6 @@ func testPostgresDatastoreWithoutCommitTimestamps(t *testing.T, config postgresT
 	pgVersion := config.pgVersion
 	enablePgbouncer := config.pgbouncer
 	t.Run(fmt.Sprintf("postgres-%s", pgVersion), func(t *testing.T) {
-		t.Parallel()
-
 		ctx := context.Background()
 		b := testdatastore.RunPostgresForTestingWithCommitTimestamps(t, "", "head", false, pgVersion, enablePgbouncer)
 

internal/datastore/postgres/postgres_test.go

@@ -29,26 +29,20 @@ func postgresTestVersion() string {
 var postgresConfig = postgresTestConfig{"head", "", postgresTestVersion(), false}
 
 func TestPostgresDatastore(t *testing.T) {
-	t.Parallel()
 	testPostgresDatastore(t, postgresConfig)
 }
 
 func TestPostgresDatastoreWithoutCommitTimestamps(t *testing.T) {
-	t.Parallel()
 	testPostgresDatastoreWithoutCommitTimestamps(t, postgresConfig)
 }
 
 func TestPostgresDatastoreGC(t *testing.T) {
-	t.Parallel()
-
 	config := postgresConfig
 	pgbouncerStr := ""
 	if config.pgbouncer {
 		pgbouncerStr = "pgbouncer-"
 	}
 	t.Run(fmt.Sprintf("%spostgres-gc-%s-%s-%s", pgbouncerStr, config.pgVersion, config.targetMigration, config.migrationPhase), func(t *testing.T) {
-		t.Parallel()
-
 		b := testdatastore.RunPostgresForTesting(t, "", config.targetMigration, config.pgVersion, config.pgbouncer)
 
 		t.Run("GarbageCollection", createDatastoreTest(

internal/datastore/proxy/relationshipintegrity_test.go

@@ -245,8 +245,6 @@ func TestBasicIntegrityFailureDueToWriteWithExpiredKey(t *testing.T) {
 }
 
 func TestWatchIntegrityFailureDueToInvalidHashSignature(t *testing.T) {
-	t.Parallel()
-
 	ds, err := dsfortesting.NewMemDBDatastoreForTesting(t, 0, 5*time.Second, 1*time.Hour)
 	require.NoError(t, err)
 

internal/datastore/proxy/schemacaching/estimatedsize_test.go

@@ -89,8 +89,6 @@ func TestEstimatedDefinitionSizes(t *testing.T) {
 
 			for _, caveatDef := range fullyResolved.CaveatDefinitions {
 				t.Run("caveat "+caveatDef.Name, func(t *testing.T) {
-					t.Parallel()
-
 					serialized, _ := caveatDef.MarshalVT()
 					sizevt := caveatDef.SizeVT()
 					estimated := estimatedCaveatDefinitionSize(sizevt)

internal/datastore/spanner/spanner_test.go

@@ -23,8 +23,6 @@ import (
 var spannerFactory = test.NewTesterFactory(status.New(codes.Aborted, "retryable").Err())
 
 func TestSpannerDatastore(t *testing.T) {
-	// t.Parallel() //nolint:tparallel, the test sets environment variables (the emulator)
-
 	ctx := context.Background()
 	b := testdatastore.RunSpannerForTesting(t, "", "head")