From 22741b5bfdcad335a0944f6b569e8fc475edc416 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 23 Feb 2024 08:53:23 -0800 Subject: [PATCH] Remove duplicate testing code --- .../postgres/postgres_shared_test.go | 57 +------------------ pkg/datastore/test/watch.go | 16 +++--- 2 files changed, 10 insertions(+), 63 deletions(-) diff --git a/internal/datastore/postgres/postgres_shared_test.go b/internal/datastore/postgres/postgres_shared_test.go index e4affaceb6..af217a0986 100644 --- a/internal/datastore/postgres/postgres_shared_test.go +++ b/internal/datastore/postgres/postgres_shared_test.go @@ -5,7 +5,6 @@ package postgres import ( "context" - "errors" "fmt" "math/rand" "strings" @@ -28,7 +27,6 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgconn" "github.com/samber/lo" - "github.com/scylladb/go-set/strset" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/sdk/trace" @@ -1442,7 +1440,7 @@ func NullCaveatWatchTest(t *testing.T, ds datastore.Datastore) { require.NoError(err) // Verify the relationship create was tracked by the watch. - verifyUpdates(require, [][]*core.RelationTupleUpdate{ + test.VerifyUpdates(require, [][]*core.RelationTupleUpdate{ { tuple.Touch(tuple.Parse("resource:someresourceid#somerelation@subject:somesubject")), }, @@ -1458,7 +1456,7 @@ func NullCaveatWatchTest(t *testing.T, ds datastore.Datastore) { require.NoError(err) // Verify the delete. - verifyUpdates(require, [][]*core.RelationTupleUpdate{ + test.VerifyUpdates(require, [][]*core.RelationTupleUpdate{ { tuple.Delete(tuple.Parse("resource:someresourceid#somerelation@subject:somesubject")), }, @@ -1470,54 +1468,3 @@ func NullCaveatWatchTest(t *testing.T, ds datastore.Datastore) { } const waitForChangesTimeout = 5 * time.Second - -// TODO(jschorr): Combine with the same impl in the datastore shared tests -func verifyUpdates( - require *require.Assertions, - testUpdates [][]*core.RelationTupleUpdate, - changes <-chan *datastore.RevisionChanges, - errchan <-chan error, - expectDisconnect bool, -) { - for _, expected := range testUpdates { - changeWait := time.NewTimer(waitForChangesTimeout) - select { - case change, ok := <-changes: - if !ok { - require.True(expectDisconnect, "unexpected disconnect") - errWait := time.NewTimer(waitForChangesTimeout) - select { - case err := <-errchan: - require.True(errors.As(err, &datastore.ErrWatchDisconnected{})) - return - case <-errWait.C: - require.Fail("Timed out waiting for ErrWatchDisconnected") - } - return - } - - expectedChangeSet := setOfChanges(expected) - actualChangeSet := setOfChanges(change.RelationshipChanges) - - missingExpected := strset.Difference(expectedChangeSet, actualChangeSet) - unexpected := strset.Difference(actualChangeSet, expectedChangeSet) - - require.True(missingExpected.IsEmpty(), "expected changes missing: %s", missingExpected) - require.True(unexpected.IsEmpty(), "unexpected changes: %s", unexpected) - - time.Sleep(1 * time.Millisecond) - case <-changeWait.C: - require.Fail("Timed out", "waiting for changes: %s", expected) - } - } - - require.False(expectDisconnect, "all changes verified without expected disconnect") -} - -func setOfChanges(changes []*core.RelationTupleUpdate) *strset.Set { - changeSet := strset.NewWithSize(len(changes)) - for _, change := range changes { - changeSet.Add(fmt.Sprintf("OPERATION_%s(%s)", change.Operation, tuple.StringWithoutCaveat(change.Tuple))) - } - return changeSet -} diff --git a/pkg/datastore/test/watch.go b/pkg/datastore/test/watch.go index 2d02ab6609..93efd9af6f 100644 --- a/pkg/datastore/test/watch.go +++ b/pkg/datastore/test/watch.go @@ -125,16 +125,16 @@ func WatchTest(t *testing.T, tester DatastoreTester) { testUpdates = append(testUpdates, bulkDeletes) } - verifyUpdates(require, testUpdates, changes, errchan, tc.expectFallBehind) + VerifyUpdates(require, testUpdates, changes, errchan, tc.expectFallBehind) // Test the catch-up case changes, errchan = ds.Watch(ctx, lowestRevision, opts) - verifyUpdates(require, testUpdates, changes, errchan, tc.expectFallBehind) + VerifyUpdates(require, testUpdates, changes, errchan, tc.expectFallBehind) }) } } -func verifyUpdates( +func VerifyUpdates( require *require.Assertions, testUpdates [][]*core.RelationTupleUpdate, changes <-chan *datastore.RevisionChanges, @@ -263,7 +263,7 @@ func WatchWithTouchTest(t *testing.T, tester DatastoreTester) { tuple.Parse("document:firstdoc#viewer@user:fred[thirdcaveat]"), ) - verifyUpdates(require, [][]*core.RelationTupleUpdate{ + VerifyUpdates(require, [][]*core.RelationTupleUpdate{ { tuple.Touch(tuple.Parse("document:firstdoc#viewer@user:tom")), tuple.Touch(tuple.Parse("document:firstdoc#viewer@user:sarah")), @@ -307,7 +307,7 @@ func WatchWithTouchTest(t *testing.T, tester DatastoreTester) { tuple.Parse("document:firstdoc#viewer@user:fred[thirdcaveat]"), ) - verifyUpdates(require, [][]*core.RelationTupleUpdate{ + VerifyUpdates(require, [][]*core.RelationTupleUpdate{ {tuple.Touch(tuple.Parse("document:firstdoc#viewer@user:tom[somecaveat]"))}, }, changes, @@ -328,7 +328,7 @@ func WatchWithTouchTest(t *testing.T, tester DatastoreTester) { tuple.Parse("document:firstdoc#viewer@user:fred[thirdcaveat]"), ) - verifyUpdates(require, [][]*core.RelationTupleUpdate{ + VerifyUpdates(require, [][]*core.RelationTupleUpdate{ {tuple.Touch(tuple.Parse("document:firstdoc#viewer@user:tom[somecaveat:{\"somecondition\": 42}]"))}, }, changes, @@ -368,7 +368,7 @@ func WatchWithDeleteTest(t *testing.T, tester DatastoreTester) { tuple.Parse("document:firstdoc#viewer@user:fred[thirdcaveat]"), ) - verifyUpdates(require, [][]*core.RelationTupleUpdate{ + VerifyUpdates(require, [][]*core.RelationTupleUpdate{ { tuple.Touch(tuple.Parse("document:firstdoc#viewer@user:tom")), tuple.Touch(tuple.Parse("document:firstdoc#viewer@user:sarah")), @@ -392,7 +392,7 @@ func WatchWithDeleteTest(t *testing.T, tester DatastoreTester) { tuple.Parse("document:firstdoc#viewer@user:fred[thirdcaveat]"), ) - verifyUpdates(require, [][]*core.RelationTupleUpdate{ + VerifyUpdates(require, [][]*core.RelationTupleUpdate{ {tuple.Delete(tuple.Parse("document:firstdoc#viewer@user:tom"))}, }, changes,