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)