Handle foreign keys and indices properly in SQLite and add e2e tests

This patch resolves issues with `change_column` and `drop_column` which previously caused duplicate or missing indices and foreign keys. To prevent future regressions and help debugging,
a new e2e-test pipeline has been added to the project and the CI. Documentation for running the tests has been added to the README.

End-to-end tests currently support MySQL, PostgreSQL, and SQLite and do not yet cover other SQL dialects.

During review, please help ensure that the SQL fixtures are not missing something :)

Closes gobuffalo#92

.github/workflows/tests.yml

@@ -51,6 +51,18 @@ jobs:
           MYSQL_PORT: 3307
         run: |
           go test -tags sqlite -race ./...
+      - name: Reset soda
+        env:
+          SODA_DIALECT: "sqlite"
+        run: |
+          $HOME/go/bin/soda drop -e $SODA_DIALECT -p ./testdata/migrations
+          $HOME/go/bin/soda create -e $SODA_DIALECT -p ./testdata/migrations
+        shell: bash
+      - name: Run e2e
+        env:
+          SODA_DIALECT: "sqlite"
+        run: |
+          go test -tags sqlite,e2e -race ./internal/e2e/...
 
   pg-tests:
     name: PostgreSQL tests
@@ -102,6 +114,18 @@ jobs:
           POSTGRESQL_URL: "postgres://postgres:postgres@${{job.services.postgres.host}}:${{ job.services.postgres.ports[5432] }}/pop_test?sslmode=disable"
         run: |
           go test -tags sqlite -race ./...
+      - name: Reset soda
+        env:
+          SODA_DIALECT: "sqlite"
+        run: |
+          $HOME/go/bin/soda drop -e $SODA_DIALECT -p ./testdata/migrations
+          $HOME/go/bin/soda create -e $SODA_DIALECT -p ./testdata/migrations
+        shell: bash
+      - name: Run e2e
+        env:
+          SODA_DIALECT: "sqlite"
+        run: |
+          go test -tags sqlite,e2e -race ./internal/e2e/...
 
   crdb-tests:
     name: Cockroach tests
@@ -147,6 +171,18 @@ jobs:
           SODA_DIALECT: "cockroach"
         run: |
           go test -tags sqlite -race ./...
+      - name: Reset soda
+        env:
+          SODA_DIALECT: "sqlite"
+        run: |
+          $HOME/go/bin/soda drop -e $SODA_DIALECT -p ./testdata/migrations
+          $HOME/go/bin/soda create -e $SODA_DIALECT -p ./testdata/migrations
+        shell: bash
+      - name: Run e2e
+        env:
+          SODA_DIALECT: "sqlite"
+        run: |
+          go test -tags sqlite,e2e -race ./internal/e2e/...
 
   sqlite-tests:
     name: SQLite tests
@@ -188,3 +224,15 @@ jobs:
           SODA_DIALECT: "sqlite"
         run: |
           go test -tags sqlite -race ./...
+      - name: Reset soda
+        env:
+          SODA_DIALECT: "sqlite"
+        run: |
+          $HOME/go/bin/soda drop -e $SODA_DIALECT -p ./testdata/migrations
+          $HOME/go/bin/soda create -e $SODA_DIALECT -p ./testdata/migrations
+        shell: bash
+      - name: Run e2e
+        env:
+          SODA_DIALECT: "sqlite"
+        run: |
+          go test -tags sqlite,e2e -race ./internal/e2e/...

README.md

@@ -197,3 +197,20 @@ Sometimes during a migration you need to shell out to an external command.
 ```javascript
 exec("echo hello")
 ```
+
+## Development
+
+### Testing
+
+To run end-to-end tests, use
+
+```
+make test
+```
+
+If you made changes to the end-to-end tests and want to update the fixtures,
+run:
+
+```
+REFRESH_FIXTURES=true make test
+```

database.yml

@@ -32,4 +32,6 @@ sqlserver:
 sqlite:
   dialect: "sqlite3"
   database: "./sql_scripts/sqlite/test.sqlite"
+  options:
+    mode: rwc
 

go.mod

