Opened 6 years ago

Last modified 2 weeks ago

#11505 assigned New feature

Django's TestCase should reset the cache

Reported by: andrewfong Owned by: andrewfong
Component: Testing framework Version: master
Severity: Normal Keywords: cache testing flush
Cc: kmike, jim.dalton@…, hv@…, someuniquename@…, hirokiky@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In between test cases, the cache should be flushed in order to prevent one test case from polluting another, similar to how the database is currently flushed (or transactions rolled back) in Django's test case.

This will be easier to implement if #11503 is complete

Attachments (4)

patch.diff (1.9 KB) - added by andrewfong 6 years ago.
Adds cache flushing after every testcase + a simple regression test
doc_patch.patch (722 bytes) - added by andrewfong 5 years ago.
Documentation of cache flushing
restore_cache_during_tests.diff (14.8 KB) - added by jsdalton 4 years ago.
restore_cache_during_tests.v2.diff (20.6 KB) - added by jsdalton 3 years ago.

Download all attachments as: .zip

Change History (27)

comment:1 Changed 6 years ago by andrewfong

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to andrewfong
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:2 Changed 6 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

Changed 6 years ago by andrewfong

Adds cache flushing after every testcase + a simple regression test

comment:3 Changed 6 years ago by andrewfong

  • Has patch set
  • Needs documentation set

Changed 5 years ago by andrewfong

Documentation of cache flushing

comment:4 Changed 5 years ago by andrewfong

  • Needs documentation unset

comment:5 Changed 4 years ago by kmike

  • Cc kmike added

There is a pattern to run tests on production machine. With this patch e.g. production memcached instance can be flushed after every commit. Databases are different because one DB server can handle several databases and test database can be created during test setup. I think it can't be done for 'test memcached instance'.

Memcached key space can be easily shared between production and test environment using custom cache backend that prepends all key with fixed string (giving us an equivalent of 'test database' inside single memcached instance). But invalidation is nearly impossible.

comment:6 Changed 4 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:7 follow-up: Changed 4 years ago by julien

  • Easy pickings unset
  • UI/UX unset

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

comment:8 Changed 4 years ago by julien

  • Type changed from Bug to New feature

comment:9 Changed 4 years ago by julien

  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set

comment:10 in reply to: ↑ 7 Changed 4 years ago by jsdalton

  • Cc jim.dalton@… added
  • Needs tests unset

Replying to 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.

Changed 4 years ago by jsdalton

comment:11 Changed 3 years ago by jsdalton

  • Patch needs improvement unset

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.

Changed 3 years ago by jsdalton

comment:12 Changed 3 years ago by aaugustin

In [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.

comment:13 Changed 3 years ago by guettli

  • Cc hv@… added

comment:14 Changed 2 years ago by Fak3

  • Cc someuniquename@… added

comment:15 Changed 12 months ago by hirokiky

  • Cc hirokiky@… added

comment:16 Changed 3 months ago by tchaumeny

I ran 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

Last edited 3 months ago by tchaumeny (previous) (diff)

comment:17 Changed 3 months ago by Tim Graham <timograham@…>

In 3bb78c5e7a330ed70c518e77899bf8a5ac6bf766:

Cleanup cache in contrib.sites to prevent test interference -- refs #11505

comment:18 Changed 6 weeks ago by Tim Graham <timograham@…>

In 4135d837027eac43ec416856d9476c478167d8a6:

Isolated a flatpages test; refs #11505.

comment:19 Changed 6 weeks ago by Tim Graham <timograham@…>

In 1806e059f6127e3e4572db09f297984c96ea9d02:

[1.8.x] Isolated a flatpages test; refs #11505.

Backport of 4135d837027eac43ec416856d9476c478167d8a6 from master

comment:20 Changed 3 weeks ago by Tim Graham <timograham@…>

In e0b3926026984dccc86a09c0c4f32e8bec6f3fe1:

Isolated auth_tests from contenttypes_tests; refs #11505.

comment:21 Changed 3 weeks ago by Tim Graham <timograham@…>

In 259259a819a12a9ecfd0448c646899fad78e23e0:

[1.8.x] Isolated auth_tests from contenttypes_tests; refs #11505.

Backport of e0b3926026984dccc86a09c0c4f32e8bec6f3fe1 from master

comment:22 Changed 2 weeks ago by Tim Graham <timograham@…>

In f668bac9d223408627ca92b2281cf7110039510b:

Fixed #24345 -- Isolated sitemaps_tests from contenttypes_tests; refs #11505

comment:23 Changed 2 weeks ago by Tim Graham <timograham@…>

In bc2eb6bfef00fd8ea4395e3462366b6571f88601:

[1.8.x] Fixed #24345 -- Isolated sitemaps_tests from contenttypes_tests; refs #11505

Backport of f668bac9d223408627ca92b2281cf7110039510b from master

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