[Django] #25388: Allow disabling of all migrations during tests
Django
---------------------------------------------+------------------------
Reporter: MarkusH | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: master
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+------------------------
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`.
--
Ticket URL: <https://code.djangoproject.com/ticket/25388>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: MarkusH | Owner: viciu
Type: New feature | Status: assigned
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by viciu):
* status: new => assigned
* owner: nobody => viciu
* has_patch: 0 => 1
* needs_tests: 0 => 1
* needs_docs: 0 => 1
Comment:
https://github.com/django/django/pull/5601
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:1>
Django
Reporter: MarkusH | Owner: viciu
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by berkerpeksag):
* cc: berker.peksag@… (added)
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:2>
Django
Reporter: MarkusH | Owner: viciu
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by berkerpeksag):
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
* needs_docs: 1 => 0
Comment:
Updated PR: https://github.com/django/django/pull/6323
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:3>
Django
Reporter: MarkusH | Owner: viciu
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markus Holtermann <info@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"157d7f1f1de4705daddebb77f21bd7097a74513d" 157d7f1f]:
{{{
#!CommitTicketReference repository=""
revision="157d7f1f1de4705daddebb77f21bd7097a74513d"
Fixed #25388 -- Added an option to allow disabling of migrations during
test database creation
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:4>
Django
Reporter: MarkusH | Owner: viciu
Type: New feature | Status: closed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"14e6823d090a077d27f9f7caff2d505db31e55a7" 14e6823]:
{{{
#!CommitTicketReference repository=""
revision="14e6823d090a077d27f9f7caff2d505db31e55a7"
Refs #25388 -- Used in-memory database in test_disable_migrations.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:5>
Django
Reporter: MarkusH | Owner: viciu
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: closed => new
* has_patch: 1 => 0
* version: master => 1.10
* resolution: fixed =>
* severity: Normal => Release blocker
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:6>
Django
Reporter: MarkusH | Owner: viciu
Component: Testing framework | Version: 1.10
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"944e66cb1db6614ef0644b9030dd1d75e950767c" 944e66cb]:
{{{
#!CommitTicketReference repository=""
revision="944e66cb1db6614ef0644b9030dd1d75e950767c"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:7>
Django
Reporter: MarkusH | Owner: viciu
Type: New feature | Status: closed
Component: Testing framework | Version: 1.10
Severity: Release blocker | Resolution: fixed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"5c56ce7a3f7568745225b190c5fce6b3442d5cdb" 5c56ce7]:
{{{
#!CommitTicketReference repository=""
revision="5c56ce7a3f7568745225b190c5fce6b3442d5cdb"
[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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:8>
Django
Reporter: MarkusH | Owner: viciu
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: closed => new
* version: 1.10 => master
* resolution: fixed =>
* severity: Release blocker => Normal
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:9>
Django
Reporter: MarkusH | Owner: viciu
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by berkerpeksag):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:10>
Django
Reporter: Markus Holtermann | Owner: Eyad Toma
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Eyad Toma):
* owner: Wiktor => Eyad Toma
* status: new => assigned
Comment:
Assigned to myself due to inactivity for 2 months.
Working on top of Berker's solution.
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:11>
Django
Reporter: Markus Holtermann | Owner: Eyad Toma
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Eyad Toma):
* has_patch: 0 => 1
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:12>
Django
Reporter: Markus Holtermann | Owner: Tobias
| McNulty
Type: New feature | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tobias McNulty):
* owner: Eyad Toma => Tobias McNulty
Comment:
Reviewing at DUTH sprint
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:13>
Django
Reporter: Markus Holtermann | Owner: (none)
Type: New feature | Status: new
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Changes (by Tobias McNulty):
* owner: Tobias McNulty => (none)
* needs_better_patch: 0 => 1
* status: assigned => new
* needs_tests: 0 => 1
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:14>
Django
Reporter: Markus Holtermann | Owner: (none)
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (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).
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:15>
Django
Reporter: Markus Holtermann | Owner: (none)
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (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?
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:16>
Django
Reporter: Markus Holtermann | Owner: (none)
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (by Markus Holtermann):
Replying to [comment:15 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:17>
Django
Reporter: Markus Holtermann | Owner: (none)
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (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 :)
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:18>
Django
Reporter: Markus Holtermann | Owner: (none)
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:19>
Django
Reporter: Markus Holtermann | Owner: (none)
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Comment (by Jaap Roes):
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 DisableMigrations:
"""Disable migrations for all apps"""
def __getitem__(self, item):
return None
def __contains__(self, item):
return True
MIGRATION_MODULES = NoMigrations()
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:20>
Django
Reporter: Markus Holtermann | Owner: (none)
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Dufresne):
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
* needs_docs: 1 => 0
Comment:
I took a new approach in PR https://github.com/django/django/pull/12062
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:21>
Django
Reporter: Markus Holtermann | Owner: Mariusz
| Felisiak <felisiak.mariusz@…>
Type: New feature | Status: closed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: new => closed
* owner: (none) => Mariusz Felisiak <felisiak.mariusz@…>
* resolution: => fixed
Comment:
In [changeset:"f5ebdfce5c417f9844e86bccc2f12577064d4bad" f5ebdfce]:
{{{
#!CommitTicketReference repository=""
revision="f5ebdfce5c417f9844e86bccc2f12577064d4bad"
Fixed #25388 -- Added an option to allow disabling of migrations during
test database creation.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:22>
Django
Reporter: Markus Holtermann | Owner: Mariusz
| Felisiak <felisiak.mariusz@…>
Type: New feature | Status: closed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"a69c4d626afe9e56131aeb624caf3137ddd98bcb" a69c4d62]:
{{{
#!CommitTicketReference repository=""
revision="a69c4d626afe9e56131aeb624caf3137ddd98bcb"
Refs #25388 -- Corrected value of TEST MIGRATE setting in
MIGRATION_MODULES docs.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25388#comment:23>