Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#27124 closed Cleanup/optimization (fixed)

caches_setting_for_tests passes cull related options to memcached tests

Reported by: Ed Morley Owned by: Ed Morley
Component: Core (Cache system) Version: dev
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

The cache tests have a helper named caches_setting_for_tests() (source), that generates the config that is used to override CACHES at various points during the test run.

However it uses _caches_setting_base, which contains cache OPTIONS such as MAX_ENTRIES and CULL_FREQUENCY that are only relevant to the locmem, filesystem and database backends (docs).

This is problematic, since in #20892 we're going to start passing the contents of OPTIONS verbatim to the memcache client constructors, which causes the tests to fail like so:

ERROR: test_memcached_uses_highest_pickle_version (cache.tests.MemcachedCacheTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vagrant/src/_todo/django/tests/cache/tests.py", line 1198, in test_memcached_uses_highest_pickle_version
    self.assertEqual(caches[cache_key]._cache.pickleProtocol,
  File "/home/vagrant/src/_todo/django/django/core/cache/backends/memcached.py", line 169, in _cache
    self._client = self._lib.Client(self._servers, **client_kwargs)
TypeError: __init__() got an unexpected keyword argument 'MAX_ENTRIES'

As such, the cache tests need to be adjusted to only pass those options to the backends that support them.

One way of doing this might be for caches_setting_for_tests() to take an additional include_cull_settings bool parameter, which would determine whether the problematic cull and zero_cull cache keys (source) were included. This parameter would default to True, but then be set to False in MemcachedCacheTests.

Thoughts?

Many thanks :-)

Change History (4)

comment:1 Changed 6 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:2 Changed 6 years ago by Ed Morley

Has patch: set
Owner: changed from nobody to Ed Morley
Status: newassigned

PR opened. I decided to go with a new argument named exclude instead, which takes a tuple of test cache key names that will be excluded from the generated cache settings.

comment:3 Changed 6 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 606a303:

Fixed #27124 -- Excluded cull-related cache configs from memcached tests.

Since the cull and zero_cull test cache configs set MAX_ENTRIES
and CULL_FREQUENCY in OPTIONS, which are only intended for use with
the locmem, filesystem, and database backends. This prevents test
failures once refs #20892 is fixed.

comment:4 Changed 6 years ago by Tim Graham <timograham@…>

In fb8eea56:

[1.10.x] Fixed #27124 -- Excluded cull-related cache configs from memcached tests.

Since the cull and zero_cull test cache configs set MAX_ENTRIES
and CULL_FREQUENCY in OPTIONS, which are only intended for use with
the locmem, filesystem, and database backends. This prevents test
failures once refs #20892 is fixed.

Backport of 606a303856afee684563f9349c2a55578854f1ba from master

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