Add new options, normalization, and empty result checks in assertQueries().

Review Request #12367 — Created June 14, 2022 and submitted

Information

Djblets
release-3.x

Reviewers

This introduces the following new comparison keys in
TestCase.assertQueries():

  • limit - The value for a SQL LIMIT
  • offset - The value for a SQL OFFSET
  • select_for_update - Whether model.select_for_update(...) was used
  • values_select - Any fields passed to .values() or .value_list()

It also does a few other things to fix and aid in comparisons.

Any values in extra are now normalized. Whitespace is collapsed and
stripped. Unit tests no longer have to be aware of the exact formatting
of the SQL, which helps for more complex statements.

Queries that are generated but not executed (any that would raise a
EmptyResultSet) are now skipped for comparison. An example would be a
query on pk__in=[], which Django knows makes no sense to pass to the
database. This is a way that code can short-circuit and optimize out an
entire query or a particular clause of the query in Django.
assertQueries() now ignores it just as Django does.

Made use of these in other unit tests.

Summary ID
Add new options, normalization, and empty result checks in assertQueries().
This introduces the following new comparison keys in `TestCase.assertQueries()`: * `limit` - The value for a SQL `LIMIT` * `offset` - The value for a SQL `OFFSET` * `select_for_update` - Whether `model.select_for_update(...)` was used * `values_select` - Any fields passed to `.values()` or `.value_list()`. It also does a few other things to fix and aid in comparisons. Any values in `extra` are now normalized. Whitespace is collapsed and stripped. Unit tests no longer have to be aware of the exact formatting of the SQL, which helps for more complex statements. Queries that are generated but not executed (any that would raise a `EmptyResultSet`) are now skipped for comparison. An example would be a query on `pk__in=[]`, which Django knows makes no sense to pass to the database. This is a way that code can short-circuit and optimize out an entire query or a particular clause of the query in Django. `assertQueries()` now ignores it just as Django does.
b43560c84443580a365c1a63e566e7874f9983fc
Description From Last Updated

I'm a little confused on this (particularly the Callers do not need to supply this default), maybe some rewording is …

maubinmaubin
maubin
  1. 
      
  2. djblets/testing/testcases.py (Diff revision 1)
     
     
     
    Show all issues

    I'm a little confused on this (particularly the Callers do not need to supply this default), maybe some rewording is in order. Does this mean Django may use a value of 21 for LIMIT even when this limit is set None?

    1. Basically, behind the scenes, Django will often (but not always) do batching of results, fetching 21 at a time. They define this in an internal constant in the codebase, and the number and the conditions under which it's used is an implementation detail in Django. Because of that, it'd likely do more harm than good to require callers to supply this number. We treat None and 21 both as defaults, if an explicit value isn't provided in the query dictionary.

      Definitely open to revising the wording on this. I can take another shot at it.

  3. 
      
chipx86
david
  1. Ship It!
  2. 
      
maubin
  1. Much clearer now, looks good.

  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-3.x (dcac982)