diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index 71eeba3e3d..ad3d15334f 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -100,14 +100,14 @@ jobs: - name: "Integration tests" run: "go run mage.go test:integration" - datastore: - name: "Datastore Tests" + datastoreinttest: + name: "Datastore Integration Tests" runs-on: "buildjet-8vcpu-ubuntu-2204" needs: "paths-filter" strategy: fail-fast: false matrix: - datastore: ["crdb", "mysql", "postgres", "spanner", "pgbouncer"] + datastore: ["crdb", "mysql", "spanner"] steps: - uses: "actions/checkout@v4" if: | @@ -125,11 +125,87 @@ jobs: if: | needs.paths-filter.outputs.codechange == 'true' run: "go run mage.go testds:${{ matrix.datastore }}" + + datastoreconstest: + name: "Datastore Consistency Tests" + runs-on: "buildjet-4vcpu-ubuntu-2204" + needs: "paths-filter" + strategy: + fail-fast: false + matrix: + datastore: ["crdb", "mysql", "spanner"] + steps: + - uses: "actions/checkout@v4" + if: | + needs.paths-filter.outputs.codechange == 'true' + - uses: "authzed/actions/setup-go@main" + if: | + needs.paths-filter.outputs.codechange == 'true' + - uses: "docker/login-action@v3" + if: | + needs.paths-filter.outputs.codechange == 'true' + with: + username: "${{ env.DOCKERHUB_PUBLIC_USER }}" + password: "${{ env.DOCKERHUB_PUBLIC_ACCESS_TOKEN }}" - name: "Consistency tests" if: | needs.paths-filter.outputs.codechange == 'true' run: "go run mage.go testcons:${{ matrix.datastore }}" + pgdatastoreinttest: + name: "Datastore Integration Tests" + runs-on: "buildjet-4vcpu-ubuntu-2204" + needs: "paths-filter" + strategy: + fail-fast: false + matrix: + datastore: ["postgres", "pgbouncer"] + pgversion: ["13.8", "14", "15", "16", "17"] + steps: + - uses: "actions/checkout@v4" + if: | + needs.paths-filter.outputs.codechange == 'true' + - uses: "authzed/actions/setup-go@main" + if: | + needs.paths-filter.outputs.codechange == 'true' + - uses: "docker/login-action@v3" + if: | + needs.paths-filter.outputs.codechange == 'true' + with: + username: "${{ env.DOCKERHUB_PUBLIC_USER }}" + password: "${{ env.DOCKERHUB_PUBLIC_ACCESS_TOKEN }}" + - name: "Integration tests" + if: | + needs.paths-filter.outputs.codechange == 'true' + run: "go run mage.go testds:${{ matrix.datastore }}ver ${{ matrix.pgversion }}" + + pgdatastoreconstest: + name: "Datastore Consistency Tests" + runs-on: "buildjet-4vcpu-ubuntu-2204" + needs: "paths-filter" + strategy: + fail-fast: false + matrix: + datastore: ["postgres"] + pgversion: ["13.8", "14", "15", "16", "17"] + steps: + - uses: "actions/checkout@v4" + if: | + needs.paths-filter.outputs.codechange == 'true' + - uses: "authzed/actions/setup-go@main" + if: | + needs.paths-filter.outputs.codechange == 'true' + - uses: "docker/login-action@v3" + if: | + needs.paths-filter.outputs.codechange == 'true' + with: + username: "${{ env.DOCKERHUB_PUBLIC_USER }}" + password: "${{ env.DOCKERHUB_PUBLIC_ACCESS_TOKEN }}" + - name: "Consistency tests" + if: | + needs.paths-filter.outputs.codechange == 'true' + run: "go run mage.go testcons:postgresver ${{ matrix.pgversion }}" + e2e: name: "E2E" runs-on: "buildjet-8vcpu-ubuntu-2204" diff --git a/internal/datastore/postgres/pgbouncer_test.go b/internal/datastore/postgres/pgbouncer_test.go index 23958a3ee9..423dddd7fa 100644 --- a/internal/datastore/postgres/pgbouncer_test.go +++ b/internal/datastore/postgres/pgbouncer_test.go @@ -4,24 +4,27 @@ package postgres import ( + "os" "testing" - pgversion "github.com/authzed/spicedb/internal/datastore/postgres/version" - - "github.com/samber/lo" + "github.com/authzed/spicedb/internal/datastore/postgres/version" ) -var pgbouncerConfigs = lo.Map( - []string{pgversion.MinimumSupportedPostgresVersion, "14", "15", "16"}, - func(postgresVersion string, _ int) postgresConfig { - return postgresConfig{"head", "", postgresVersion, true} - }, -) +func pgbouncerTestVersion() string { + ver := os.Getenv("POSTGRES_TEST_VERSION") + if ver != "" { + return ver + } + + return version.LatestTestedPostgresVersion +} + +var pgbouncerConfig = postgresTestConfig{"head", "", pgbouncerTestVersion(), true} func TestPostgresWithPgBouncerDatastore(t *testing.T) { - testPostgresDatastore(t, pgbouncerConfigs) + testPostgresDatastore(t, pgbouncerConfig) } func TestPostgresDatastoreWithPgBouncerWithoutCommitTimestamps(t *testing.T) { - testPostgresDatastoreWithoutCommitTimestamps(t, pgbouncerConfigs) + testPostgresDatastoreWithoutCommitTimestamps(t, pgbouncerConfig) } diff --git a/internal/datastore/postgres/postgres_shared_test.go b/internal/datastore/postgres/postgres_shared_test.go index ef84f58e81..489495233d 100644 --- a/internal/datastore/postgres/postgres_shared_test.go +++ b/internal/datastore/postgres/postgres_shared_test.go @@ -16,7 +16,6 @@ import ( sq "github.com/Masterminds/squirrel" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgconn" - "github.com/samber/lo" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/sdk/trace" @@ -49,7 +48,7 @@ func (pgd *pgDatastore) ExampleRetryableError() error { } } -type postgresConfig struct { +type postgresTestConfig struct { targetMigration string migrationPhase string pgVersion string @@ -60,12 +59,6 @@ type postgresConfig struct { var ( otelMutex = sync.Mutex{} testTraceProvider *trace.TracerProvider - postgresConfigs = lo.Map( - []string{pgversion.MinimumSupportedPostgresVersion, "14", "15", "16"}, - func(postgresVersion string, _ int) postgresConfig { - return postgresConfig{"head", "", postgresVersion, false} - }, - ) ) func init() { @@ -75,59 +68,81 @@ func init() { otel.SetTracerProvider(testTraceProvider) } -func testPostgresDatastore(t *testing.T, pc []postgresConfig) { - for _, config := range pc { - pgbouncerStr := "" - if config.pgbouncer { - pgbouncerStr = "pgbouncer-" - } +func testPostgresDatastore(t *testing.T, config postgresTestConfig) { + pgbouncerStr := "" + if config.pgbouncer { + pgbouncerStr = "pgbouncer-" + } + + t.Run(fmt.Sprintf("%spostgres-%s-%s-%s-gc", pgbouncerStr, config.pgVersion, config.targetMigration, config.migrationPhase), func(t *testing.T) { + b := testdatastore.RunPostgresForTesting(t, "", config.targetMigration, config.pgVersion, config.pgbouncer) + ctx := context.Background() + + // NOTE: gc tests take exclusive locks, so they are run under non-parallel. + test.OnlyGCTests(t, test.DatastoreTesterFunc(func(revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) { + ds := b.NewDatastore(t, func(engine, uri string) datastore.Datastore { + ds, err := newPostgresDatastore(ctx, uri, primaryInstanceID, + RevisionQuantization(revisionQuantization), + GCWindow(gcWindow), + GCInterval(gcInterval), + WatchBufferLength(watchBufferLength), + DebugAnalyzeBeforeStatistics(), + MigrationPhase(config.migrationPhase), + ) + require.NoError(t, err) + return ds + }) + return ds, nil + }), false) + }) + + 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() + + test.AllWithExceptions(t, test.DatastoreTesterFunc(func(revisionQuantization, _, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) { + ds := b.NewDatastore(t, func(engine, uri string) datastore.Datastore { + ds, err := newPostgresDatastore(ctx, uri, primaryInstanceID, + RevisionQuantization(revisionQuantization), + GCWindow(gcWindow), + GCInterval(veryLargeGCInterval), + WatchBufferLength(watchBufferLength), + DebugAnalyzeBeforeStatistics(), + MigrationPhase(config.migrationPhase), + ) + require.NoError(t, err) + return ds + }) + return ds, nil + }), test.WithCategories(test.GCCategory), false) + + t.Run("TransactionTimestamps", createDatastoreTest( + b, + TransactionTimestampsTest, + RevisionQuantization(0), + GCWindow(1*time.Millisecond), + GCInterval(veryLargeGCInterval), + WatchBufferLength(1), + MigrationPhase(config.migrationPhase), + )) + + t.Run("QuantizedRevisions", func(t *testing.T) { + QuantizedRevisionTest(t, b) + }) - t.Run(fmt.Sprintf("%spostgres-%s-%s-%s-gc", pgbouncerStr, config.pgVersion, config.targetMigration, config.migrationPhase), func(t *testing.T) { - b := testdatastore.RunPostgresForTesting(t, "", config.targetMigration, config.pgVersion, config.pgbouncer) - ctx := context.Background() - - // NOTE: gc tests take exclusive locks, so they are run under non-parallel. - test.OnlyGCTests(t, test.DatastoreTesterFunc(func(revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) { - ds := b.NewDatastore(t, func(engine, uri string) datastore.Datastore { - ds, err := newPostgresDatastore(ctx, uri, primaryInstanceID, - RevisionQuantization(revisionQuantization), - GCWindow(gcWindow), - GCInterval(gcInterval), - WatchBufferLength(watchBufferLength), - DebugAnalyzeBeforeStatistics(), - MigrationPhase(config.migrationPhase), - ) - require.NoError(t, err) - return ds - }) - return ds, nil - }), false) + t.Run("WatchNotEnabled", func(t *testing.T) { + WatchNotEnabledTest(t, b, config.pgVersion) }) - 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() - - test.AllWithExceptions(t, test.DatastoreTesterFunc(func(revisionQuantization, _, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) { - ds := b.NewDatastore(t, func(engine, uri string) datastore.Datastore { - ds, err := newPostgresDatastore(ctx, uri, primaryInstanceID, - RevisionQuantization(revisionQuantization), - GCWindow(gcWindow), - GCInterval(veryLargeGCInterval), - WatchBufferLength(watchBufferLength), - DebugAnalyzeBeforeStatistics(), - MigrationPhase(config.migrationPhase), - ) - require.NoError(t, err) - return ds - }) - return ds, nil - }), test.WithCategories(test.GCCategory), false) - - t.Run("TransactionTimestamps", createDatastoreTest( + t.Run("GCQueriesServedByExpectedIndexes", func(t *testing.T) { + GCQueriesServedByExpectedIndexes(t, b, config.pgVersion) + }) + + if config.migrationPhase == "" { + t.Run("RevisionInversion", createDatastoreTest( b, - TransactionTimestampsTest, + RevisionInversionTest, RevisionQuantization(0), GCWindow(1*time.Millisecond), GCInterval(veryLargeGCInterval), @@ -135,190 +150,164 @@ func testPostgresDatastore(t *testing.T, pc []postgresConfig) { MigrationPhase(config.migrationPhase), )) - t.Run("QuantizedRevisions", func(t *testing.T) { - QuantizedRevisionTest(t, b) - }) - - t.Run("WatchNotEnabled", func(t *testing.T) { - WatchNotEnabledTest(t, b, config.pgVersion) - }) - - t.Run("GCQueriesServedByExpectedIndexes", func(t *testing.T) { - GCQueriesServedByExpectedIndexes(t, b, config.pgVersion) - }) - - if config.migrationPhase == "" { - t.Run("RevisionInversion", createDatastoreTest( - b, - RevisionInversionTest, - RevisionQuantization(0), - GCWindow(1*time.Millisecond), - GCInterval(veryLargeGCInterval), - WatchBufferLength(1), - MigrationPhase(config.migrationPhase), - )) - - t.Run("ConcurrentRevisionHead", createDatastoreTest( - b, - ConcurrentRevisionHeadTest, - RevisionQuantization(0), - GCWindow(1*time.Millisecond), - GCInterval(veryLargeGCInterval), - WatchBufferLength(1), - MigrationPhase(config.migrationPhase), - )) - - t.Run("ConcurrentRevisionWatch", createDatastoreTest( - b, - ConcurrentRevisionWatchTest, - RevisionQuantization(0), - GCWindow(1*time.Millisecond), - GCInterval(veryLargeGCInterval), - WatchBufferLength(50), - MigrationPhase(config.migrationPhase), - )) + t.Run("ConcurrentRevisionHead", createDatastoreTest( + b, + ConcurrentRevisionHeadTest, + RevisionQuantization(0), + GCWindow(1*time.Millisecond), + GCInterval(veryLargeGCInterval), + WatchBufferLength(1), + MigrationPhase(config.migrationPhase), + )) - t.Run("OverlappingRevisionWatch", createDatastoreTest( - b, - OverlappingRevisionWatchTest, - RevisionQuantization(0), - GCWindow(1*time.Millisecond), - GCInterval(veryLargeGCInterval), - WatchBufferLength(50), - MigrationPhase(config.migrationPhase), - )) + t.Run("ConcurrentRevisionWatch", createDatastoreTest( + b, + ConcurrentRevisionWatchTest, + RevisionQuantization(0), + GCWindow(1*time.Millisecond), + GCInterval(veryLargeGCInterval), + WatchBufferLength(50), + MigrationPhase(config.migrationPhase), + )) - t.Run("RepairTransactionsTest", createDatastoreTest( - b, - RepairTransactionsTest, - RevisionQuantization(0), - GCWindow(1*time.Millisecond), - GCInterval(veryLargeGCInterval), - WatchBufferLength(1), - MigrationPhase(config.migrationPhase), - )) + t.Run("OverlappingRevisionWatch", createDatastoreTest( + b, + OverlappingRevisionWatchTest, + RevisionQuantization(0), + GCWindow(1*time.Millisecond), + GCInterval(veryLargeGCInterval), + WatchBufferLength(50), + MigrationPhase(config.migrationPhase), + )) - t.Run("TestNullCaveatWatch", createDatastoreTest( - b, - NullCaveatWatchTest, - RevisionQuantization(0), - GCWindow(1*time.Millisecond), - GCInterval(veryLargeGCInterval), - WatchBufferLength(50), - MigrationPhase(config.migrationPhase), - )) + t.Run("RepairTransactionsTest", createDatastoreTest( + b, + RepairTransactionsTest, + RevisionQuantization(0), + GCWindow(1*time.Millisecond), + GCInterval(veryLargeGCInterval), + WatchBufferLength(1), + MigrationPhase(config.migrationPhase), + )) - t.Run("TestRevisionTimestampAndTransactionID", createDatastoreTest( - b, - RevisionTimestampAndTransactionIDTest, - RevisionQuantization(0), - GCWindow(1*time.Millisecond), - GCInterval(veryLargeGCInterval), - WatchBufferLength(50), - MigrationPhase(config.migrationPhase), - )) + t.Run("TestNullCaveatWatch", createDatastoreTest( + b, + NullCaveatWatchTest, + RevisionQuantization(0), + GCWindow(1*time.Millisecond), + GCInterval(veryLargeGCInterval), + WatchBufferLength(50), + MigrationPhase(config.migrationPhase), + )) - t.Run("TestCheckpointsOnOutOfBandChanges", createDatastoreTest( - b, - CheckpointsOnOutOfBandChangesTest, - RevisionQuantization(0), - GCWindow(1*time.Millisecond), - GCInterval(veryLargeGCInterval), - WatchBufferLength(50), - MigrationPhase(config.migrationPhase), - )) + t.Run("TestRevisionTimestampAndTransactionID", createDatastoreTest( + b, + RevisionTimestampAndTransactionIDTest, + RevisionQuantization(0), + GCWindow(1*time.Millisecond), + GCInterval(veryLargeGCInterval), + WatchBufferLength(50), + MigrationPhase(config.migrationPhase), + )) - t.Run("TestSerializationError", createDatastoreTest( - b, - SerializationErrorTest, - RevisionQuantization(0), - GCWindow(1*time.Millisecond), - GCInterval(veryLargeGCInterval), - WatchBufferLength(50), - MigrationPhase(config.migrationPhase), - )) + t.Run("TestCheckpointsOnOutOfBandChanges", createDatastoreTest( + b, + CheckpointsOnOutOfBandChangesTest, + RevisionQuantization(0), + GCWindow(1*time.Millisecond), + GCInterval(veryLargeGCInterval), + WatchBufferLength(50), + MigrationPhase(config.migrationPhase), + )) - t.Run("TestStrictReadMode", createReplicaDatastoreTest( - b, - StrictReadModeTest, - RevisionQuantization(0), - GCWindow(1000*time.Second), - GCInterval(veryLargeGCInterval), - WatchBufferLength(50), - MigrationPhase(config.migrationPhase), - )) + t.Run("TestSerializationError", createDatastoreTest( + b, + SerializationErrorTest, + RevisionQuantization(0), + GCWindow(1*time.Millisecond), + GCInterval(veryLargeGCInterval), + WatchBufferLength(50), + MigrationPhase(config.migrationPhase), + )) - t.Run("TestLocking", createMultiDatastoreTest( - b, - LockingTest, - RevisionQuantization(0), - GCWindow(1000*time.Second), - GCInterval(veryLargeGCInterval), - WatchBufferLength(50), - MigrationPhase(config.migrationPhase), - )) - } + t.Run("TestStrictReadMode", createReplicaDatastoreTest( + b, + StrictReadModeTest, + RevisionQuantization(0), + GCWindow(1000*time.Second), + GCInterval(veryLargeGCInterval), + WatchBufferLength(50), + MigrationPhase(config.migrationPhase), + )) - t.Run("OTelTracing", createDatastoreTest( + t.Run("TestLocking", createMultiDatastoreTest( b, - OTelTracingTest, + LockingTest, RevisionQuantization(0), - GCWindow(1*time.Millisecond), + GCWindow(1000*time.Second), GCInterval(veryLargeGCInterval), - WatchBufferLength(1), + WatchBufferLength(50), MigrationPhase(config.migrationPhase), )) - }) - } + } + + t.Run("OTelTracing", createDatastoreTest( + b, + OTelTracingTest, + RevisionQuantization(0), + GCWindow(1*time.Millisecond), + GCInterval(veryLargeGCInterval), + WatchBufferLength(1), + MigrationPhase(config.migrationPhase), + )) + }) } -func testPostgresDatastoreWithoutCommitTimestamps(t *testing.T, pc []postgresConfig) { - for _, config := range pc { - 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) - - // NOTE: watch API requires the commit timestamps, so we skip those tests here. - // NOTE: gc tests take exclusive locks, so they are run under non-parallel. - test.AllWithExceptions(t, test.DatastoreTesterFunc(func(revisionQuantization, _, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) { - ds := b.NewDatastore(t, func(engine, uri string) datastore.Datastore { - ds, err := newPostgresDatastore(ctx, uri, primaryInstanceID, - RevisionQuantization(revisionQuantization), - GCWindow(gcWindow), - GCInterval(veryLargeGCInterval), - WatchBufferLength(watchBufferLength), - DebugAnalyzeBeforeStatistics(), - ) - require.NoError(t, err) - return ds - }) - return ds, nil - }), test.WithCategories(test.WatchCategory, test.GCCategory), false) - }) +func testPostgresDatastoreWithoutCommitTimestamps(t *testing.T, config postgresTestConfig) { + pgVersion := config.pgVersion + enablePgbouncer := config.pgbouncer + t.Run(fmt.Sprintf("postgres-%s", pgVersion), func(t *testing.T) { + t.Parallel() - t.Run(fmt.Sprintf("postgres-%s-gc", pgVersion), func(t *testing.T) { - ctx := context.Background() - b := testdatastore.RunPostgresForTestingWithCommitTimestamps(t, "", "head", false, pgVersion, enablePgbouncer) - test.OnlyGCTests(t, test.DatastoreTesterFunc(func(revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) { - ds := b.NewDatastore(t, func(engine, uri string) datastore.Datastore { - ds, err := newPostgresDatastore(ctx, uri, primaryInstanceID, - RevisionQuantization(revisionQuantization), - GCWindow(gcWindow), - GCInterval(gcInterval), - WatchBufferLength(watchBufferLength), - DebugAnalyzeBeforeStatistics(), - ) - require.NoError(t, err) - return ds - }) - return ds, nil - }), false) - }) - } + ctx := context.Background() + b := testdatastore.RunPostgresForTestingWithCommitTimestamps(t, "", "head", false, pgVersion, enablePgbouncer) + + // NOTE: watch API requires the commit timestamps, so we skip those tests here. + // NOTE: gc tests take exclusive locks, so they are run under non-parallel. + test.AllWithExceptions(t, test.DatastoreTesterFunc(func(revisionQuantization, _, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) { + ds := b.NewDatastore(t, func(engine, uri string) datastore.Datastore { + ds, err := newPostgresDatastore(ctx, uri, primaryInstanceID, + RevisionQuantization(revisionQuantization), + GCWindow(gcWindow), + GCInterval(veryLargeGCInterval), + WatchBufferLength(watchBufferLength), + DebugAnalyzeBeforeStatistics(), + ) + require.NoError(t, err) + return ds + }) + return ds, nil + }), test.WithCategories(test.WatchCategory, test.GCCategory), false) + }) + + t.Run(fmt.Sprintf("postgres-%s-gc", pgVersion), func(t *testing.T) { + ctx := context.Background() + b := testdatastore.RunPostgresForTestingWithCommitTimestamps(t, "", "head", false, pgVersion, enablePgbouncer) + test.OnlyGCTests(t, test.DatastoreTesterFunc(func(revisionQuantization, gcInterval, gcWindow time.Duration, watchBufferLength uint16) (datastore.Datastore, error) { + ds := b.NewDatastore(t, func(engine, uri string) datastore.Datastore { + ds, err := newPostgresDatastore(ctx, uri, primaryInstanceID, + RevisionQuantization(revisionQuantization), + GCWindow(gcWindow), + GCInterval(gcInterval), + WatchBufferLength(watchBufferLength), + DebugAnalyzeBeforeStatistics(), + ) + require.NoError(t, err) + return ds + }) + return ds, nil + }), false) + }) } type datastoreTestFunc func(t *testing.T, ds datastore.Datastore) diff --git a/internal/datastore/postgres/postgres_test.go b/internal/datastore/postgres/postgres_test.go index f1aaf27959..3504068179 100644 --- a/internal/datastore/postgres/postgres_test.go +++ b/internal/datastore/postgres/postgres_test.go @@ -5,9 +5,11 @@ package postgres import ( "fmt" + "os" "testing" "time" + "github.com/authzed/spicedb/internal/datastore/postgres/version" testdatastore "github.com/authzed/spicedb/internal/testserver/datastore" "github.com/jackc/pgx/v5" @@ -15,56 +17,66 @@ import ( "github.com/stretchr/testify/require" ) +func postgresTestVersion() string { + ver := os.Getenv("POSTGRES_TEST_VERSION") + if ver != "" { + return ver + } + + return version.LatestTestedPostgresVersion +} + +var postgresConfig = postgresTestConfig{"head", "", postgresTestVersion(), false} + func TestPostgresDatastore(t *testing.T) { - testPostgresDatastore(t, postgresConfigs) + testPostgresDatastore(t, postgresConfig) } func TestPostgresDatastoreWithoutCommitTimestamps(t *testing.T) { - testPostgresDatastoreWithoutCommitTimestamps(t, postgresConfigs) + testPostgresDatastoreWithoutCommitTimestamps(t, postgresConfig) } func TestPostgresDatastoreGC(t *testing.T) { - for _, config := range postgresConfigs { - 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( - b, - GarbageCollectionTest, - RevisionQuantization(0), - GCWindow(1*time.Millisecond), - GCInterval(veryLargeGCInterval), - WatchBufferLength(1), - MigrationPhase(config.migrationPhase), - )) - - t.Run("GarbageCollectionByTime", createDatastoreTest( - b, - GarbageCollectionByTimeTest, - RevisionQuantization(0), - GCWindow(1*time.Millisecond), - GCInterval(veryLargeGCInterval), - WatchBufferLength(1), - MigrationPhase(config.migrationPhase), - )) - - t.Run("ChunkedGarbageCollection", createDatastoreTest( - b, - ChunkedGarbageCollectionTest, - RevisionQuantization(0), - GCWindow(1*time.Millisecond), - GCInterval(veryLargeGCInterval), - WatchBufferLength(1), - MigrationPhase(config.migrationPhase), - )) - }) + 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( + b, + GarbageCollectionTest, + RevisionQuantization(0), + GCWindow(1*time.Millisecond), + GCInterval(veryLargeGCInterval), + WatchBufferLength(1), + MigrationPhase(config.migrationPhase), + )) + + t.Run("GarbageCollectionByTime", createDatastoreTest( + b, + GarbageCollectionByTimeTest, + RevisionQuantization(0), + GCWindow(1*time.Millisecond), + GCInterval(veryLargeGCInterval), + WatchBufferLength(1), + MigrationPhase(config.migrationPhase), + )) + + t.Run("ChunkedGarbageCollection", createDatastoreTest( + b, + ChunkedGarbageCollectionTest, + RevisionQuantization(0), + GCWindow(1*time.Millisecond), + GCInterval(veryLargeGCInterval), + WatchBufferLength(1), + MigrationPhase(config.migrationPhase), + )) + }) } func TestDefaultQueryExecMode(t *testing.T) { diff --git a/internal/datastore/postgres/version/version.go b/internal/datastore/postgres/version/version.go index 6618769452..3ca8a0930e 100644 --- a/internal/datastore/postgres/version/version.go +++ b/internal/datastore/postgres/version/version.go @@ -4,3 +4,8 @@ package version // // NOTE: must match a tag on DockerHub for the `postgres` image. const MinimumSupportedPostgresVersion = "13.8" + +// LatestTestedPostgresVersion is the latest version of Postgres that has been tested with this driver. +// +// NOTE: must match a tag on DockerHub for the `postgres` image. +const LatestTestedPostgresVersion = "17.2" diff --git a/internal/services/integrationtesting/consistency_datastore_test.go b/internal/services/integrationtesting/consistency_datastore_test.go index 4f49b7e608..d941390a3a 100644 --- a/internal/services/integrationtesting/consistency_datastore_test.go +++ b/internal/services/integrationtesting/consistency_datastore_test.go @@ -44,6 +44,11 @@ func TestConsistencyPerDatastore(t *testing.T) { filePath := filePath t.Run(path.Base(filePath), func(t *testing.T) { + // FIXME errors arise if spanner is run in parallel + if engineID != "spanner" { + t.Parallel() + } + rde := testdatastore.RunDatastoreEngine(t, engineID) ds := rde.NewDatastore(t, config.DatastoreConfigInitFunc(t, dsconfig.WithWatchBufferLength(0), diff --git a/internal/testserver/datastore/datastore.go b/internal/testserver/datastore/datastore.go index 3f9a65cba9..c40066dbdb 100644 --- a/internal/testserver/datastore/datastore.go +++ b/internal/testserver/datastore/datastore.go @@ -4,6 +4,7 @@ package datastore import ( + "os" "testing" "time" @@ -63,7 +64,11 @@ func RunDatastoreEngineWithBridge(t testing.TB, engine string, bridgeNetworkName case "cockroachdb": return RunCRDBForTesting(t, bridgeNetworkName) case "postgres": - return RunPostgresForTesting(t, bridgeNetworkName, migrate.Head, version.MinimumSupportedPostgresVersion, false) + ver := os.Getenv("POSTGRES_TEST_VERSION") + if ver == "" { + ver = version.LatestTestedPostgresVersion + } + return RunPostgresForTesting(t, bridgeNetworkName, migrate.Head, ver, false) case "mysql": return RunMySQLForTesting(t, bridgeNetworkName) case "spanner": diff --git a/internal/testserver/datastore/postgres.go b/internal/testserver/datastore/postgres.go index cbd5dfbbb7..88562539a6 100644 --- a/internal/testserver/datastore/postgres.go +++ b/internal/testserver/datastore/postgres.go @@ -38,10 +38,10 @@ type container struct { type postgresTester struct { container - hostConn *pgx.Conn creds string targetMigration string pgbouncerProxy *container + pool *dockertest.Pool useContainerHostname bool } @@ -98,13 +98,10 @@ func RunPostgresForTestingWithCommitTimestamps(t testing.TB, bridgeNetworkName s creds: POSTGRES_TEST_USER + ":" + POSTGRES_TEST_PASSWORD, targetMigration: targetMigration, useContainerHostname: bridgeSupplied, + pool: pool, } t.Cleanup(func() { - if builder.hostConn != nil { - require.NoError(t, builder.hostConn.Close(context.Background())) - } - require.NoError(t, pool.Purge(postgres)) }) @@ -113,8 +110,6 @@ func RunPostgresForTestingWithCommitTimestamps(t testing.TB, bridgeNetworkName s builder.runPgbouncerForTesting(t, pool, bridgeNetworkName) } - builder.hostConn = builder.initializeHostConnection(t, pool) - return builder } @@ -124,10 +119,14 @@ func (b *postgresTester) NewDatabase(t testing.TB) string { newDBName := "db" + uniquePortion - _, err = b.hostConn.Exec(context.Background(), "CREATE DATABASE "+newDBName) + ctx := context.Background() + conn := b.initializeHostConnection(t) + defer conn.Close(ctx) + + _, err = conn.Exec(ctx, "CREATE DATABASE "+newDBName) require.NoError(t, err) - row := b.hostConn.QueryRow(context.Background(), "SELECT datname FROM pg_catalog.pg_database WHERE datname = $1", newDBName) + row := conn.QueryRow(ctx, "SELECT datname FROM pg_catalog.pg_database WHERE datname = $1", newDBName) var dbName string err = row.Scan(&dbName) require.NoError(t, err) @@ -218,10 +217,10 @@ func (b *postgresTester) runPgbouncerForTesting(t testing.TB, pool *dockertest.P } } -func (b *postgresTester) initializeHostConnection(t testing.TB, pool *dockertest.Pool) (conn *pgx.Conn) { +func (b *postgresTester) initializeHostConnection(t testing.TB) (conn *pgx.Conn) { hostname, port := b.getHostHostnameAndPort() uri := fmt.Sprintf("postgresql://%s@%s:%s/?sslmode=disable", b.creds, hostname, port) - err := pool.Retry(func() error { + err := b.pool.Retry(func() error { var err error ctx, cancelConnect := context.WithTimeout(context.Background(), dockerBootTimeout) defer cancelConnect() diff --git a/magefiles/test.go b/magefiles/test.go index d60cc3675b..fba434b6dd 100644 --- a/magefiles/test.go +++ b/magefiles/test.go @@ -13,6 +13,8 @@ import ( type Test mg.Namespace +var emptyEnv map[string]string + // All Runs all test suites func (t Test) All() error { ds := Testds{} @@ -94,51 +96,80 @@ type Testds mg.Namespace // Crdb Run datastore tests for crdb func (Testds) Crdb() error { - return datastoreTest("crdb") + return datastoreTest("crdb", emptyEnv) } // Spanner Run datastore tests for spanner func (Testds) Spanner() error { - return datastoreTest("spanner") + return datastoreTest("spanner", emptyEnv) } // Postgres Run datastore tests for postgres -func (Testds) Postgres() error { - return datastoreTest("postgres", "postgres") +func (tds Testds) Postgres() error { + return tds.postgres("") +} + +func (tds Testds) PostgresVer(version string) error { + return tds.postgres(version) +} + +func (Testds) postgres(version string) error { + return datastoreTest("postgres", map[string]string{ + "POSTGRES_TEST_VERSION": version, + }, "postgres") } // Pgbouncer Run datastore tests for postgres with Pgbouncer -func (Testds) Pgbouncer() error { - return datastoreTest("postgres", "pgbouncer") +func (tds Testds) Pgbouncer() error { + return tds.pgbouncer("") +} + +func (tds Testds) PgbouncerVer(version string) error { + return tds.pgbouncer(version) +} + +func (Testds) pgbouncer(version string) error { + return datastoreTest("postgres", map[string]string{ + "POSTGRES_TEST_VERSION": version, + }, "pgbouncer") } // Mysql Run datastore tests for mysql func (Testds) Mysql() error { - return datastoreTest("mysql") + return datastoreTest("mysql", emptyEnv) } -func datastoreTest(datastore string, tags ...string) error { +func datastoreTest(datastore string, env map[string]string, tags ...string) error { mergedTags := append([]string{"ci", "docker"}, tags...) tagString := strings.Join(mergedTags, ",") mg.Deps(checkDocker) - return goTest(fmt.Sprintf("./internal/datastore/%s/...", datastore), "-tags", tagString, "-timeout", "10m") + return goDirTestWithEnv(".", fmt.Sprintf("./internal/datastore/%s/...", datastore), env, "-tags", tagString, "-timeout", "10m") } type Testcons mg.Namespace // Crdb Run consistency tests for crdb func (Testcons) Crdb() error { - return consistencyTest("cockroachdb") + return consistencyTest("cockroachdb", emptyEnv) } // Spanner Run consistency tests for spanner func (Testcons) Spanner() error { - return consistencyTest("spanner") + return consistencyTest("spanner", emptyEnv) } -// Postgres Run consistency tests for postgres -func (Testcons) Postgres() error { - return consistencyTest("postgres") +func (tc Testcons) Postgres() error { + return tc.postgres("") +} + +func (tc Testcons) PostgresVer(version string) error { + return tc.postgres(version) +} + +func (Testcons) postgres(version string) error { + return datastoreTest("postgres", map[string]string{ + "POSTGRES_TEST_VERSION": version, + }, "postgres") } // Pgbouncer Run consistency tests for postgres with pgbouncer @@ -148,14 +179,20 @@ func (Testcons) Pgbouncer() error { return nil } +func (Testcons) PgbouncerVer(version string) error { + println("postgres+pgbouncer consistency tests are not implemented") + return nil +} + // Mysql Run consistency tests for mysql func (Testcons) Mysql() error { - return consistencyTest("mysql") + return consistencyTest("mysql", emptyEnv) } -func consistencyTest(datastore string) error { +func consistencyTest(datastore string, env map[string]string) error { mg.Deps(checkDocker) - return goTest("./internal/services/integrationtesting/...", + return goDirTestWithEnv(".", "./internal/services/integrationtesting/...", + env, "-tags", "ci,docker,datastoreconsistency", "-timeout", "10m", "-run", fmt.Sprintf("TestConsistencyPerDatastore/%s", datastore)) diff --git a/magefiles/util.go b/magefiles/util.go index 5824468801..1834decb58 100644 --- a/magefiles/util.go +++ b/magefiles/util.go @@ -25,6 +25,11 @@ func goDirTest(dir string, path string, args ...string) error { return RunSh(goCmdForTests(), WithV(), WithDir(dir), WithArgs(testArgs...))(path) } +func goDirTestWithEnv(dir string, path string, env map[string]string, args ...string) error { + testArgs := append([]string{"test", "-failfast", "-count=1"}, args...) + return RunSh(goCmdForTests(), WithV(), WithDir(dir), WithEnv(env), WithArgs(testArgs...))(path) +} + // check if docker is installed and running func checkDocker() error { if !hasBinary("docker") {