Merge pull request #1762 from josephschorr/remove-duplicate-test-code

Remove duplicate testing code
authzed · Feb 23, 2024 · 3810d67 · 3810d67
2 parents 056b5ca + 22741b5
commit 3810d67
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 63 deletions.
diff --git 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
@@ -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,