@@ -6,10 +6,12 @@ require (
 	github.com/Masterminds/semver/v3 v3.0.3
 	github.com/go-sql-driver/mysql v1.5.0
 	github.com/gobuffalo/plush/v4 v4.0.0
+	github.com/gobuffalo/pop/v5 v5.1.3
+	github.com/jackc/pgconn v1.6.0 // indirect
+	github.com/jackc/pgx/v4 v4.6.0
 	github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
-	github.com/kr/pretty v0.2.0 // indirect
-	github.com/stretchr/testify v1.4.0
-	golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7 // indirect
-	gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
-	gopkg.in/yaml.v2 v2.2.8 // indirect
+	github.com/stretchr/testify v1.5.1
+	golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9 // indirect
+	golang.org/x/text v0.3.3 // indirect
+	golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 // indirect
 )

internal/e2e/cockroach_test.go

@@ -0,0 +1,20 @@
+package e2e_test
+
+import (
+	"github.com/gobuffalo/pop/v5"
+	"github.com/stretchr/testify/suite"
+)
+
+type CockroachSuite struct {
+	suite.Suite
+}
+
+func (s *CockroachSuite) Test_Cockroach_MigrationSteps() {
+	r := s.Require()
+
+	c, err := pop.Connect("cockroach")
+	r.NoError(err)
+	r.NoError(retryOpen(c))
+
+	run(&s.Suite, c, runTestData(&s.Suite, c, true))
+}

internal/e2e/fixtures/cockroach/down/0.sql

@@ -0,0 +1,5 @@
+CREATE TABLE schema_migration (
+	version VARCHAR(14) NOT NULL,
+	UNIQUE INDEX schema_migration_version_idx (version ASC),
+	FAMILY "primary" (version, rowid)
+);

internal/e2e/fixtures/cockroach/down/1.sql

@@ -0,0 +1,13 @@
+CREATE TABLE e2e_users (
+	id UUID NOT NULL,
+	created_at TIMESTAMP NOT NULL,
+	updated_at TIMESTAMP NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	FAMILY "primary" (id, created_at, updated_at)
+);
+
+CREATE TABLE schema_migration (
+	version VARCHAR(14) NOT NULL,
+	UNIQUE INDEX schema_migration_version_idx (version ASC),
+	FAMILY "primary" (version, rowid)
+);

internal/e2e/fixtures/cockroach/down/2.sql

@@ -0,0 +1,30 @@
+CREATE TABLE e2e_users (
+	id UUID NOT NULL,
+	created_at TIMESTAMP NOT NULL,
+	updated_at TIMESTAMP NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	FAMILY "primary" (id, created_at, updated_at)
+);
+
+CREATE TABLE e2e_user_notes (
+	id UUID NOT NULL,
+	notes VARCHAR(255) NULL,
+	user_id UUID NOT NULL,
+	title VARCHAR(64) NOT NULL DEFAULT '':::STRING,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	INDEX e2e_user_notes_auto_index_e2e_user_notes_e2e_users_id_fk (user_id ASC),
+	INDEX e2e_user_notes_user_id_idx (user_id ASC),
+	INDEX e2e_user_notes_title_idx (title ASC),
+	FAMILY "primary" (id, notes, user_id, title)
+);
+
+CREATE TABLE schema_migration (
+	version VARCHAR(14) NOT NULL,
+	UNIQUE INDEX schema_migration_version_idx (version ASC),
+	FAMILY "primary" (version, rowid)
+);
+
+ALTER TABLE e2e_user_notes ADD CONSTRAINT e2e_user_notes_e2e_users_id_fk FOREIGN KEY (user_id) REFERENCES e2e_users (id) ON DELETE CASCADE;
+
+-- Validate foreign key constraints. These can fail if there was unvalidated data during the dump.
+ALTER TABLE e2e_user_notes VALIDATE CONSTRAINT e2e_user_notes_e2e_users_id_fk;

internal/e2e/fixtures/cockroach/down/3.sql

@@ -0,0 +1,28 @@
+CREATE TABLE e2e_users (
+	id UUID NOT NULL,
+	created_at TIMESTAMP NOT NULL,
+	updated_at TIMESTAMP NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	FAMILY "primary" (id, created_at, updated_at)
+);
+
+CREATE TABLE e2e_user_notes (
+	id UUID NOT NULL,
+	notes VARCHAR(255) NULL,
+	user_id UUID NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	INDEX e2e_user_notes_auto_index_e2e_user_notes_e2e_users_id_fk (user_id ASC),
+	INDEX e2e_user_notes_user_id_idx (user_id ASC),
+	FAMILY "primary" (id, notes, user_id)
+);
+
+CREATE TABLE schema_migration (
+	version VARCHAR(14) NOT NULL,
+	UNIQUE INDEX schema_migration_version_idx (version ASC),
+	FAMILY "primary" (version, rowid)
+);
+
+ALTER TABLE e2e_user_notes ADD CONSTRAINT e2e_user_notes_e2e_users_id_fk FOREIGN KEY (user_id) REFERENCES e2e_users (id) ON DELETE CASCADE;
+
+-- Validate foreign key constraints. These can fail if there was unvalidated data during the dump.
+ALTER TABLE e2e_user_notes VALIDATE CONSTRAINT e2e_user_notes_e2e_users_id_fk;

internal/e2e/fixtures/cockroach/down/4.sql

@@ -0,0 +1,30 @@
+CREATE TABLE e2e_users (
+	id UUID NOT NULL,
+	created_at TIMESTAMP NOT NULL,
+	updated_at TIMESTAMP NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	FAMILY "primary" (id, created_at, updated_at)
+);
+
+CREATE TABLE e2e_user_notes (
+	id UUID NOT NULL,
+	notes VARCHAR(255) NULL,
+	user_id UUID NOT NULL,
+	slug VARCHAR(64) NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	INDEX e2e_user_notes_auto_index_e2e_user_notes_e2e_users_id_fk (user_id ASC),
+	INDEX e2e_user_notes_user_id_idx (user_id ASC),
+	UNIQUE INDEX e2e_user_notes_slug_idx (slug ASC),
+	FAMILY "primary" (id, notes, user_id, slug)
+);
+
+CREATE TABLE schema_migration (
+	version VARCHAR(14) NOT NULL,
+	UNIQUE INDEX schema_migration_version_idx (version ASC),
+	FAMILY "primary" (version, rowid)
+);
+
+ALTER TABLE e2e_user_notes ADD CONSTRAINT e2e_user_notes_e2e_users_id_fk FOREIGN KEY (user_id) REFERENCES e2e_users (id) ON DELETE CASCADE;
+
+-- Validate foreign key constraints. These can fail if there was unvalidated data during the dump.
+ALTER TABLE e2e_user_notes VALIDATE CONSTRAINT e2e_user_notes_e2e_users_id_fk;

internal/e2e/fixtures/cockroach/down/5.sql

@@ -0,0 +1,30 @@
+CREATE TABLE e2e_users (
+	id UUID NOT NULL,
+	created_at TIMESTAMP NOT NULL,
+	updated_at TIMESTAMP NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	FAMILY "primary" (id, created_at, updated_at)
+);
+
+CREATE TABLE e2e_user_notes (
+	id UUID NOT NULL,
+	notes VARCHAR(255) NULL,
+	user_id UUID NOT NULL,
+	slug VARCHAR(64) NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	INDEX e2e_user_notes_auto_index_e2e_user_notes_e2e_users_id_fk (user_id ASC),
+	INDEX e2e_user_notes_user_id_idx (user_id ASC),
+	UNIQUE INDEX e2e_user_notes_slug_idx (slug ASC),
+	FAMILY "primary" (id, notes, user_id, slug)
+);
+
+CREATE TABLE schema_migration (
+	version VARCHAR(14) NOT NULL,
+	UNIQUE INDEX schema_migration_version_idx (version ASC),
+	FAMILY "primary" (version, rowid)
+);
+
+ALTER TABLE e2e_user_notes ADD CONSTRAINT e2e_user_notes_e2e_users_id_fk FOREIGN KEY (user_id) REFERENCES e2e_users (id) ON DELETE CASCADE;
+
+-- Validate foreign key constraints. These can fail if there was unvalidated data during the dump.
+ALTER TABLE e2e_user_notes VALIDATE CONSTRAINT e2e_user_notes_e2e_users_id_fk;

internal/e2e/fixtures/cockroach/down/6.sql

