Opened 9 years ago

Closed 4 years ago

Last modified 4 years ago

#25388 closed New feature (fixed)

Allow disabling of all migrations during tests

Reported by: Markus Holtermann Owned by: Mariusz Felisiak <felisiak.mariusz@…>
Component: Testing framework Version: dev
Severity: Normal Keywords:
Cc: berker.peksag@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
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 (23)

comment:1 by Wiktor, 8 years ago

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

comment:2 by Berker Peksag, 8 years ago

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

comment:3 by Berker Peksag, 8 years ago

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

comment:4 by Markus Holtermann <info@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In 157d7f1f:

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

comment:5 by Tim Graham <timograham@…>, 8 years ago

In 14e6823:

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

comment:6 by Tim Graham, 8 years ago

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 by Tim Graham <timograham@…>, 8 years ago

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 by Tim Graham <timograham@…>, 8 years ago

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 by Tim Graham, 8 years ago

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

comment:10 by Berker Peksag, 8 years ago

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 by Eyad Toma, 7 years ago

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 by Eyad Toma, 7 years ago

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 by Tobias McNulty, 7 years ago

Owner: changed from Eyad Toma to Tobias McNulty

Reviewing at DUTH sprint

comment:14 by Tobias McNulty, 7 years ago

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

comment:15 by Tobias McNulty, 7 years ago

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 by Tobias McNulty, 7 years ago

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?

in reply to:  15 comment:17 by Markus Holtermann, 7 years ago

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 by Berker Peksag, 7 years ago

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 by Tim Graham, 7 years ago

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.

comment:20 by Jaap Roes, 4 years ago

This would be a very welcome addition to Django. In the mean time, we've been adding the following to all our projects test_settings to speed up testing:

class NoMigrations:
    """Disable migrations for all apps"""
    def __getitem__(self, item):
        return None

    def __contains__(self, item):
        return True

MIGRATION_MODULES = NoMigrations()
Last edited 4 years ago by Jaap Roes (previous) (diff)

comment:21 by Jon Dufresne, 4 years ago

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

comment:22 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Owner: set to Mariusz Felisiak <felisiak.mariusz@…>
Resolution: fixed
Status: newclosed

In f5ebdfce:

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

comment:23 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In a69c4d62:

Refs #25388 -- Corrected value of TEST MIGRATE setting in MIGRATION_MODULES docs.

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