Update integration test suite #713
Conversation
| github.com/mfridman/interpolate v0.0.2 | ||
| github.com/microsoft/go-mssqldb v1.6.0 | ||
| github.com/ory/dockertest/v3 v3.10.0 |
There was a problem hiding this comment.
I want the main goose module to have no dependency on this and any of the indirect deps. I think removing it and having an independent "integration test" module is cleaner.
# Conflicts: # .github/workflows/lint.yaml # Makefile # go.mod # go.sum # internal/testing/integration/testdata/migrations/duckdb/00001_table.sql # internal/testing/integration/testdata/migrations/duckdb/00003_alter.sql # internal/testing/integration/testdata/migrations/duckdb/00004_empty.sql # internal/testing/integration/testdata/migrations/ydb/00002_b.sql # internal/testing/integration/testdata/migrations/ydb/00003_c.sql # internal/testing/integration/testdata/migrations/ydb/00005_e.sql # internal/testing/testdb/duckdb.go # tests/e2e/main_test.go # tests/e2e/migrations_test.go
| timeout-minutes: 10 | ||
| runs-on: ubuntu-latest | ||
| strategy: | ||
| max-parallel: 2 |
There was a problem hiding this comment.
This was a bit of a bottleneck, and I think we should be able to run more tests in parallel.
| var pgErr *pgconn.PgError | ||
| ok := errors.As(err, &pgErr) | ||
| check.Bool(t, ok, true) | ||
| check.Equal(t, pgErr.Code, "42P07") // duplicate_table |
There was a problem hiding this comment.
The entire dialect store can be tested with SQLite. The only reason we had postgres in here was to test for this duplicate table error, confirming the error can be correctly asserted from caller -> driver -> goose -> caller.
| @@ -0,0 +1,87 @@ | |||
| module github.com/pressly/goose/v3/internal/testing | |||
|
|
|||
| go 1.22.1 | |||
There was a problem hiding this comment.
This enables us to keep up-to-date with drivers without having to fight them on the minimum supported Go version.
This will especially be useful if we go down this route with #664
| ) | ||
|
|
||
| func TestGoMigrationByOne(t *testing.T) { | ||
| db, cleanup, err := testdb.NewPostgres() |
There was a problem hiding this comment.
We can test go migrations with SQLite, there is nothing inherently special about using postgres in these tests, since we're asserting for the correct goose library behaviour handling go migrations.
This PR restructures the integration tests (formally named e2e) and moves it into its own Go module.
There are a few things I didn't like about the previous setup:
Instead, we now have a single job which runs all the integration tests in parallel. Yes, we'll reach some upper limit on how many containers can be run at once, but so far it has been consistently running in <2min which is fairly nice. We may even want to consider running the integration tests only on merge to main, with a manual dispatch.
github.com/pressly/goose/v3Go module contained go.mod dependencies not required by the main module. All the drivers, container setup (namelytestdb) and test utils are unnecessary.I know this doesn't have a significant implication on consumer builds, but it's overall cleaner (IMO).
Changes
./internal/testing/integrationtestdbinto this new module, updating dependenciesgithub.com/stretchr/testify, this is so widely used I've kind of given up trying to fight itTestInsertDatabaseNameand then add the corresponding migration files intointernal/testing/integration/testdata/migrations/<insert_database_name>make add-goworkandmake remove-gowork. This allows you to use the local goose library directly. This leverages go module workspacesThe tests run fairly quickly on my M1 machine, taking ~24s. The longest test can be improved, and I have plans for that. Vertica does take longer, but that's fine.