Opened 2 years ago

Last modified 12 months ago

#25388 new New feature

Allow disabling of all migrations during tests

Reported by: Markus Holtermann Owned by:
Component: Testing framework Version: master
Severity: Normal Keywords:
Cc: berker.peksag@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

As an extension to #24919 a setting DATABASE['TEST']['MIGRATE'] = False should disable all migrations on that particular database. This can be done by hooking into django.db.migrations.loader.MigrationLoader.migrations_module() and returning None.

Change History (19)

comment:1 Changed 2 years ago by Wiktor

Has patch: set
Needs documentation: set
Needs tests: set
Owner: changed from nobody to Wiktor
Status: newassigned

comment:2 Changed 20 months ago by Berker Peksag

Cc: berker.peksag@… added
Patch needs improvement: set

comment:3 Changed 19 months ago by Berker Peksag

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

comment:4 Changed 19 months ago by Markus Holtermann <info@…>

Resolution: fixed
Status: assignedclosed

In 157d7f1f:

Fixed #25388 -- Added an option to allow disabling of migrations during test database creation

comment:5 Changed 19 months ago by Tim Graham <timograham@…>

In 14e6823:

Refs #25388 -- Used in-memory database in test_disable_migrations.

comment:6 Changed 16 months ago by Tim Graham

Has patch: unset
Resolution: fixed
Severity: NormalRelease blocker
Status: closednew
Version: master1.10

As described in #26838, this feature doesn't seem to work as migrations are disabled in more than just tests when setting DATABASES['default']['TEST'] = {'MIGRATE': False}. If this requires more than a trivial fix, let's remove the feature from 1.10.

comment:7 Changed 15 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 944e66cb:

Reverted "Fixed #25388 -- Added an option to allow disabling of migrations during test database creation"

This reverts commit 157d7f1f1de4705daddebb77f21bd7097a74513d since it
disables migrations all the time, not just during tests.

comment:8 Changed 15 months ago by Tim Graham <timograham@…>

In 5c56ce7:

[1.10.x] Reverted "Fixed #25388 -- Added an option to allow disabling of migrations during test database creation"

This reverts commit 157d7f1f1de4705daddebb77f21bd7097a74513d since it
disables migrations all the time, not just during tests.

Backport of 944e66cb1db6614ef0644b9030dd1d75e950767c from master

comment:9 Changed 15 months ago by Tim Graham

Resolution: fixed
Severity: Release blockerNormal
Status: closednew
Version: 1.10master

comment:10 Changed 14 months ago by Berker Peksag

Here's a WIP concept: https://github.com/django/django/compare/master...berkerpeksag:25388-migrate-false-v2 Added an attribute to BaseDatabaseWrapper to be used during the creation of the test database. It's probably not the best way to solve the problem, but it prevents the bug described in #26838.

comment:11 Changed 12 months ago by Eyad Toma

Owner: changed from Wiktor to Eyad Toma
Status: newassigned

Assigned to myself due to inactivity for 2 months.
Working on top of Berker's solution.

comment:12 Changed 12 months ago by Eyad Toma

Has patch: set

Patch submitted https://github.com/django/django/pull/7499

it wasn't possible to write tests because setting "_run_in_test_case" on the connection only works before creating the test DB.
I tried override setting and creating "ConnectionHandler" but failed because of the reason above.

comment:13 Changed 12 months ago by Tobias McNulty

Owner: changed from Eyad Toma to Tobias McNulty

Reviewing at DUTH sprint

comment:14 Changed 12 months ago by Tobias McNulty

Needs documentation: set
Needs tests: set
Owner: Tobias McNulty deleted
Patch needs improvement: set
Status: assignednew

comment:15 Changed 12 months ago by Tobias McNulty

Another option (which I prefer over the current implementation, the more I think about it) would be to add a new option to the migrate management command that would *only* run syncdb (i.e., it would pass an argument into the MigrationLoader telling it to do what you do now in the above PR). This seems less magical than the current implementation as it would facilitate passing the option down through the stack rather than setting it on a global variable (the db connection).

comment:16 Changed 12 months ago by Tobias McNulty

Another question: What about the currently documented method of suppressing migrations is insufficient? I.e., would simply improving that documentation be a suitable alternative to this ticket?

comment:17 in reply to:  15 Changed 12 months ago by Markus Holtermann

Replying to Tobias McNulty:

Another option (which I prefer over the current implementation, the more I think about it) would be to add a new option to the migrate management command that would *only* run syncdb

I don't like that option. syncdb in it's current form will go away in some way or another in the future. migrate doesn't run syncdb automatically anymore unless explicitly said otherwise.

comment:18 Changed 12 months ago by Berker Peksag

Assigned to myself due to inactivity for 2 months.

Thank you for working on this, Eyad. By the way, the inactivity was mainly due to the lack of feedback. I'm not very experienced on this part of Django and I wasn't sure that my solution is a good one. So it would be great if you could get some feedback from the domain experts :)

comment:19 Changed 12 months ago by Tim Graham

My take on the motivation of this ticket is to ease the boilerplate of setting MIGRATIONS_MODULES['app'] = None for all apps in a test settings file. I think it has some value if a nice solution is found.

Note: See TracTickets for help on using tickets.
Back to Top