@@ -0,0 +1,30 @@
+CREATE TABLE e2e_users (
+	id UUID NOT NULL,
+	created_at TIMESTAMP NOT NULL,
+	updated_at TIMESTAMP NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	FAMILY "primary" (id, created_at, updated_at)
+);
+
+CREATE TABLE e2e_user_notes (
+	id UUID NOT NULL,
+	notes VARCHAR(255) NULL,
+	user_id UUID NOT NULL,
+	slug VARCHAR(64) NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	INDEX e2e_user_notes_auto_index_e2e_user_notes_e2e_users_id_fk (user_id ASC),
+	INDEX e2e_user_notes_user_id_idx (user_id ASC),
+	UNIQUE INDEX e2e_user_notes_slug_idx (slug ASC),
+	FAMILY "primary" (id, notes, user_id, slug)
+);
+
+CREATE TABLE schema_migration (
+	version VARCHAR(14) NOT NULL,
+	UNIQUE INDEX schema_migration_version_idx (version ASC),
+	FAMILY "primary" (version, rowid)
+);
+
+ALTER TABLE e2e_user_notes ADD CONSTRAINT e2e_user_notes_e2e_users_id_fk FOREIGN KEY (user_id) REFERENCES e2e_users (id) ON DELETE CASCADE;
+
+-- Validate foreign key constraints. These can fail if there was unvalidated data during the dump.
+ALTER TABLE e2e_user_notes VALIDATE CONSTRAINT e2e_user_notes_e2e_users_id_fk;

internal/e2e/fixtures/cockroach/up/0.sql

@@ -0,0 +1,13 @@
+CREATE TABLE e2e_users (
+	id UUID NOT NULL,
+	created_at TIMESTAMP NOT NULL,
+	updated_at TIMESTAMP NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	FAMILY "primary" (id, created_at, updated_at)
+);
+
+CREATE TABLE schema_migration (
+	version VARCHAR(14) NOT NULL,
+	UNIQUE INDEX schema_migration_version_idx (version ASC),
+	FAMILY "primary" (version, rowid)
+);

internal/e2e/fixtures/cockroach/up/1.sql

@@ -0,0 +1,30 @@
+CREATE TABLE e2e_users (
+	id UUID NOT NULL,
+	created_at TIMESTAMP NOT NULL,
+	updated_at TIMESTAMP NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	FAMILY "primary" (id, created_at, updated_at)
+);
+
+CREATE TABLE e2e_user_notes (
+	id UUID NOT NULL,
+	notes VARCHAR(255) NULL,
+	title VARCHAR(64) NOT NULL DEFAULT '':::STRING,
+	user_id UUID NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	INDEX e2e_user_notes_auto_index_e2e_user_notes_e2e_users_id_fk (user_id ASC),
+	INDEX e2e_user_notes_user_id_idx (user_id ASC),
+	INDEX e2e_user_notes_title_idx (title ASC),
+	FAMILY "primary" (id, notes, title, user_id)
+);
+
+CREATE TABLE schema_migration (
+	version VARCHAR(14) NOT NULL,
+	UNIQUE INDEX schema_migration_version_idx (version ASC),
+	FAMILY "primary" (version, rowid)
+);
+
+ALTER TABLE e2e_user_notes ADD CONSTRAINT e2e_user_notes_e2e_users_id_fk FOREIGN KEY (user_id) REFERENCES e2e_users (id) ON DELETE CASCADE;
+
+-- Validate foreign key constraints. These can fail if there was unvalidated data during the dump.
+ALTER TABLE e2e_user_notes VALIDATE CONSTRAINT e2e_user_notes_e2e_users_id_fk;

internal/e2e/fixtures/cockroach/up/2.sql

@@ -0,0 +1,28 @@
+CREATE TABLE e2e_users (
+	id UUID NOT NULL,
+	created_at TIMESTAMP NOT NULL,
+	updated_at TIMESTAMP NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	FAMILY "primary" (id, created_at, updated_at)
+);
+
+CREATE TABLE e2e_user_notes (
+	id UUID NOT NULL,
+	notes VARCHAR(255) NULL,
+	user_id UUID NOT NULL,
+	CONSTRAINT "primary" PRIMARY KEY (id ASC),
+	INDEX e2e_user_notes_auto_index_e2e_user_notes_e2e_users_id_fk (user_id ASC),
+	INDEX e2e_user_notes_user_id_idx (user_id ASC),
+	FAMILY "primary" (id, notes, user_id)
+);
+
+CREATE TABLE schema_migration (
+	version VARCHAR(14) NOT NULL,
+	UNIQUE INDEX schema_migration_version_idx (version ASC),
+	FAMILY "primary" (version, rowid)
+);
+
+ALTER TABLE e2e_user_notes ADD CONSTRAINT e2e_user_notes_e2e_users_id_fk FOREIGN KEY (user_id) REFERENCES e2e_users (id) ON DELETE CASCADE;
+
+-- Validate foreign key constraints. These can fail if there was unvalidated data during the dump.
+ALTER TABLE e2e_user_notes VALIDATE CONSTRAINT e2e_user_notes_e2e_users_id_fk;