Opened 2 years ago

Closed 22 months ago

Last modified 22 months ago

#33422 closed Cleanup/optimization (fixed)

Document @isolate_apps

Reported by: Adam Johnson Owned by: Christopher Adams
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

django.test.utils.isolate_apps is a key tool for keeping Django's test suite fast and maintainable. It can also be useful for testing third party packages and projects. For example, I'm currently writing about a custom system check for model names. The best way I can find to test this check is to use @isolate_apps and create temporary model classes that fail the check.

isolate_apps was added in https://github.com/django/django/commit/7bb373e3097fe8e000e0bba005ff2dcfc18ab9a5 and has remained stable since, minus refactoring to extract TestContextDecorator.

I can't find much use in the wild, indeed the only result I found in the first few pages of a GitHub search is from Sage's JSONField backport package: https://github.com/laymonage/django-jsonfield-backport/blob/2072fb39b6681f2bf8741e033702920b59238941/tests/test_invalid_models.py#L12

I think it's worth documenting and supporting this tool, which can be useful in many situations.

Change History (10)

comment:1 by Mariusz Felisiak, 2 years ago

Personally, I'm not convinced. As far as I'm aware it's stable because we now how and when it works fine. Sometimes it's trickier to use, e.g. in schema tests or when a model with the same name already exists. I'm afraid that docs will quickly grow to the list of caveats.

comment:2 by Adam Johnson, 2 years ago

If there are too many concerns, we could document it similarly to available_apps (https://docs.djangoproject.com/en/stable/topics/testing/advanced/#django.test.TransactionTestCase.available_apps) with a note that it is still regarded as private and liable to change. I think noting some caveats is fine.

Is there a good way to create an isolated model class for a test without isolated_apps? It seems like a common enough problem when making custom model classes, fields, or similar.

comment:3 by Simon Charette, 2 years ago

Is there a good way to create an isolated model class for a test without isolated_apps? It seems like a common enough problem when making custom model classes, fields, or similar.

You can use the undocumented Model.Meta.apps option to point to a dedicated Apps instance, some tests do it when defining models at the module level, but it involves a lot of boilerplate and is easy to forget that you're polluting the main registry hence why isolated_apps is useful.

comment:4 by Carlton Gibson, 2 years ago

I think documenting @isolate_apps is probably a good idea. (One alternative that I've used is a custom test-only app using Simon's setup_test_app idea/proposal on https://code.djangoproject.com/ticket/7835#comment:46 — but…)

I agree we shouldn't document every caveat… Perhaps noting the more complex cases, and then we can come up with patterns as a group on the forum, and maybe address some of the issues as folks hit them 🤔

comment:5 by Mariusz Felisiak, 2 years ago

Triage Stage: UnreviewedAccepted

comment:6 by Christopher Adams, 2 years ago

Owner: changed from nobody to Christopher Adams
Status: newassigned

Cool, happy to take this on.

Looks like there's already some kind of instructions written on how to use this function, with no caveat about it being a private API. In fact, the instruction seems to encourage developers to actively use the function. This is located in the contributing guide however, so I don't know what normative authoritative weight that recommendation has. Here's the source, at "Unit tests > Tips for writing tests > Isolating model registration":
https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/unit-tests/#django.test.utils.isolate_apps

I did just write a PR to include the function spec in the django.test.utils section of the Django documentation, which is where I would have expected it to be. Here's my PR: https://github.com/django/django/pull/15735
@adam: want to take a quick look and see if this is what you are looking for?

For the moment, I did not include a warning about isolate_apps having an unstable API, because the reference in "Tips for writing tests" did not have that warning. Let me know if you want me to add that however.

comment:7 by Mariusz Felisiak, 23 months ago

Has patch: set
Patch needs improvement: set

comment:8 by Mariusz Felisiak, 22 months ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:9 by Mariusz Felisiak <felisiak.mariusz@…>, 22 months ago

Resolution: fixed
Status: assignedclosed

In 90d2f9f4:

Fixed #33422 -- Improved docs about isolating apps.

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 22 months ago

In aed1a73e:

[4.1.x] Fixed #33422 -- Improved docs about isolating apps.

Backport of 90d2f9f41671ef01c8e8e7b5648f95c9bf512aae from main

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