Summary

Add expected queries and enhance tests for Group access queries.

Review Request #13406 — Created Nov. 7, 2023 and submitted Dec. 7, 2023, 1:33 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-5.0.x

Bugs

Depends On

Blocks

13407

Reviewers

Groups

reviewboard

People

Description

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.

Testing Done

Unit tests pass.

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.	19eaaa95f857a5be74f08b83d1545a27c1980289

Summary

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

Issues

Description	From	Last Updated
Hold off on reviewing this. I'm going to make some changes and combine with the .accessible() change.	chipx86	Nov. 8, 2023, 10:16 p.m.
undefined name 'get_local_site_profile_equeries' Column: 29 Error code: F821	reviewbot	Nov. 7, 2023, 9:09 p.m.
'django.contrib.auth.models.Permission' imported but unused Column: 1 Error code: F401	reviewbot	Nov. 8, 2023, 10:16 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/reviews/testing/queries.py (Diff revision 1)
The issue has been resolved. Show all issues
```
undefined name 'get_local_site_profile_equeries'

Column: 29
Error code: F821
```

Change Summary:

Removed some dead code.

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.	103a8e361bb000ae0697535498a7e40466a4900f
	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

Diff:

Revision 2 (+1774 -264)

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 (2 succeeded)

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues

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

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

reviewboard/reviews/tests/test_review_group_manager.py (Diff revision 3)
The issue has been resolved. Show all issues
```
'django.contrib.auth.models.Permission' imported but unused

Column: 1
Error code: F401
```

Change Summary:

Removed an unused import.

Commits:

	Summary	ID
	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
	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.	8ce69c0e4ea4e893324eb418d7feab6613e3a431

Diff:

Revision 4 (+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 (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Change Summary:


Moved this module to reviewboard.reviews.testing.queries.review_groups, since the file would otherwise grow considerably.
Broke out the Q-building into a standalone function, for use in upcoming datagrid test updates.

Description:

~		This introduces a new `reviewboard.reviews.testing.queries` module that
~		contains the following utility functions for building expected queries
~		for unit tests:
	~	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_equeries()`
~		`get_review_groups_accessible_ids_equeries()`
	~	`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 queries that matches calls to
	~	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.

Commits:

	Summary	ID
	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.	8ce69c0e4ea4e893324eb418d7feab6613e3a431
	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

Diff:

Revision 5 (+2522 -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.

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.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (357b84a)