Skip to content

Remove redundant check from MissingMigrations method #911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 17, 2025
Merged

Remove redundant check from MissingMigrations method #911

merged 1 commit into from
Jun 17, 2025

Conversation

andrew-farries
Copy link
Collaborator

There is no need for this check, as the method will anyway return an empty slice if the schema history is empty.

There is a test for this case here:

pgroll/pkg/roll/missing_test.go

Lines 169 to 185 in 68fc1f6

t.Run("no migrations have been applied to the target database", func(t *testing.T) {
fs := fstest.MapFS{
"01_migration_1.json": &fstest.MapFile{Data: exampleMigJSON(t, "01_migration_1")},
"02_migration_2.json": &fstest.MapFile{Data: exampleMigJSON(t, "02_migration_2")},
}
testutils.WithMigratorAndConnectionToContainer(t, func(m *roll.Roll, _ *sql.DB) {
ctx := context.Background()
// Get missing migrations
migs, err := m.MissingMigrations(ctx, fs)
require.NoError(t, err)
// Assert that no migrations are missing from the local directory
require.Len(t, migs, 0)
})
})

Related to #872

@andrew-farries
Remove check from MissingMigrations
68fc1f6 
There is no need for this check, as the method will anyway return an
empty slice if the schema history is empty.
@github-actions github-actions bot temporarily deployed to Docs Preview June 16, 2025 14:20 Inactive
@github-actions GitHub Actions
Copy link

Merging this branch will not change overall coverage

Impacted Packages Coverage Δ 🤖
github.com/xataio/pgroll/pkg/roll 79.75% (-0.00%) 👎
Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/xataio/pgroll/pkg/roll/missing.go 83.33% (+0.72%) 18 (-5) 15 (-4) 3 (-1) 👍

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

@andrew-farries andrew-farries marked this pull request as ready for review June 16, 2025 14:23
@andrew-farries andrew-farries requested a review from kvch June 16, 2025 14:23
@andrew-farries andrew-farries merged commit 1fe1af8 into main Jun 17, 2025
30 checks passed
@andrew-farries andrew-farries deleted the remove-check-from-missing-migrations branch June 17, 2025 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kvch kvch kvch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andrew-farries @kvch