Skip to content

Commit

Permalink
make unit tests faster
Browse files Browse the repository at this point in the history
by enabling parallel tests.
Made unit tests run 30% faster

Also added some parallelism to Datastore
tests, but opt-in. MemDB and datastore proxy
tests are now parallel.

Also changed CRDB test entry points to run in
parallel, since the introduction of integrity tests
meant double the tests to run, and sequentially
  • Loading branch information
vroldanbet committed Oct 2, 2024
1 parent 6710269 commit 2560b66
Show file tree
Hide file tree
Showing 31 changed files with 258 additions and 146 deletions.
10 changes: 8 additions & 2 deletions internal/datastore/crdb/crdb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func (cds *crdbDatastore) ExampleRetryableError() error {
}

func TestCRDBDatastoreWithoutIntegrity(t *testing.T) {
t.Parallel()
b := testdatastore.RunCRDBForTesting(t, "")
test.All(t, test.DatastoreTesterFunc(func(revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) {
ctx := context.Background()
Expand All @@ -72,10 +73,11 @@ func TestCRDBDatastoreWithoutIntegrity(t *testing.T) {
})

return ds, nil
}))
}), false)
}

func TestCRDBDatastoreWithFollowerReads(t *testing.T) {
t.Parallel()
followerReadDelay := time.Duration(4.8 * float64(time.Second))
gcWindow := 100 * time.Second

Expand Down Expand Up @@ -136,6 +138,7 @@ var defaultKeyForTesting = proxy.KeyConfig{
}

func TestCRDBDatastoreWithIntegrity(t *testing.T) {
t.Parallel()
b := testdatastore.RunCRDBForTesting(t, "")

test.All(t, test.DatastoreTesterFunc(func(revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) {
Expand All @@ -159,7 +162,7 @@ func TestCRDBDatastoreWithIntegrity(t *testing.T) {
})

return ds, nil
}))
}), false)

unwrappedTester := test.DatastoreTesterFunc(func(revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) {
ctx := context.Background()
Expand Down Expand Up @@ -187,6 +190,7 @@ func TestCRDBDatastoreWithIntegrity(t *testing.T) {
}

func TestWatchFeatureDetection(t *testing.T) {
t.Parallel()
pool, err := dockertest.NewPool("")
require.NoError(t, err)
cases := []struct {
Expand Down Expand Up @@ -229,7 +233,9 @@ 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)
Expand Down
5 changes: 4 additions & 1 deletion internal/datastore/memdb/memdb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ func (mdbt memDBTest) New(revisionQuantization, _, gcWindow time.Duration, watch
}

func TestMemdbDatastore(t *testing.T) {
test.All(t, memDBTest{})
t.Parallel()
test.All(t, memDBTest{}, true)
}

func TestConcurrentWritePanic(t *testing.T) {
t.Parallel()
require := require.New(t)

ds, err := NewMemdbDatastore(0, 1*time.Hour, 1*time.Hour)
Expand Down Expand Up @@ -81,6 +83,7 @@ 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)
Expand Down
6 changes: 5 additions & 1 deletion internal/datastore/mysql/datastore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,16 @@ func createDatastoreTest(b testdatastore.RunningEngineForTest, tf datastoreTestF
}

func TestMySQLDatastoreDSNWithoutParseTime(t *testing.T) {
t.Parallel()
_, err := NewMySQLDatastore(context.Background(), "root:password@(localhost:1234)/mysql")
require.ErrorContains(t, err, "https://spicedb.dev/d/parse-time-mysql")
}

func TestMySQL8Datastore(t *testing.T) {
t.Parallel()
b := testdatastore.RunMySQLForTestingWithOptions(t, testdatastore.MySQLTesterOptions{MigrateForNewDatastore: true}, "")
dst := datastoreTester{b: b, t: t}
test.AllWithExceptions(t, test.DatastoreTesterFunc(dst.createDatastore), test.WithCategories(test.WatchSchemaCategory, test.WatchCheckpointsCategory))
test.AllWithExceptions(t, test.DatastoreTesterFunc(dst.createDatastore), test.WithCategories(test.WatchSchemaCategory, test.WatchCheckpointsCategory), true)
additionalMySQLTests(t, b)
}

Expand Down Expand Up @@ -660,6 +662,7 @@ func TransactionTimestampsTest(t *testing.T, ds datastore.Datastore) {
}

func TestMySQLMigrations(t *testing.T) {
t.Parallel()
req := require.New(t)

db := datastoreDB(t, false)
Expand All @@ -681,6 +684,7 @@ func TestMySQLMigrations(t *testing.T) {
}

func TestMySQLMigrationsWithPrefix(t *testing.T) {
t.Parallel()
req := require.New(t)

prefix := "spicedb_"
Expand Down
5 changes: 0 additions & 5 deletions internal/datastore/mysql/migrations/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
sq "github.com/Masterminds/squirrel"
sqlDriver "github.com/go-sql-driver/mysql"

log "github.com/authzed/spicedb/internal/logging"
"github.com/authzed/spicedb/pkg/migrate"
)

Expand Down Expand Up @@ -57,10 +56,6 @@ func NewMySQLDriverFromDSN(url string, tablePrefix string, credentialsProvider d
}

db := sql.OpenDB(connector)
err = sqlDriver.SetLogger(&log.Logger)
if err != nil {
return nil, fmt.Errorf("unable to set logging to mysql driver: %w", err)
}
return NewMySQLDriverFromDB(db, tablePrefix), nil
}

Expand Down
4 changes: 2 additions & 2 deletions internal/datastore/postgres/postgres_shared_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func testPostgresDatastore(t *testing.T, pc []postgresConfig) {
return ds
})
return ds, nil
}))
}), false)

