Add testing utilities for building review request Q-expressions.

 






 

 


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

 




















 

  

   Information

  





 
 

  chipx86
 









 
 

  Review Board
 









 
 

  release-5.0.x
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
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.

     


    		
    b3596dc11e0d36587d8dbb6b871fb702245d15c7

   



  
 




 













 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      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




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  








 
Change Summary:

 
Added a missing table join expectation in get_review_requests_from_user_q() when passing a user as a string.





 



  

   
Commits:

   
    





 

  
   


    

    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.

     


    		
    066eadd679fcbd13a975990cecd3e638b17d16a1

   




   


    

    

     

      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.

     


    		
    b3596dc11e0d36587d8dbb6b871fb702245d15c7

   



  
 







   		
  




  

   
Diff:

   
    
 Revision 4 (+5016 -1100)
Show changes
 
    docs/manual/extending/coderef/index.rst 
    reviewboard/reviews/testing/queries/review_requests.py 
    reviewboard/reviews/tests/test_review_request_manager.py 
 

   		
  



 









 

  
Checks run (2 succeeded)



  

 flake8
 passed.
 





  

 JSHint
 passed.
 





 















  

 
















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




maubin




   

   







  

 



 

  


   

   

    
 
maubin


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    Typo: "use used"
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  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. 




  4. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 




  

 









     

    

   

  

  

   
   Loading...