Opened 9 years ago
Closed 9 years ago
#25746 closed Cleanup/optimization (fixed)
Isolate inlined test models
Reported by: | Simon Charette | Owned by: | Simon Charette |
---|---|---|---|
Component: | Testing framework | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Many tests in Django's own test suite inline model definition in order to prevent table creation or simply isolate them.
Unfortunately most of these model don't explicitly declare an explicit apps
and end up being registered to the global apps
instance. This has three downsides:
- The call to
register_model
busts theapps
and registered models (1k+)_meta
cache. I didn't run any benchmark here but I suspect this slow down things a bit. - If a model is already registered for the specified
app_label.ModelName
(see theschema
issue detailed in #25745) aRuntimeWarning
is raised. - If no model is registered for this particular
app_label.ModelName
any future call tomigrate
will end up creating the table since we don't have explicit migration defined for the test apps.
To prevent the enumerated issues the inlined models should use a per-test isolated registry. Examples of such a pattern can be found on this PR.
e.g.
def test_foo(self): test_apps = Apps(['test_app']) class Foo(models.Model): class Meta: apps = test_apps
In order to reduce the boiler plate required to achieve isolation I suggest introducing a test decorator/context manager that register models declared in it's body in an isolated Apps()
instance.
e.g.
@isolate_apps('test_app', 'test_app_2') def test_foo(self): class Foo(models.Model): pass with isolate_apps('test_app3') as apps: class Bar(models.Model): class Meta: app_label = 'test_app3' assert Bar is apps.get_model('test_app3', 'Bar')
This would also make fixing the actual and future isolation problems quite easy by simply decorating the faulty test.
Change History (8)
comment:1 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:2 by , 9 years ago
Description: | modified (diff) |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 9 years ago
Has patch: | set |
---|
comment:4 by , 9 years ago
Needs documentation: | set |
---|
comment:6 by , 9 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
PR