t.Run("TransactionTimestamps", createDatastoreTest(
b,
Expand Down Expand Up @@ -236,7 +236,7 @@ func testPostgresDatastoreWithoutCommitTimestamps(t *testing.T, pc []postgresCon
return ds
})
return ds, nil
}), test.WithCategories(test.WatchCategory))
}), test.WithCategories(test.WatchCategory), false)
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/datastore/proxy/observable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func (obs observableTest) New(revisionQuantization, _, gcWindow time.Duration, w
}

func TestObservableProxy(t *testing.T) {
test.All(t, observableTest{})
test.All(t, observableTest{}, true)
}

func (p *observableProxy) ExampleRetryableError() error {
Expand Down
2 changes: 2 additions & 0 deletions internal/datastore/proxy/schemacaching/estimatedsize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ func TestEstimatedDefinitionSizes(t *testing.T) {
for _, caveatDef := range fullyResolved.CaveatDefinitions {
caveatDef := caveatDef
t.Run("caveat "+caveatDef.Name, func(t *testing.T) {
t.Parallel()

serialized, _ := caveatDef.MarshalVT()
sizevt := caveatDef.SizeVT()
estimated := estimatedCaveatDefinitionSize(sizevt)
Expand Down
4 changes: 3 additions & 1 deletion internal/datastore/spanner/spanner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ func (sd *spannerDatastore) ExampleRetryableError() error {
}

func TestSpannerDatastore(t *testing.T) {
t.Parallel()

ctx := context.Background()
b := testdatastore.RunSpannerForTesting(t, "", "head")

Expand All @@ -43,7 +45,7 @@ func TestSpannerDatastore(t *testing.T) {
return ds
})
return ds, nil
}), test.WithCategories(test.GCCategory, test.WatchCategory, test.StatsCategory))
}), test.WithCategories(test.GCCategory, test.WatchCategory, test.StatsCategory), true)

