From a469d2ad8c2219ca3e41d0e9ebf642b8c3b71d9d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 17 Jul 2026 10:05:13 +0300 Subject: [PATCH] test(migrations): pin 000086 test to version 86 instead of HEAD TestMigration_RemoveApproveOwnFromStandardUsers failed on PR #808 (000087 marketplace) and PR #1428 (000088 view:config) because: - Sub-test 1 called RunMigrations (to HEAD) instead of pinning to 86; later migrations could alter the same group's permissions. - Sub-test 2 (down migration) called RunMigrations then RollbackMigrations(1), which rolls back the LATEST migration rather than 000086.down. On a branch where HEAD=087, the rollback runs 087.down (marketplace), leaving approve-own absent and failing the assertion. Fix: replace both RunMigrations calls with MigrateToVersion(86) so the test is isolated to exactly the migration it covers, regardless of how many migrations land on downstream branches. This is the same pattern already used correctly in 000088_grant_view_config_to_nonadmin_groups_test.go. Verified: go test -tags=integration -count=3 passes on the fix branch (latest=086), a worktree simulating HEAD=087, and a worktree simulating HEAD=088. Closes the cross-migration interaction blocking #808 and #1428 CI. --- ...move_approve_own_from_standard_users_test.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/internal/database/postgres/migrations/000086_remove_approve_own_from_standard_users_test.go b/internal/database/postgres/migrations/000086_remove_approve_own_from_standard_users_test.go index d198e604e..6947f11c5 100644 --- a/internal/database/postgres/migrations/000086_remove_approve_own_from_standard_users_test.go +++ b/internal/database/postgres/migrations/000086_remove_approve_own_from_standard_users_test.go @@ -59,8 +59,11 @@ func TestMigration_RemoveApproveOwnFromStandardUsers(t *testing.T) { require.True(t, standardUsersHasPurchaseVerb(t, ctx, pool, "retry-own"), "precondition: Standard Users must hold retry-own:purchases at v83") - // Apply the remaining chain, which includes 000086. - require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", "")) + // Pin at exactly 000086 (the migration under test). Using RunMigrations + // (migrate to HEAD) would also run any migrations added on later branches + // (e.g. 000087, 000088) whose permission side-effects could mask a regression + // in 000086 or cause spurious failures here if they alter the same group. + require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 86)) // Four-eyes (issue #1407): approve-own must be gone. assert.False(t, standardUsersHasPurchaseVerb(t, ctx, pool, "approve-own"), @@ -79,10 +82,14 @@ func TestMigration_RemoveApproveOwnFromStandardUsers(t *testing.T) { defer container.Cleanup(ctx) pool := container.DB.Pool() - // Full head has 000086 applied, so approve-own is removed. - require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", "")) + // Pin at exactly 000086 before rolling back. RunMigrations would migrate + // to HEAD (which varies by branch), so RollbackMigrations(1) would roll + // back whatever the LATEST migration is (000087, 000088, ...) rather than + // 000086.down -- leaving approve-own absent and causing a spurious failure. + // Pinning to 86 guarantees RollbackMigrations(1) always exercises 000086.down. + require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 86)) require.False(t, standardUsersHasPurchaseVerb(t, ctx, pool, "approve-own"), - "head state: approve-own must be absent after 000086") + "post-086 state: approve-own must be absent at version 86") // Roll back one step (000086.down) and confirm approve-own is restored. require.NoError(t, migrations.RollbackMigrations(ctx, pool, migrationsPath, 1))