• 
      

    Add testing utilities for building review request Q-expressions.

    Review Request #13414 — Created Nov. 14, 2023 and submitted

    Information

    Review Board
    release-5.0.x

    Reviewers

    This introduces several new functions in
    reviewboard.reviews.testing.queries.review_requests for building
    expected queries for unit tests:

    • get_review_requests_accessible_q()
    • get_review_requests_from_user_q()
    • get_review_requests_to_group_q()
    • get_review_requests_to_user_q()
    • get_review_requests_to_user_directly_q()
    • get_review_requests_to_user_groups_q()
    • get_review_requests_to_or_from_user_q()
    • get_review_requests_accessible_prep_equeries()
    • get_review_requests_accessible_equeries()
    • get_review_requests_accessible_from_user_equeries()
    • get_review_requests_accessible_to_group_equeries()
    • get_review_requests_accessible_to_user_equeries()
    • get_review_requests_accessible_to_user_directly_equeries()
    • get_review_requests_accessible_to_user_groups_equeries()
    • get_review_requests_accessible_to_or_from_user_equeries()

    This covers all the queries used via ReviewRequestManager, offering
    both Q-expressions (for embedding in other queries, needed for the
    datagrids) and standalone queries.

    The logic for building Q-expressions is verbose, checking each
    combination of states individually and returning complete Q-expressions
    as often as possible. Truth tables and comments help document these.
    This is built this way in order to ensure we're testing against
    fully-known expressions, making it harder to introduce a bug in any
    queries.

    Unit tests for ReviewRequestManager have been updated to use these.
    There's still work that should be done to make these tests more
    comprehensive, but between them, upcoming dashboard unit test changes,
    and API tests, we have complete coverage.

    As an implementation note, this is also the first change to make use of
    the new PEP 692 support for defining **kwargs arguments using
    TypedDict, helping with the complexity of the number of arguments
    these functions all take. With this PEP, a value in the TypedDict
    cannot also be present as a standalone argument (to, say, type-narrow
    or override a value), and cannot be passed to a function if also
    present in kwargs, so we have to resort to manipulating kwargs
    directly. This is a reasonable trade-off in this code, but we'll need
    to consider this approach elsewhere on a case-by-case basis.

    All unit tests pass.

    Summary ID
    Add testing utilities for building review request Q-expressions.
    This introduces several new functions in `reviewboard.reviews.testing.queries.review_requests` for building Q-expressions for unit tests: * `get_review_requests_accessible_q()` * `get_review_requests_from_user_q()` * `get_review_requests_to_group_q()` * `get_review_requests_to_user_q()` * `get_review_requests_to_user_directly_q()` * `get_review_requests_to_user_groups_q()` * `get_review_requests_to_or_from_user_q()` * `get_review_requests_accessible_prep_equeries()` * `get_review_requests_accessible_equeries()` * `get_review_requests_accessible_from_user_equeries()` * `get_review_requests_accessible_to_group_equeries()` * `get_review_requests_accessible_to_user_equeries()` * `get_review_requests_accessible_to_user_directly_equeries()` * `get_review_requests_accessible_to_user_groups_equeries()` * `get_review_requests_accessible_to_or_from_user_equeries()` This covers all the queries used via `ReviewRequestManager`, offering both `Q`-expressions (for embedding in other queries, needed for the datagrids) and standalone queries. The logic for building Q-expressions is verbose, checking each combination of states individually and returning complete Q-expressions as often as possible. Truth tables and comments help document these. This is built this way in order to ensure we're testing against fully-known expressions, making it harder to introduce a bug in any queries. Unit tests for `ReviewRequestManager` have been updated to use these. There's still work that should be done to make these tests more comprehensive, but between them, upcoming dashboard unit test changes, and API tests, we have complete coverage.
    cfd9a1e3232cbef3eb39b0a8a9ea03ad01fbe8f2
    Description From Last Updated

    SyntaxError: trailing comma not allowed without surrounding parentheses Column: 5 Error code: E999

    reviewbotreviewbot

    Typo: "use used"

    maubinmaubin

    Instead of raising a ValueError here, maybe it'd be better to just assert isinstance(to_user, str) like you do in the …

    maubinmaubin
    Checks run (1 failed, 1 succeeded)
    flake8 failed.
    JSHint passed.

    flake8

    chipx86
    chipx86
    chipx86
    chipx86
    david
    1. Ship It!
    2. 
        
    maubin
    1. 
        
    2. Show all issues

      Typo: "use used"

    3. reviewboard/reviews/testing/queries/review_requests.py (Diff revision 4)
       
       
       
       
       
       
      Show all issues

      Instead of raising a ValueError here, maybe it'd be better to just assert isinstance(to_user, str) like you do in the other get_review_requests_... methods. Or if not we should document the ValueError in a "Raises" section in the docs.

    4. 
        
    chipx86
    maubin
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-5.0.x (b6917a4)