Re: [Django] #11505: Django's TestCase should reset the cache

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: | Owner: andrewfong
andrewfong | Status: assigned
Type: Bug | Component: Testing framework
Milestone: | Severity: Normal
Version: SVN | Keywords: cache testing flush
Resolution: | Has patch: 1
Triage Stage: Accepted | Needs tests: 0
Needs documentation: 0 | Easy pickings: 0
Patch needs improvement: 0 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by julien):

* ui_ux: => 0
* easy: => 0

Comment:

#16401 is a duplicate and contains a much more thorough patch.

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:7>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: | Owner: andrewfong
andrewfong | Status: assigned

Type: New | Component: Testing framework
feature | Severity: Normal
Milestone: | Keywords: cache testing flush
Version: SVN | Has patch: 1
Resolution: | Needs tests: 0
Triage Stage: Accepted | Easy pickings: 0
Needs documentation: 0 |

Patch needs improvement: 0 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by julien):

* type: Bug => New feature

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:8>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: | Owner: andrewfong
andrewfong | Status: assigned
Type: New | Component: Testing framework
feature | Severity: Normal
Milestone: | Keywords: cache testing flush
Version: SVN | Has patch: 1

Resolution: | Needs tests: 1

Triage Stage: Accepted | Easy pickings: 0

Needs documentation: 1 |
Patch needs improvement: 1 |

UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by julien):

* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
* needs_docs: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:9>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: | Owner: andrewfong
andrewfong | Status: assigned
Type: New | Component: Testing framework
feature | Severity: Normal
Milestone: | Keywords: cache testing flush
Version: SVN | Has patch: 1

Resolution: | Needs tests: 0

Triage Stage: Accepted | Easy pickings: 0
Needs documentation: 1 |
Patch needs improvement: 1 |
UI/UX: 0 |
-------------------------------------+-------------------------------------

Changes (by jsdalton):

* cc: jim.dalton@… (added)
* needs_tests: 1 => 0

Comment:

Replying to [comment:7 julien]:

> #16401 is a duplicate and contains a much more thorough patch.

Thanks julien for consolidating the new ticket I opened back to this one.

I'm uploading the latest patch from that ticket here. In a nutshell, this
approach monkey patches cache and get_cache during test setup and keeps
track of any keys used during the test. After each test, those keys are
flushed. We also prepend '_test' to the key prefix to ensure existing
values are not overwritten. The patch is tested and working. I'm leaving
it as "needs improvement" for now mainly because there is one test in the
suite that is failing and I haven't determined why yet. It's minor
whatever is is though.

In sum, the patch works great and solves the problem. There was a bit of
discussion here about the approach, however:
https://groups.google.com/forum/#!topic/django-developers/zlaPsP13dUY .
Jacob stated he wasn't necessarily a fan of monkey patching and suggested
an alternative. I had some thoughts which I posted in response as well.

If anyone reading this comment is interested in help push this ticket
toward resolution, a great way would be to review this patch and then
weigh in on that django dev discussion. If we can reach a consensus on
django-dev I'd be happy to put in some time to implement whatever approach
is agreed upon.

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:10>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------

Reporter: andrewfong | Owner:
Type: New feature | andrewfong
Component: Testing framework | Status: assigned

Severity: Normal | Version: SVN
Keywords: cache testing flush | Resolution:
Has patch: 1 | Triage Stage: Accepted

Needs tests: 0 | Needs documentation: 1
Easy pickings: 0 | Patch needs improvement: 0

| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by jsdalton):

* needs_better_patch: 1 => 0

Comment:

Adding a new patch for this, which extends the modified cache approach to
cache middleware. I had noticed on some of my projects that certain tests
were failing when using cache middleware, which turned out to be because
the cache was returning the response and thus the context was not included
in the response to the test client. Tests reproducing this error state as
well as a fix are included in the latest patch. The full test suite passes
as well (minus 5 unrelated failures that are on trunk right now).

The failures to me are a good illustration of why this patch is important.
The system should ideally be as clean as possible in between each test;
cache values carrying over from test to test create problematic behavior
when a full test suite is run.

I might post again to the dev list if there's no movement on this. If
anyone on the CC list here would like to review this patch and give any
feedback please do so. At this point, all that's really required from what
I can see is a few notes in the documentation.

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:11>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: andrewfong | Owner:
Type: New feature | andrewfong
Component: Testing framework | Status: assigned
Severity: Normal | Version: SVN
Keywords: cache testing flush | Resolution:
Has patch: 1 | Triage Stage: Accepted
Needs tests: 0 | Needs documentation: 1
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by aaugustin):

