Opened 10 years ago
Last modified 8 months ago
#23746 new New feature
Allow assertNumQueries to clear caches before it runs
Reported by: | Wojtek Ruszczewski | Owned by: | nobody |
---|---|---|---|
Component: | Testing framework | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Tim Graham, Ülgen Sarıkavak | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
The assertNumQueries
manager / method may return different results depending on what is in various caches. Consequently, some tests using it rely on test ordering, particularly those that count some content type queries. For instance if you run the suite in reverse you'll see test_group_permission_performance
failing due to ContentType
being or not being asked about auth.Group
in some preceding test 1.
The proposed solution resets "query" caches on entering the manager (with a switch to disable it for actual caching tests). After introducing the change 10 tests started to fail (ContentType
cache tests and some prefetch_related
tests), 2 specific workarounds were no longer necessary (#17377, #20432) and a failure from PR 3426 disappeared 2.
Note to prospective reviewers: Are there any other caches that should be cleared besides the ContentType
and Site
? Related objects caches maybe? Do the affected prefetch_related
operations actually require content type queries? Would it be better to clear all the caches after every test case to avoid other kinds of dependencies?
[1] What happens in the likewise failing with "--reverse" test_user_permission_performance
is a bit intriguing and probably more involved -- two identical content type queries, savepoint, content type insertion and a release?
[2] Only tested with Python 3.3 / Postgres,
Change History (8)
comment:1 by , 10 years ago
Description: | modified (diff) |
---|
comment:2 by , 10 years ago
Patch needs improvement: | set |
---|---|
Summary: | Remedy assertNumQueries dependence on some caches state → Allow assertNumQueries to clear caches before it runs |
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → New feature |
comment:3 by , 10 years ago
Patch needs improvement: | unset |
---|
A somewhat different solution, with the same effect:
- added a new
pre_capture_queries
signal sent when entering theCaptureQueriesContext
(also for the base manager not onlyassertNumQueries
); - dropped the
clear_caches
toggle as in the new context it seemed too general (which caches?); if you really want to miss some queries, selectively disconnecting their respective receivers is a less convenient, but more fine-grained option.
There are some other possibilities to consider: -- a more general non-testing-only clear_caches
signal, -- some register
function on the test runner, or just bringing back the toggle for convenience.
comment:4 by , 10 years ago
comment:6 by , 10 years ago
I think it would be interesting to see a benchmark of adding a similar signal in SimpleTestCase._pre_setup()
to clear the sites and contenttypes caches before each test. How much would that increase the runtime of the test suite? If we went with that approach, we probably wouldn't need the pre_capture_queries
signal.
comment:7 by , 10 years ago
Cc: | added |
---|---|
Patch needs improvement: | set |
comment:8 by , 8 months ago
Cc: | added |
---|
The idea seems good, but rather than hard-coding a list of functions in
clear_query_caches()
I think it would be better to have some way to register a function to be included in that list. That way third-party code that uses caching can also take advantage of the functionality.