Add expected queries and enhance tests for Group access queries.

 






 

 


  Review Request #13406
  —
  Created Nov. 7, 2023 and updated 

 




















 

  

   Information

  





 
 

  chipx86
 









 
 

  Review Board
 









 
 

  release-5.0.x
 









 
 

  
 









 
 

  
 









 
 

  13407
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
This introduces the reviewboard.reviews.testing.queries.review_groups

module for query-building functions for tests. It contains the following

methods:


    

  • get_review_groups_accessible_q()
    • 

  • get_review_groups_accessible_prep_equeries()
    • 

  • get_review_groups_accessible_equeries()
    • 

  • get_review_groups_accessible_ids_equeries()
    • 



These generate expected Q objects and queries that matches calls to

Group.objects.accessible() and Group.objects.accessible_ids().

They're written in a verbose manner that ensures we only ever test

against finalized Q expressions, to minimize chances of regressions or

unexpected behavior.


The unit tests for .accessible() and .accessible_ids() have been

completely rewritten. They now check against these expected queries, but

they've also been expanded considerably to more thoroughly check access

with positive and negative test data, to better catch regressions. The

approach being used here will be carried over to other accessibility

tests for the next batch of this peformance work.

 








  


 
 

  
Unit tests pass.

 







 









 





  

 


 
 

  



 

  
   


    Summary
    ID

   

  
  

   


    

     

      Add expected queries and enhance tests for Group access queries.
      
This introduces the `reviewboard.reviews.testing.queries.review_groups`
module for query-building functions for tests. It contains the following
methods:

* `get_review_groups_accessible_q()`
* `get_review_groups_accessible_prep_equeries()`
* `get_review_groups_accessible_equeries()`
* `get_review_groups_accessible_ids_equeries()`

These generate expected `Q` objects and queries that matches calls to
`Group.objects.accessible()` and `Group.objects.accessible_ids()`.
They're written in a verbose manner that ensures we only ever test
against finalized Q expressions, to minimize chances of regressions or
unexpected behavior.

The unit tests for `.accessible()` and `.accessible_ids()` have been
completely rewritten. They now check against these expected queries, but
they've also been expanded considerably to more thoroughly check access
with positive and negative test data, to better catch regressions. The
approach being used here will be carried over to other accessibility
tests for the next batch of this peformance work.

     


    		
    19eaaa95f857a5be74f08b83d1545a27c1980289

   



  
 




 













 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
Hold off on reviewing this. I'm going to make some changes and combine with the .accessible() change.

      		
      
       chipx86chipx86
      
      
       
      
     




     

      
       
       
undefined name 'get_local_site_profile_equeries' Column: 29 Error code: F821

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
'django.contrib.auth.models.Permission' imported but unused Column: 1 Error code: F401

      		
      
       reviewbotreviewbot
      
      
       
      
     


    
   

  

 







 






  

 



















 




 

  


   

   

    
 
Checks run (1 failed, 1 succeeded)


    
   


  

  

  





 

 flake8
 failed.
 





 

 JSHint
 passed.
 













 
flake8

 


 










  

 





 














 



 

  

   




chipx86




   

   

  

 



 















 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 







  2. 
 
    
  
  
    
  
   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    Hold off on reviewing this. I'm going to make some changes and combine with the .accessible() change.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 













  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  








 
Change Summary:

 
Rewrote unit tests based on the new repository tests, which test for positive and negative results in user/access-focused tests.





 



  

   
Summary:

   
    
-Add expected queries and enhance tests for Group.objects.accessible_ids().
+Add expected queries and enhance tests for Group access queries.

   		
  




  

   
Description:

   
    
     
This introduces a new reviewboard.reviews.testing.queries module that
 ~   contains the first utility function for building expected queries for
 ~   unit tests: get_review_groups_accessible_ids_equeries().
   ~ contains the following utility functions for building expected queries
   ~ for unit tests:
     

 ~   
This function generates expected queries that matches calls to
 ~   Group.objects.accessible_ids(). This will be the first of many. This
   ~ 
  • get_review_groups_accessible_equeries()
   ~ 
  • get_review_groups_accessible_ids_equeries()
 -   function very intently does not copy the Q-building logic of
 -   accessible_ids(), instead opting to have explicit conditionals that
 -   all result in fully-formed Q expressions to check against. This will
 -   likely get more complex in time.
     

 ~   
The unit tests for accessible_ids() have been updated to test for
 ~   queries, and to broaden the test suite. Previously, we had test
 ~   functions that tested multiple conditions at once, but this led to
 ~   caching that would interfere with query expectations. These have now
 ~   been split into individual test functions, which have themselves been
 ~   extended to also incorporate Local Sites into the checks.
   ~ 
These generate expected queries that matches calls to
   ~ Group.objects.accessible() and Group.objects.accessible_ids().
   ~ They're written in a verbose manner that ensures we only ever test
   ~ against finalized Q expressions, to minimize chances of regressions or
   ~ unexpected behavior.
   ~ 

   + 
The unit tests for .accessible() and .accessible_ids() have been
   + completely rewritten. They now check against these expected queries, but
   + they've also been expanded considerably to more thoroughly check access
   + with positive and negative test data, to better catch regressions. The
   + approach being used here will be carried over to other accessibility
   + tests for the next batch of this peformance work.

   		
  




  

   
Commits:

   
    





 

  
   


    

    Summary
    ID

   

  
  

   


    

    

     

      Add expected queries and enhance tests for Group.objects.accessible_ids().
      
