Fix a regression in assertQueries and update our dependency.

Review Request #14384 — Created March 20, 2025 and updated

Information

Djblets
release-5.x

Reviewers

assertQueries() was updated to wrap django-assert-queries, but this
ended up being invoked incorrectly, causing the assertion to always
succeed. We were yielding the result and not entering its context.

This change updates the call to simply return the assert_queries()'s
context manager directly. This avoids the issue and avoids wrapping in
another layer of context managers.

We also now depend on django-assert-queries 2.0.1, which, along with the
above, has the added benefit of not including assertQueries() in the
stack trace.

Djblets unit tests pass.

Summary ID
Fix a regression in assertQueries and update our dependency.
`assertQueries()` was updated to wrap `django-assert-queries`, but this ended up being invoked incorrectly, causing the assertion to always succeed. We were yielding the result and not entering its context. This change updates the call to simply return the `assert_queries()`'s context manager directly. This avoids the issue and avoids wrapping in another layer of context managers. We also now depend on django-assert-queries 2.0.1, which, along with the above, has the added benefit of not including `assertQueries()` in the stack trace.
82549b59fd7ca121bee936dc1cc25848e9a81e44
Description From Last Updated

Can we not just do return assert_queries(...)?

daviddavid
There are no open issues
david
  1. 
      
  2. djblets/testing/testcases.py (Diff revision 1)
     
     
     
     
     
     
     
     
    Show all issues

    Can we not just do return assert_queries(...)?

  3. 
      
chipx86
Review request changed
Change Summary:
  • We now return the context manager directly.
  • Updated out dependency to django-assert-queries 2.0.1.
Summary:
Fix a regression in assertQueries.
Fix a regression in assertQueries and update our dependency.
Description:
   

assertQueries() was updated to wrap django-assert-queries, but this

    ended up being invoked incorrectly, causing the assertion to always
    succeed. We were yielding the result and not entering its context.

   
~  

This change updates the call to properly invoke it as a context manager.

  ~

This change updates the call to simply return the assert_queries()'s

  + context manager directly. This avoids the issue and avoids wrapping in
  + another layer of context managers.

  +
  +

We also now depend on django-assert-queries 2.0.1, which, along with the

  + above, has the added benefit of not including assertQueries() in the
  + stack trace.

Commits:
Summary ID
Fix a regression in assertQueries.
`assertQueries()` was updated to wrap `django-assert-queries`, but this ended up being invoked incorrectly, causing the assertion to always succeed. We were yielding the result and not entering its context. This change updates the call to properly invoke it as a context manager.
23d70b7528006ba736637ac8ce3a243c965f3583
Fix a regression in assertQueries and update our dependency.
`assertQueries()` was updated to wrap `django-assert-queries`, but this ended up being invoked incorrectly, causing the assertion to always succeed. We were yielding the result and not entering its context. This change updates the call to simply return the `assert_queries()`'s context manager directly. This avoids the issue and avoids wrapping in another layer of context managers. We also now depend on django-assert-queries 2.0.1, which, along with the above, has the added benefit of not including `assertQueries()` in the stack trace.
82549b59fd7ca121bee936dc1cc25848e9a81e44
Diff:

Revision 2 (+14 -16)

Show changes

djblets/dependencies.py
djblets/testing/testcases.py

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
maubin
  1. Ship It!
  2. 
      
david
  1. Ship It!
  2. 
      
david
  1. Ship It!
  2. 
      
Loading...