Skip to content

Commit

Permalink
Add a test to assert that migrations are ordered as we declared them (#…
Browse files Browse the repository at this point in the history
…467)

This tests is coming from this PR
#466

We didn't fully understood how the migration library sorts migration
before having to troubleshoot that PR. Even if we declare them in order
as part of the GetMigrations function the library still applies quick
sort on the migration ID to stay concurrency safe

https://github.com/rubenv/sql-migrate/blob/011dc47c6043b25483490739b61cabbc5da7ee9a/migrate.goL216

In order to improve the visbility and to akwnoldge that the way we
append migrations to GetMigrations is the same we get after the sorting
we wrote this tests.

I will start a conversation with the lib maintainer to figure out how we can improve our current workflow.
  • Loading branch information
mergify[bot] authored Mar 26, 2021
2 parents 0f2a86f + 8face1a commit a49e7e6
Showing 1 changed file with 35 additions and 0 deletions.
35 changes: 35 additions & 0 deletions db/migration/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,41 @@ import (
assert "github.com/stretchr/testify/require"
)

// This tests is coming from this PR
// https://github.com/tinkerbell/tink/pull/466
//
// We didn't fully understood how
// the migration library sorts migration before having to troubleshoot
// that PR. Even if we declare them in order as part of the GetMigrations
// function the library still applies quick sort on the migration ID to
// stay concurrency safe
//
// https://github.com/rubenv/sql-migrate/blob/011dc47c6043b25483490739b61cabbc5da7ee9a/migrate.go#L216
//
// In order to improve the visibility and to acknowledge that the way we
// append migrations to GetMigrations is the same we get after the sorting
// we wrote this tests.
// A possible solution is to write our own MigrationSet who won't do the
// ordering. In the meantime I (gianarb) will start a conversation with library
// maintainer to figure out how we can improve this.
func TestMigrationsAreOrderendInTheWayWeDeclaredThem(t *testing.T) {
declaredMigration := GetMigrations().Migrations
orderedMigration, err := GetMigrations().FindMigrations()
if err != nil {
t.Fatal(err)
}
if len(declaredMigration) != len(orderedMigration) {
t.Error("Migrations do not have the same number of elements. This should never happen.")
}

for ii, dm := range declaredMigration {
if dm.Id != orderedMigration[ii].Id {
t.Errorf("Expected migration \"%s\" but got \"%s\"", dm.Id, orderedMigration[ii].Id)
}
}

}

func TestMigrationFuncNamesMatchIDs(t *testing.T) {
timestamps := map[string]bool{}
for _, m := range migrations {
Expand Down

0 comments on commit a49e7e6

Please sign in to comment.