Opened 8 years ago

Closed 8 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 Tim Graham)

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:

  1. The call to register_model busts the apps and registered models (1k+) _meta cache. I didn't run any benchmark here but I suspect this slow down things a bit.
  2. If a model is already registered for the specified app_label.ModelName (see the schema issue detailed in #25745) a RuntimeWarning is raised.
  3. If no model is registered for this particular app_label.ModelName any future call to migrate 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 Simon Charette, 8 years ago

Owner: changed from nobody to Simon Charette
Status: newassigned

comment:2 by Tim Graham, 8 years ago

Description: modified (diff)
Triage Stage: UnreviewedAccepted

comment:3 by Simon Charette, 8 years ago

Has patch: set

comment:4 by Tim Graham, 8 years ago

Needs documentation: set

comment:5 by Simon Charette, 8 years ago

Needs documentation: unset

Added docs and rebased on top of master.

comment:6 by Tim Graham, 8 years ago

Triage Stage: AcceptedReady for checkin

comment:7 by Simon Charette <charette.s@…>, 8 years ago

In 7bb373e3:

Refs #25746 -- Added a test utility to isolate inlined model registration.

Thanks to Tim for the review.

comment:8 by Simon Charette <charette.s@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In a08fda21:

Fixed #25746 -- Isolated inlined test models registration.

Thanks to Tim for the review.

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