Clearing cache between tests (#11505)
I noticed that Aymeric Augustin committed a patch last week [17042] that fixed some issues in the cache backends. Apparently the fact that the cache carries over test to test remains a problem. I've progressed pretty far with an aggressive "fix" for this problem with the patch in #11505. The current patch modifies all the keys set during a test run (so they don't overwrite any existing values), it tracks any values set during each test and it deletes those and only those that were set during that test. In essence, the cache is "restored" to its original state after each test. This is achieved by monkeypatching each cache backend in use and adding some "bookkeeping" code so to speak to track the keys set. The patch includes thorough tests, and as of last week includes a fix for the cache middleware problem as well (where views cached by middleware get carried over test to test so that the view function does not run.). The full Django test suite passes with it. That all said, for the most part reception has been lukewarm for this patch. I think that's pretty understandable, since there's a fair amount of fiddly stuff taking place that makes people uncomfortable. Anyhow, this morning I thought of a vastly simpler alternative proposal, which would fix the cache between tests and ensure we did not step on anyone's toes by unknowingly clearing a cache that the developer did not want to be cleared. As an alternative, I propose we: 1. Add a new optional setting under CACHES, CLEAR_BETWEEN_TESTS. The default for this setting is False. To be clear, the value is set independently for each cache defined in CACHES. 2. Between each test, cycle through each cache in CACHES. If CLEAR_BETWEEN_TESTS is True, then clear() that cache. Otherwise, move on. Since test problems caused by cache values set in the tests are fairly infrequent, the default use case is just to leave this setting alone. However, if your test(s) need it, the option is there. It's also then trivial to e.g. set it for True in your local environment but have it on False in production, if e.g. you wanted to run your tests there but didn't want to clear your entire production cache. I'd love to move #11505 through the logjam here and get it taken care of. Are any core devs willing to buy in to this alternative proposal? If so, I can create a patch for it with relative ease. Part of me still likes my original patch and I think there are good reasons to take the "thorough" approach. But this works as well and I could really go either way. Let me know and thanks for listening. Here are some references for previous conversations that have taken place about this: - https://groups.google.com/d/topic/django-developers/zlaPsP13dUY/discussion - https://code.djangoproject.com/ticket/11505 -- You received this message because you are subscribed to the Google Groups "Django developers" group. To view this discussion on the web visit https://groups.google.com/d/msg/django-developers/-/TTma3dJU1ccJ. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Clearing cache between tests (#11505)
Hi Jim, On 29 oct. 2011, at 19:07, Jim Dalton wrote: > I noticed that Aymeric Augustin committed a patch last week [17042] that > fixed some issues in the cache backends. Apparently the fact that the cache > carries over test to test remains a problem. Yes, while working on the cache tests, I came across #11505. But I just fixed the cache tests themselves, not the general issue. > Anyhow, this morning I thought of a vastly simpler alternative proposal, > which would fix the cache between tests and ensure we did not step on > anyone's toes by unknowingly clearing a cache that the developer did not want > to be cleared. Note that clearing a cache should never be a catastrophic event. > As an alternative, I propose we: > > 1. Add a new optional setting under CACHES, CLEAR_BETWEEN_TESTS. The default > for this setting is False. To be clear, the value is set independently for > each cache defined in CACHES. > > 2. Between each test, cycle through each cache in CACHES. If > CLEAR_BETWEEN_TESTS is True, then clear() that cache. Otherwise, move on. Here's what cache.clear() does on each cache backend: - db: it runs a SQL query "DELETE FROM " - dummy: it's a no-op - filebased: it calls shutil.rmtree on the cache directory - locmem: it clears two Python dicts - memcached: it calls the "flush_all" API method It's dirt cheap with locmem, who is probably the most popular cache backend for testing. It's a little bit expensive with db, but if you're using the database cache backend, you're clearly not a performance junkie. The fact that Django doesn't reset cache state between tests is a bug, which means we don't need to be 100% backwards compatible. I'd be comfortable with calling clear() unconditionally on each cache, after each test. I don't feel strongly against the CLEAR_BETWEEN_TESTS flag, but if you add it, I suggest making it True by default. To sum up: +1 on .clear(), -0 on CLEAR_BETWEEN_TESTS. > Since test problems caused by cache values set in the tests are fairly > infrequent, the default use case is just to leave this setting alone. > However, if your test(s) need it, the option is there. It's also then trivial > to e.g. set it for True in your local environment but have it on False in > production, if e.g. you wanted to run your tests there but didn't want to > clear your entire production cache. Let's not evoke the idea running tests on production machines -- please :) Whoever does that is one typo away from wiping his production database; the cache is the least of his problems… > I'd love to move #11505 through the logjam here and get it taken care of. Are > any core devs willing to buy in to this alternative proposal? Sure, I like it. Best regards, -- Aymeric Augustin. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Clearing cache between tests (#11505)
On Oct 29, 2011, at 11:14 AM, Aymeric Augustin wrote: Thank Aymeric, > >> As an alternative, I propose we: >> >> 1. Add a new optional setting under CACHES, CLEAR_BETWEEN_TESTS. The default >> for this setting is False. To be clear, the value is set independently for >> each cache defined in CACHES. >> >> 2. Between each test, cycle through each cache in CACHES. If >> CLEAR_BETWEEN_TESTS is True, then clear() that cache. Otherwise, move on. > > Here's what cache.clear() does on each cache backend: > - db: it runs a SQL query "DELETE FROM " > - dummy: it's a no-op > - filebased: it calls shutil.rmtree on the cache directory > - locmem: it clears two Python dicts > - memcached: it calls the "flush_all" API method > > It's dirt cheap with locmem, who is probably the most popular cache backend > for testing. It's a little bit expensive with db, but if you're using the > database cache backend, you're clearly not a performance junkie. > > The fact that Django doesn't reset cache state between tests is a bug, which > means we don't need to be 100% backwards compatible. I'd be comfortable with > calling clear() unconditionally on each cache, after each test. I don't feel > strongly against the CLEAR_BETWEEN_TESTS flag, but if you add it, I suggest > making it True by default. > > To sum up: +1 on .clear(), -0 on CLEAR_BETWEEN_TESTS. > I *believe* that when this conversation has been had in the past, the simple solution of cache.clear() has been sort of a nonstarter. Certainly it's by far the simplest solution, and it's tolerable for most of the caches. The main issue is that, using the memcached backend, clear() flushes out the entire cache….i.e. not just the cache for the tests, not just the cache you might have set with this application, but literally the whole of the cache. So the challenge has been to find a way to clear the cache between tests without potentially destructive side effects. If I personally were given the choice between cache.clear() and nothing at all, i'd take cache.clear() and be careful with it. But I'm not sure if the rest of the community is willing to accept such a solution. Hence the CLEAR_BETWEEN_TESTS flag as a compromise. Cheers, Jim -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
