• 
      

    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:
    Completed
    Change Summary:
    Pushed to release-3.x (f3abb8f)