Re: [Django] #36083: LiveServerTestCase fails in parallel test runner if django.contrib.auth.backends has not yet been imported (was: TransactionTestCase available_apps not set correctly for parallel test runner)

[Django]

#36083: LiveServerTestCase fails in parallel test runner if
django.contrib.auth.backends has not yet been imported
-------------------------------------+-------------------------------------
Reporter: Adam Zapletal | Owner: (none)
Type: Bug | Status: new
Component: Testing framework | Version: dev
Severity: Release blocker | Resolution:
Keywords: TransactionTestCase | Triage Stage: Accepted
setupclass available_apps |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* summary:
TransactionTestCase available_apps not set correctly for parallel test
runner
=>
LiveServerTestCase fails in parallel test runner if
django.contrib.auth.backends has not yet been imported

Comment:

This was a fun one. Here's what I found:

- The commit I blamed exposed this by lifting the first call to
`set_available_apps()` up into `TransactionTestCase.setUpClass()` (via
`_pre_setup()`), and so the order of operations in `LiveServerTestCase`
became different.
- The reported issue goes away by moving `super().setUpClass()` to the
bottom of `LiveServerTestCase.setUpClass()`.

Why?
- The `check_middleware` system check registered by `django.contrib.auth`
has the side effect of importing `django.contrib.auth.backends`
- The parallel test runner does not run the system checks per-process
- When a parallel test runner worker tries to `setUpClass()` a
`LiveServerTestCase`, it may be importing `django.contrib.auth.backends`
for the first time and thus running `get_user_model()`
- `get_user_model()` will fail (as designed) if `django.contrib.auth` is
not in `available_apps`, which is now the case based on the change in the
order of operations
----
I tested two solutions, both of which work:
1. Running system checks inside each worker thread, to ensure the side
effect described above is executed in each process:
{{{#!diff
diff --git a/django/test/runner.py b/django/test/runner.py
index b83cd37343..360bba4fd7 100644
--- a/django/test/runner.py
+++ b/django/test/runner.py
@@ -419,6 +419,7 @@ def _init_worker(
process_setup_args=None,
debug_mode=None,
used_aliases=None,
+ verbosity=1,
):
"""
Switch to databases dedicated to this worker.
@@ -442,6 +443,7 @@ def _init_worker(
process_setup(*process_setup_args)
django.setup()
setup_test_environment(debug=debug_mode)
+ call_command("check", verbosity=verbosity,
databases=used_aliases)

db_aliases = used_aliases if used_aliases is not None else
connections
for alias in db_aliases:
@@ -496,7 +498,13 @@ class ParallelTestSuite(unittest.TestSuite):
runner_class = RemoteTestRunner

def __init__(
- self, subsuites, processes, failfast=False, debug_mode=False,
buffer=False
+ self,
+ subsuites,
+ processes,
+ failfast=False,
+ debug_mode=False,
+ buffer=False,
+ verbosity=1,
):
self.subsuites = subsuites
self.processes = processes
@@ -506,6 +514,7 @@ class ParallelTestSuite(unittest.TestSuite):
self.initial_settings = None
self.serialized_contents = None
self.used_aliases = None
+ self.verbosity = verbosity
super().__init__()

def run(self, result):
@@ -536,6 +545,7 @@ class ParallelTestSuite(unittest.TestSuite):
self.process_setup_args,
self.debug_mode,
self.used_aliases,
+ self.verbosity,
],
)
args = [
@@ -983,6 +993,7 @@ class DiscoverRunner:
self.failfast,
self.debug_mode,
self.buffer,
+ self.verbosity,
)
return suite
}}}
2. Adjusting the order of operations in `LiveServerTestCase.setUpClass()`
to act as before:
{{{#!diff
diff --git a/django/test/testcases.py b/django/test/testcases.py
index 36366bd777..d641bb3c6b 100644
--- a/django/test/testcases.py
+++ b/django/test/testcases.py
@@ -1817,11 +1817,11 @@ class LiveServerTestCase(TransactionTestCase):

@classmethod
def setUpClass(cls):
- super().setUpClass()
cls.enterClassContext(
modify_settings(ALLOWED_HOSTS={"append": cls.allowed_host})
)
cls._start_server_thread()
+ super().setUpClass()

@classmethod
def _start_server_thread(cls):
}}}
Then of course there is:
3. Fiddle with `available_apps` and/or `MIDDLEWARE` so that
`LiveServerTestCase` subclasses always either have contrib.auth in
available_apps or else no middleware that relies on it.
----
Option 1 feels like a workaround for the fact that a certain system check
has a side effect. It's a more invasive diff, and it adds performance
drag.
Option 2 feels like the simplest thing to do for now to preserve prior
behavior, although it's a workaround for the fact that a system check has
a side effect
Option 3 feels more semantically correct but would need some thought
around the design to make it ergonomic (fiddling with this in every
subclass doesn't seem right)

Adam, what do you think? Would you like to prepare a patch with a
regression test?
--
Ticket URL: <https://code.djangoproject.com/ticket/36083#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.