This introduces a new `reviewboard.reviews.testing.queries` module that
contains the first utility function for building expected queries for
unit tests: `get_review_groups_accessible_ids_equeries()`.

This function generates expected queries that matches calls to
`Group.objects.accessible_ids()`. This will be the first of many. This
function very intently does not copy the `Q`-building logic of
`accessible_ids()`, instead opting to have explicit conditionals that
all result in fully-formed `Q` expressions to check against. This will
likely get more complex in time.

The unit tests for `accessible_ids()` have been updated to test for
queries, and to broaden the test suite. Previously, we had test
functions that tested multiple conditions at once, but this led to
caching that would interfere with query expectations. These have now
been split into individual test functions, which have themselves been
extended to also incorporate Local Sites into the checks.

     


    		
    2c7bde47ac8de8081d81d8a68a3720282f1a97d7

   




   


    

    

     

      Add expected queries and enhance tests for Group access queries.
      
This introduces a new `reviewboard.reviews.testing.queries` module that
contains the following utility functions for building expected queries
for unit tests:

* `get_review_groups_accessible_equeries()`
* `get_review_groups_accessible_ids_equeries()`

These generate expected queries that matches calls to
`Group.objects.accessible()` and `Group.objects.accessible_ids()`.
They're written in a verbose manner that ensures we only ever test
against finalized Q expressions, to minimize chances of regressions or
unexpected behavior.

The unit tests for `.accessible()` and `.accessible_ids()` have been
completely rewritten. They now check against these expected queries, but
they've also been expanded considerably to more thoroughly check access
with positive and negative test data, to better catch regressions. The
approach being used here will be carried over to other accessibility
tests for the next batch of this peformance work.

     


    		
    f91c7709d9b681c8e8b99ee7a03481fa06de6e88

   



  
 







   		
  




  

   
Diff:

   
    
 Revision 3 (+2270 -644)
Show changes
 
    reviewboard/reviews/managers.py 
    reviewboard/reviews/testing/__init__.py 
    reviewboard/reviews/testing/queries.py 
    reviewboard/reviews/tests/test_review_group_manager.py 
 

   		
  



 









 

  
Checks run (1 failed, 1 succeeded)



  

 flake8
 failed.
 





  

 JSHint
 passed.
 





 







 
flake8

 


 















  

 
















 



 

  

   




chipx86




   

   

  

 



 















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 















 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  








 
Change Summary:

 
Added an option for avoiding permission lookups if already cached.





 



  

   
Commits:

   
    





 

  
   


    

    Summary
    ID

   

  
  

   


    

    

     

      Add expected queries and enhance tests for Group access queries.
      
This introduces the `reviewboard.reviews.testing.queries.review_groups`
module for query-building functions for tests. It contains the following
methods:

* `get_review_groups_accessible_q()`
* `get_review_groups_accessible_prep_equeries()`
* `get_review_groups_accessible_equeries()`
* `get_review_groups_accessible_ids_equeries()`

These generate expected `Q` objects and queries that matches calls to
`Group.objects.accessible()` and `Group.objects.accessible_ids()`.
They're written in a verbose manner that ensures we only ever test
against finalized Q expressions, to minimize chances of regressions or
unexpected behavior.

The unit tests for `.accessible()` and `.accessible_ids()` have been
completely rewritten. They now check against these expected queries, but
they've also been expanded considerably to more thoroughly check access
with positive and negative test data, to better catch regressions. The
approach being used here will be carried over to other accessibility
tests for the next batch of this peformance work.

     


    		
    e061dad69732bc6174c7c3978bcb8558c19001b8

   




   


    

    

     

      Add expected queries and enhance tests for Group access queries.
      
This introduces the `reviewboard.reviews.testing.queries.review_groups`
module for query-building functions for tests. It contains the following
methods:

* `get_review_groups_accessible_q()`
* `get_review_groups_accessible_prep_equeries()`
* `get_review_groups_accessible_equeries()`
* `get_review_groups_accessible_ids_equeries()`

These generate expected `Q` objects and queries that matches calls to
`Group.objects.accessible()` and `Group.objects.accessible_ids()`.
They're written in a verbose manner that ensures we only ever test
against finalized Q expressions, to minimize chances of regressions or
unexpected behavior.

The unit tests for `.accessible()` and `.accessible_ids()` have been
completely rewritten. They now check against these expected queries, but
they've also been expanded considerably to more thoroughly check access
with positive and negative test data, to better catch regressions. The
approach being used here will be carried over to other accessibility
tests for the next batch of this peformance work.

     


    		
    19eaaa95f857a5be74f08b83d1545a27c1980289

   



  
 







   		
  




  

   
Diff:

   
    
 Revision 6 (+2558 -644)
Show changes
 
    docs/manual/extending/coderef/index.rst 
    reviewboard/reviews/managers.py 
    reviewboard/reviews/testing/__init__.py 
    reviewboard/reviews/testing/queries/__init__.py 
    reviewboard/reviews/testing/queries/review_groups.py 
    reviewboard/reviews/tests/test_review_group_manager.py 
 

   		
  



 









 

  
Checks run (2 succeeded)



  

 flake8
 passed.
 





  

 JSHint
 passed.
 





 















  

 
















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




maubin




   

   







  

 



 

  


   

   

    
 
maubin


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 









     

    

   

  

  

   
   Loading...