t.Run("TestFakeStats", createDatastoreTest(
b,
Expand Down
11 changes: 9 additions & 2 deletions internal/dispatch/graph/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"testing"

"github.com/stretchr/testify/require"
"go.uber.org/goleak"

"github.com/authzed/spicedb/internal/datastore/common"
"github.com/authzed/spicedb/internal/datastore/memdb"
Expand All @@ -30,7 +29,7 @@ import (
var ONR = tuple.ObjectAndRelation

func TestSimpleCheck(t *testing.T) {
defer goleak.VerifyNone(t, append(testutil.GoLeakIgnores(), goleak.IgnoreCurrent())...)
t.Parallel()

type expected struct {
relation string
Expand Down Expand Up @@ -119,6 +118,7 @@ func TestSimpleCheck(t *testing.T) {
userset := userset
expected := expected
t.Run(name, func(t *testing.T) {
t.Parallel()
require := require.New(t)

ctx, dispatch, revision := newLocalDispatcher(t)
Expand Down Expand Up @@ -150,6 +150,7 @@ func TestSimpleCheck(t *testing.T) {
}

func TestMaxDepth(t *testing.T) {
t.Parallel()
require := require.New(t)

rawDS, err := memdb.NewMemdbDatastore(0, 0, memdb.DisableGC)
Expand Down Expand Up @@ -182,6 +183,7 @@ func TestMaxDepth(t *testing.T) {
}

func TestCheckMetadata(t *testing.T) {
t.Parallel()
type expected struct {
relation string
isMember bool
Expand Down Expand Up @@ -287,6 +289,7 @@ func TestCheckMetadata(t *testing.T) {
}

func TestCheckPermissionOverSchema(t *testing.T) {
t.Parallel()
testCases := []struct {
name string
schema string
Expand Down Expand Up @@ -1370,6 +1373,7 @@ func addFrame(trace *v1.CheckDebugTrace, foundFrames *mapz.Set[string]) {
}

func TestCheckDebugging(t *testing.T) {
t.Parallel()
type expectedFrame struct {
resourceType *core.RelationReference
resourceIDs []string
Expand Down Expand Up @@ -1482,6 +1486,7 @@ func TestCheckDebugging(t *testing.T) {
}

func TestCheckWithHints(t *testing.T) {
t.Parallel()
testCases := []struct {
name string
schema string
Expand Down Expand Up @@ -1853,6 +1858,7 @@ func TestCheckWithHints(t *testing.T) {
}

func TestCheckHintsPartialApplication(t *testing.T) {
t.Parallel()
require := require.New(t)

dispatcher := NewLocalOnlyDispatcher(10, 100)
Expand Down Expand Up @@ -1898,6 +1904,7 @@ func TestCheckHintsPartialApplication(t *testing.T) {
}

func TestCheckHintsPartialApplicationOverArrow(t *testing.T) {
t.Parallel()
require := require.New(t)

dispatcher := NewLocalOnlyDispatcher(10, 100)
Expand Down
6 changes: 6 additions & 0 deletions internal/dispatch/graph/dispatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
)

func TestDispatchChunking(t *testing.T) {
t.Parallel()
schema := `
definition user {
relation self: user
Expand All @@ -35,6 +36,7 @@ func TestDispatchChunking(t *testing.T) {
ctx, dispatcher, revision := newLocalDispatcherWithSchemaAndRels(t, schema, append(enabled, resources...))

t.Run("check", func(t *testing.T) {
t.Parallel()
for _, tpl := range resources[:1] {
checkResult, err := dispatcher.DispatchCheck(ctx, &v1.DispatchCheckRequest{
ResourceRelation: RR(tpl.ResourceAndRelation.Namespace, "view"),
Expand All @@ -55,6 +57,8 @@ func TestDispatchChunking(t *testing.T) {
})

t.Run("lookup-resources", func(t *testing.T) {
t.Parallel()

for _, tpl := range resources[:1] {
stream := dispatch.NewCollectingDispatchStream[*v1.DispatchLookupResourcesResponse](ctx)
err := dispatcher.DispatchLookupResources(&v1.DispatchLookupResourcesRequest{
Expand All @@ -75,6 +79,8 @@ func TestDispatchChunking(t *testing.T) {
})

t.Run("lookup-subjects", func(t *testing.T) {
t.Parallel()

for _, tpl := range resources[:1] {
stream := dispatch.NewCollectingDispatchStream[*v1.DispatchLookupSubjectsResponse](ctx)

Expand Down
7 changes: 3 additions & 4 deletions internal/dispatch/graph/expand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
"google.golang.org/protobuf/encoding/prototext"
"google.golang.org/protobuf/testing/protocmp"

Expand Down Expand Up @@ -137,7 +136,7 @@ var (
)

func TestExpand(t *testing.T) {
defer goleak.VerifyNone(t, append(testutil.GoLeakIgnores(), goleak.IgnoreCurrent())...)
t.Parallel()

testCases := []struct {
start *core.ObjectAndRelation
Expand Down Expand Up @@ -275,7 +274,7 @@ func onrExpr(onr *core.ObjectAndRelation) ast.Expr {
}

func TestMaxDepthExpand(t *testing.T) {
defer goleak.VerifyNone(t, append(testutil.GoLeakIgnores(), goleak.IgnoreCurrent())...)
t.Parallel()

require := require.New(t)

Expand Down Expand Up @@ -306,7 +305,7 @@ func TestMaxDepthExpand(t *testing.T) {
}

func TestExpandOverSchema(t *testing.T) {
defer goleak.VerifyNone(t, append(testutil.GoLeakIgnores(), goleak.IgnoreCurrent())...)
t.Parallel()

testCases := []struct {
name string
Expand Down
4 changes: 4 additions & 0 deletions internal/dispatch/graph/graph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,14 @@ import (
)

func TestUnwrapStatusError(t *testing.T) {
t.Parallel()

err := rewriteError(context.Background(), graph.NewCheckFailureErr(status.Error(codes.Canceled, "canceled")))
grpcutil.RequireStatus(t, codes.Canceled, err)
}

func TestConcurrencyLimitsWithOverallDefaultLimit(t *testing.T) {
t.Parallel()
cl := ConcurrencyLimits{}
require.Equal(t, uint16(0), cl.Check)
require.Equal(t, uint16(0), cl.LookupResources)
Expand All @@ -38,6 +41,7 @@ func TestConcurrencyLimitsWithOverallDefaultLimit(t *testing.T) {
}

func TestSharedConcurrencyLimits(t *testing.T) {
t.Parallel()
cl := SharedConcurrencyLimits(42)
require.Equal(t, uint16(42), cl.Check)
require.Equal(t, uint16(42), cl.LookupResources)
Expand Down
Loading

0 comments on commit 2560b66

Please sign in to comment.