In [17042]:
{{{
#!CommitTicketReference repository="" revision="17042"
Fixed several problems that hid one another in the cache tests and code.

1 - Used django.test.TestCase instead of unittest.TestCase so that the
override_settings decorator works.
2 - Reverted parts of r17039 that caused failures (masked until 1).
3 - Isolated tests by clearing the cache in tear down (masked until 1).
Refs #11505.
4 - Fixed a bug in cache key generation (revealed by 3).
5 - Fixed a test that relied on this bug -- hardcoding the generated cache
keys in tests isn't a very good idea anyway.
6 - Uniformized some parts of the cache tests.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:12>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: andrewfong | Owner:
Type: New feature | andrewfong
Component: Testing framework | Status: assigned
Severity: Normal | Version: SVN
Keywords: cache testing flush | Resolution:
Has patch: 1 | Triage Stage: Accepted
Needs tests: 0 | Needs documentation: 1
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by guettli):

* cc: hv@… (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:13>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: andrewfong | Owner:
Type: New feature | andrewfong
Component: Testing framework | Status: assigned

Severity: Normal | Version: master

Keywords: cache testing flush | Resolution:
Has patch: 1 | Triage Stage: Accepted
Needs tests: 0 | Needs documentation: 1
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Fak3):

* cc: someuniquename@… (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:14>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: andrewfong | Owner:
Type: New feature | andrewfong
Component: Testing framework | Status: assigned
Severity: Normal | Version: master
Keywords: cache testing flush | Resolution:
Has patch: 1 | Triage Stage: Accepted
Needs tests: 0 | Needs documentation: 1
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by hirokiky):

* cc: hirokiky@… (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:15>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: andrewfong | Owner:
Type: New feature | andrewfong
Component: Testing framework | Status: assigned
Severity: Normal | Version: master
Keywords: cache testing flush | Resolution:
Has patch: 1 | Triage Stage: Accepted
Needs tests: 0 | Needs documentation: 1
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by tchaumeny):

I run into another example of tests depending on each other execution,
because of the way `django.contrib.sites.models.Site` instances are cached
in a global variable `SITE_CACHE`

Reproductible with the command `python tests/runtests.py
contenttypes_tests.tests.ContentTypesViewsTests.test_shortcut_with_absolute_url
syndication_tests`

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:16>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: andrewfong | Owner:
Type: New feature | andrewfong
Component: Testing framework | Status: assigned
Severity: Normal | Version: master
Keywords: cache testing flush | Resolution:
Has patch: 1 | Triage Stage: Accepted
Needs tests: 0 | Needs documentation: 1
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"3bb78c5e7a330ed70c518e77899bf8a5ac6bf766"]:
{{{
#!CommitTicketReference repository=""
revision="3bb78c5e7a330ed70c518e77899bf8a5ac6bf766"
Cleanup cache in contrib.sites to prevent test interference -- refs #11505
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:17>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: andrewfong | Owner:

| andrewfong
Type: New feature | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: cache testing flush | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"4135d837027eac43ec416856d9476c478167d8a6"]:
{{{
#!CommitTicketReference repository=""
revision="4135d837027eac43ec416856d9476c478167d8a6"
Isolated a flatpages test; refs #11505.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:18>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: andrewfong | Owner:
| andrewfong
Type: New feature | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: cache testing flush | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"1806e059f6127e3e4572db09f297984c96ea9d02"]:
{{{
#!CommitTicketReference repository=""
revision="1806e059f6127e3e4572db09f297984c96ea9d02"
[1.8.x] Isolated a flatpages test; refs #11505.

Backport of 4135d837027eac43ec416856d9476c478167d8a6 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:19>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: andrewfong | Owner:
| andrewfong
Type: New feature | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: cache testing flush | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"e0b3926026984dccc86a09c0c4f32e8bec6f3fe1"]:
{{{
#!CommitTicketReference repository=""
revision="e0b3926026984dccc86a09c0c4f32e8bec6f3fe1"
Isolated auth_tests from contenttypes_tests; refs #11505.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:20>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: andrewfong | Owner:
| andrewfong
Type: New feature | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: cache testing flush | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"259259a819a12a9ecfd0448c646899fad78e23e0"]:
{{{
#!CommitTicketReference repository=""
revision="259259a819a12a9ecfd0448c646899fad78e23e0"
[1.8.x] Isolated auth_tests from contenttypes_tests; refs #11505.

Backport of e0b3926026984dccc86a09c0c4f32e8bec6f3fe1 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:21>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: andrewfong | Owner:
| andrewfong
Type: New feature | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: cache testing flush | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"f668bac9d223408627ca92b2281cf7110039510b"]:
{{{
#!CommitTicketReference repository=""
revision="f668bac9d223408627ca92b2281cf7110039510b"
Fixed #24345 -- Isolated sitemaps_tests from contenttypes_tests; refs
#11505
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:22>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: andrewfong | Owner:
| andrewfong
Type: New feature | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: cache testing flush | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"bc2eb6bfef00fd8ea4395e3462366b6571f88601"]:
{{{
#!CommitTicketReference repository=""
revision="bc2eb6bfef00fd8ea4395e3462366b6571f88601"
[1.8.x] Fixed #24345 -- Isolated sitemaps_tests from contenttypes_tests;
refs #11505

Backport of f668bac9d223408627ca92b2281cf7110039510b from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:23>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+-------------------------------------
Reporter: andrewfong | Owner:
| andrewfong
Type: New feature | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: cache testing flush | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"2a9bcb503f2aa1b7462174cf95852a2aafbe90ec" 2a9bcb5]:
{{{
#!CommitTicketReference repository=""
revision="2a9bcb503f2aa1b7462174cf95852a2aafbe90ec"
Refs #26085, #11505 -- Cleared Site cache in contenttypes_tests.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:24>

[Django]

#11505: Django's TestCase should reset the cache
-------------------------------------+------------------------------------

Reporter: andrewfong | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: dev

Severity: Normal | Resolution:
Keywords: cache testing flush | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------

Comment (by Kayvan Sylvan):

You can do this automatically for all tests by adding a pytest fixture
like this. The operative part is ''autouse=True''


{{{
from django.conf import settings
from django.core.cache import cache

@pytest.fixture(autouse=True)
def reset_cache():
yield
assert (
settings.CACHES['default']['BACKEND']
== 'django.core.cache.backends.locmem.LocMemCache'
)
cache.clear()

}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/11505#comment:25>