Changes between Version 2 and Version 4 of Ticket #33626


Ignore:
Timestamp:
Apr 8, 2022, 2:48:39 AM (3 years ago)
Author:
Himanshu Balasamanta
Comment:

Replying to Carlton Gibson:

Hi Himanshu. This may be right, yes — I need to have a sit-down and play with it.

Main question: Are you able to put together an example case where the wrong result arrises?

  • I was looking at ​PR #6906 which added the cache clearing.
  • Also noting the For use in tests only... in the _unregister_lookup docstring. So this would show up in a test inter-dependency...? 🤔

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #33626 – Description

    v2 v4  
    1 Hi Carlton.
    2 I have opened the PR https://github.com/django/django/pull/15569, and have also modified the test that was supposed to throw error(schema.tests.SchemaTests.test_func_unique_constraint_lookups).
    3 
    4 There is no test that checks if the lookup stays in cache after unregistering it. In my PR, I have added an assert statement to check it in **custom_lookups.tests.LookupTests.test_lookups_caching** test. Running the test without clearing cache from _unregister_lookup will fail.
    5 
    6 The cache stays between tests, so you are likely to get different results running tests independently and running all tests in a module. (PS: I faced this problem earlier running tests individually and together gave different results.)
     1 In current source code, in the **_unregister_lookup** method,
     2 [https://github.com/django/django/blame/main/django/db/models/query_utils.py#L212],
     3 the cache is not cleared, which should be done, as it is done in
     4 **register_lookup**,
     5 https://github.com/django/django/blame/main/django/db/models/query_utils.py#L202.
     6 Corresponding to this change, minor changes need to be brought in the
     7 **schema.tests.SchemaTests.test_func_unique_constraint_lookups** test.
     8The PR generated is https://github.com/django/django/pull/15569
Back to Top