Skip to content

Conversation

Davda-James
Copy link

Trac ticket number

ticket-36639

Branch description

Adds check_test_migrations.py a script that runs makemigrations --check using the test settings.
Adds check-test-migrations.yml to run the script in CI on push and pull requests.

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have checked the "Has patch" ticket flag in the Trac system.

Some more things (if needed will do)

  • The workflow runs makemigrations --check without app labels. If INSTALLED_APPS includes third-party or optional apps, makemigrations could report unrelated changes and cause a bit of noise.
  • Currently uses test_sqlite. If we want to validate migrations under other DBs, have to add a CI matrix with different DJANGO_SETTINGS_MODULE values and DB services (Postgres/MySQL).
  • Needed and asked by maintainer to add integrations test will do.

@github-actions github-actions bot added the no ticket Based on PR title, no linked Trac ticket label Oct 4, 2025
@Davda-James Davda-James changed the title added CI step to run makemigrations --check against test models Resolves #36639: added CI step to run makemigrations --check against test models Oct 4, 2025
@github-actions github-actions bot removed the no ticket Based on PR title, no linked Trac ticket label Oct 4, 2025
@jacobtylerwalls
Copy link
Member

Hi, thanks for getting started here.

If this check is working, I would expect it to be failing on this PR, given the lack of updates to the migration files shown in the ticket. So the check is not working in its current form.

The reason the sketched version in the ticket hooks into TransactionTestCase.setUpClass is so that the tests are discovered already and models registered in the apps registry. You might explore a similar hook, or else a lower-level solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants