Improve the usage, output, comparison, and logic of assertQueries.

Review Request #13368 — Created Oct. 23, 2023 and submitted

Information

Djblets
release-3.x

Reviewers

assertQueries() has been a useful tool for auditing database queries
and making sure they don't regress, but there have been a few issues in
using it:

  1. In the where clause, empty or deeply-nested query clauses (such as
    Q(Q(Q(a=1))) could be annoying to compare.

  2. If the number of executed queries and the number of expected queries
    did not match, the tool would assert early and fail to show results.

  3. Output was shown with the expected queryset field value on the
    left-hand side of a comparison, and the executed on the right-hand
    side, which is the inverse of how assertions usually show results.

  4. Output was not sorted, which could make it difficult to keep or
    compare to sorted queries.

  5. It wasn't easy to determine where a query actually came from.

  6. There were no type hints, and the code was difficult to maintain.

This change addresses all of that.

Query where clauses are now normalized for easier comparison, with empty
clauses and deep nesting cleaned up. They're now shown over multiple
lines, one clause per line

Mismatches in query counts are reported but do not prevent comparison.
This makes it easy to just run with an empty list and see all the
queries made.

Output of queryset comparisons is now in a standard order, and are
sorted.

New with_tracebacks and tracebacks_size arguments allow the caller
to output traceback information to help track down the origin of a
query.

Type hints are added. These are sort of loose, meant mostly for
documentation purposes. The reason being that, in order to support
both pyright and dynamically-constructed queries, the function has to
accept general dictionary, as pyright won't infer the right-hand side
from the left when concatenating lists.

Until/unless this is resolved, we type hint with the ideal type for
these entries, but accept dictionaries. This means that invalid keys or
values won't be caught, unfortunately, but it does give us
documentaiton.

Internally, much of the code has been cleaned up. A utility function has
been added to perform the comparisons in a type-safe manner and to
record results. It's now easier to reason about this logic and to keep
it updated.

Overall, this change does a lot to improve the usability of query
assertions.

All Djblets and Review Board unit tests pass.

Used this for all the new in-progress query performance optimization
tests for Review Board 5.0.7 / 6.0.1.

Summary ID
Improve the usage, output, comparison, and logic of assertQueries.
`assertQueries()` has been a useful tool for auditing database queries and making sure they don't regress, but there have been a few issues in using it: 1. In the `where` clause, empty or deeply-nested query clauses (such as `Q(Q(Q(a=1)))` could be annoying to compare. 2. If the number of executed queries and the number of expected queries did not match, the tool would assert early and fail to show results. 3. Output was shown with the expected queryset field value on the left-hand side of a comparison, and the executed on the right-hand side, which is the inverse of how assertions usually show results. 4. Output was not sorted, which could make it difficult to keep or compare to sorted queries. 5. It wasn't easy to determine where a query actually came from. 6. There were no type hints, and the code was difficult to maintain. This change addresses all of that. Query where clauses are now normalized for easier comparison, with empty clauses and deep nesting cleaned up. They're now shown over multiple lines, one clause per line Mismatches in query counts are reported but do not prevent comparison. This makes it easy to just run with an empty list and see all the queries made. Output of queryset comparisons is now in a standard order, and are sorted. New `with_tracebacks` and `tracebacks_size` arguments allow the caller to output traceback information to help track down the origin of a query. Type hints are added. These are sort of loose, meant mostly for documentation purposes. The reason being that, in order to support both pyright and dynamically-constructed queries, the function has to accept general dictionary, as pyright won't infer the right-hand side from the left when concatenating lists. Until/unless this is resolved, we type hint with the ideal type for these entries, but accept dictionaries. This means that invalid keys or values won't be caught, unfortunately, but it does give us documentaiton. Internally, much of the code has been cleaned up. A utility function has been added to perform the comparisons in a type-safe manner and to record results. It's now easier to reason about this logic and to keep it updated. Overall, this change does a lot to improve the usability of query assertions.
2d21bd2ea5c21649da249e5a99ba657c306ad3ed
Description From Last Updated

Typo? groupinf -> grouping

maubinmaubin

Missing documentation for the indent arg.

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

    Typo? groupinf -> grouping

    1. Saw this was fixed in /r/13372/

    2. Moving the fix here. Thanks!

  3. djblets/testing/testcases.py (Diff revision 1)
     
     
     
     
    Show all issues

    Missing documentation for the indent arg.

  4. 
      
chipx86
maubin
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.x (f3abb8f)
Loading...