Opened 9 months ago

Closed 9 months ago

#31121 closed Bug (fixed)

SitesFrameworkTests fails when called after ContentTypesViewsSiteRelTests

Reported by: Matthijs Kooijman Owned by: nobody
Component: Testing framework Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When running the test suite, I noticed that I sometimes, got a test failure:

======================================================================
FAIL: test_clear_site_cache (sites_tests.tests.SitesFrameworkTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.7/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/usr/lib/python3.7/unittest/case.py", line 615, in run
    testMethod()
  File "/home/matthijs/docs/src/upstream/django/django/test/utils.py", line 370, in inner
    return func(*args, **kwargs)
  File "/home/matthijs/docs/src/upstream/django/tests/sites_tests/tests.py", line 162, in test_clear_site_cache
    self.assertEqual(models.SITE_CACHE, {})
  File "/usr/lib/python3.7/unittest/case.py", line 839, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/lib/python3.7/unittest/case.py", line 1138, in assertDictEqual
    self.fail(self._formatMessage(msg, standardMsg))
  File "/usr/lib/python3.7/unittest/case.py", line 680, in fail
    raise self.failureException(msg)
AssertionError: {2: <Site: example2.com>, 'example2.com': <Site: example2.com>} != {}
- {2: <Site: example2.com>, 'example2.com': <Site: example2.com>}
+ {}

This was a bit hard to catch, since it only occurred when running the full test suite, with parallelization, using mysql. Running just this single test, or running without parallelization did not show the problem. I suspect that when running without paralellization, or not on mysql, the timings of the tests were subtly different, so the assignment of tests over processes was slightly different, preventing this problem from manifesting.

By adding some debug output, I was able to produce a sequence of two tests, when run without parallelization, produce the same symptom, also with sqlite:

./runtests.py contenttypes_tests.test_views.ContentTypesViewsSiteRelTests.test_shortcut_view_with_site_m2m sites_tests.tests.SitesFrameworkTests.test_clear_site_cache --parallel 1

Looking at the test_clear_site_cache, it starts by asserting that the SITE_CACHE is empty. However, there is not really anything to guarantee that it is. Originally, I thought that the [pre_save handler from contrib.sites](https://github.com/django/django/blob/22ce5d0031bd795ade081394043833e82046016c/django/contrib/sites/models.py#L103-L120) would clear the cache when the [test setup saved a Site object](https://github.com/django/django/blob/22ce5d0031bd795ade081394043833e82046016c/tests/sites_tests/tests.py#L25-L26), but then I realized that this only clears the saved Site from the cache, not everything.

So, it seems the problem is that test_clear_site_cache assumes that the SITE_CACHE is empty at the start of the test, but there is nothing to guarantee this. It does not seem reasonable to expect all tests to clean up the SITE_CACHE after they run (since most tests should not even need to be aware that this cache exists at all), so then I guess test_clear_site_cache (or its parent class) should clear the cache before starting?

I won't be creating a PR for this one, since I really need to get back to actual productive work on my original application...

Change History (3)

comment:1 Changed 9 months ago by Claude Paroz

Triage Stage: UnreviewedAccepted

Thanks, great work for finding the reproducible case!

comment:2 Changed 9 months ago by Claude Paroz

Has patch: set

PR which clears the cache before *and* after tests.

comment:3 Changed 9 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: newclosed

In 5660267e:

Fixed #31121 -- Cleared Site cache in SitesFrameworkTests.

Thanks Matthijs Kooijman for the report and analysis.

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