From 8face1aadb90c92668955d2bfb5112d454121468 Mon Sep 17 00:00:00 2001
From: Gianluca Arbezzano <gianarb92@gmail.com>
Date: Fri, 26 Mar 2021 10:53:42 +0100
Subject: [PATCH] Add a test to assert that migrations are ordered as we
 declared them

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.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.

Signed-off-by: Gianluca Arbezzano <gianarb92@gmail.com>
---
 db/migration/migration_test.go | 35 ++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/db/migration/migration_test.go b/db/migration/migration_test.go
index b2b783fdc..dc7b43eb4 100644
--- a/db/migration/migration_test.go
+++ b/db/migration/migration_test.go
@@ -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 {