Summary

Update webapi test suite to test invalid PUT/POST operations.

Review Request #13285 — Created Sept. 22, 2023 and submitted Sept. 29, 2023, 3:33 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-6.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

This change updates our API test mixins to test these scenarios where
appropriate. Most of the individual test suites needed to add some setup
methods to handle this appropriately.

Unfortunately, the naming of the setup methods isn't ideal.
setup_http_not_allowed_item_test should really be
setup_http_not_allowed_put_test, and
setup_http_not_allowed_list_test should be
setup_http_not_allowed_post_test. These also do not currently support
similar test cases for when there is a local site (though that shouldn't
be a problem). Rather than trying to rename these methods now, I've just
gone with the old ones, and we can address the naming later. For this
reason I'm also holding off on adding missing docstrings or typing,
merely trying to match it to the individual style of each existing test
case.

Testing Done

Ran unit tests.

Commits

Summary	ID
Update webapi test suite to test invalid PUT/POST operations. The API resource implementation previously prevented POST requests to item resources, and with a new change to Djblets, will also prevent PUT requests to list resources. However, neither of these was properly tested in the test suite. This change updates our API test mixins to test these scenarios where appropriate. Most of the individual test suites needed to add some setup methods to handle this appropriately. Unfortunately, the naming of the setup methods isn't ideal. `setup_http_not_allowed_item_test` should really be `setup_http_not_allowed_put_test`, and `setup_http_not_allowed_list_test` should be `setup_http_not_allowed_post_test`. These also do not currently support similar test cases for when there is a local site (though that shouldn't be a problem). Rather than trying to rename these methods now, I've just gone with the old ones, and we can address the naming later. For this reason I'm also holding off on adding missing docstrings or typing, merely trying to match it to the individual style of each existing test case. Testing Done: Ran unit tests.	fe63a0f3bcee7eee39c3fe08d7b0c5697dcb4950

Summary

Update webapi test suite to test invalid PUT/POST operations.

The API resource implementation previously prevented POST requests to item resources, and with a new change to Djblets, will also prevent PUT requests to list resources. However, neither of these was properly tested in the test suite. This change updates our API test mixins to test these scenarios where appropriate. Most of the individual test suites needed to add some setup methods to handle this appropriately. Unfortunately, the naming of the setup methods isn't ideal. `setup_http_not_allowed_item_test` should really be `setup_http_not_allowed_put_test`, and `setup_http_not_allowed_list_test` should be `setup_http_not_allowed_post_test`. These also do not currently support similar test cases for when there is a local site (though that shouldn't be a problem). Rather than trying to rename these methods now, I've just gone with the old ones, and we can address the naming later. For this reason I'm also holding off on adding missing docstrings or typing, merely trying to match it to the individual style of each existing test case. Testing Done: Ran unit tests.

fe63a0f3bcee7eee39c3fe08d7b0c5697dcb4950

Issues

Description	From	Last Updated
local variable 'filediff' is assigned to but never used Column: 9 Error code: F841	reviewbot	Sept. 22, 2023, 5:55 p.m.
local variable 'diffset' is assigned to but never used Column: 9 Error code: F841	reviewbot	Sept. 22, 2023, 5:56 p.m.
Can we maybe combine the logic into: if is_list or 'PUT' not in resource.allowed_methods: mixins = (BasicPutNotAllowedTestsMixin,) else: ... We …	chipx86	Sept. 28, 2023, 12:25 p.m.
str	chipx86	Sept. 28, 2023, 12:24 p.m.
str	chipx86	Sept. 28, 2023, 12:24 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/webapi/tests/test_review_comment.py (Diff revision 1)
The issue has been dropped. Show all issues
```
local variable 'filediff' is assigned to but never used

Column: 9
Error code: F841
```

Commits:

	Summary	ID
	Update webapi test suite to test invalid PUT/POST operations. The API resource implementation previously prevented POST requests to item resources, and with a new change to Djblets, will also prevent PUT requests to list resources. However, neither of these was properly tested in the test suite. This change updates our API test mixins to test these scenarios where appropriate. Most of the individual test suites needed to add some setup methods to handle this appropriately. Unfortunately, the naming of the setup methods isn't ideal. `setup_http_not_allowed_item_test` should really be `setup_http_not_allowed_put_test`, and `setup_http_not_allowed_list_test` should be `setup_http_not_allowed_post_test`. These also do not currently support similar test cases for when there is a local site (though that shouldn't be a problem). Rather than trying to rename these methods now, I've just gone with the old ones, and we can address the naming later. For this reason I'm also holding off on adding missing docstrings or typing, merely trying to match it to the individual style of each existing test case. Testing Done: Ran unit tests.	9f83d024a8a515c134d343119f7546798d2b0d93
	Update webapi test suite to test invalid PUT/POST operations. The API resource implementation previously prevented POST requests to item resources, and with a new change to Djblets, will also prevent PUT requests to list resources. However, neither of these was properly tested in the test suite. This change updates our API test mixins to test these scenarios where appropriate. Most of the individual test suites needed to add some setup methods to handle this appropriately. Unfortunately, the naming of the setup methods isn't ideal. `setup_http_not_allowed_item_test` should really be `setup_http_not_allowed_put_test`, and `setup_http_not_allowed_list_test` should be `setup_http_not_allowed_post_test`. These also do not currently support similar test cases for when there is a local site (though that shouldn't be a problem). Rather than trying to rename these methods now, I've just gone with the old ones, and we can address the naming later. For this reason I'm also holding off on adding missing docstrings or typing, merely trying to match it to the individual style of each existing test case. Testing Done: Ran unit tests.	28e329f2aa27fbdffc28e05afb349a54f0cb796d

Diff:

Revision 2 (+1216 -72)

Show changes

	reviewboard/webapi/tests/mixins.py
	reviewboard/webapi/tests/test_api_token.py
	reviewboard/webapi/tests/test_default_reviewer.py
	reviewboard/webapi/tests/test_diff.py
	reviewboard/webapi/tests/test_diff_file_attachment.py
	reviewboard/webapi/tests/test_diffcommit.py
	reviewboard/webapi/tests/test_draft_diff.py
	reviewboard/webapi/tests/test_draft_diffcommit.py
	32 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/webapi/tests/test_review_comment.py (Diff revision 2)
The issue has been dropped. Show all issues
```
local variable 'diffset' is assigned to but never used

Column: 9
Error code: F841
```

Commits:

	Summary	ID
	Update webapi test suite to test invalid PUT/POST operations. The API resource implementation previously prevented POST requests to item resources, and with a new change to Djblets, will also prevent PUT requests to list resources. However, neither of these was properly tested in the test suite. This change updates our API test mixins to test these scenarios where appropriate. Most of the individual test suites needed to add some setup methods to handle this appropriately. Unfortunately, the naming of the setup methods isn't ideal. `setup_http_not_allowed_item_test` should really be `setup_http_not_allowed_put_test`, and `setup_http_not_allowed_list_test` should be `setup_http_not_allowed_post_test`. These also do not currently support similar test cases for when there is a local site (though that shouldn't be a problem). Rather than trying to rename these methods now, I've just gone with the old ones, and we can address the naming later. For this reason I'm also holding off on adding missing docstrings or typing, merely trying to match it to the individual style of each existing test case. Testing Done: Ran unit tests.	28e329f2aa27fbdffc28e05afb349a54f0cb796d
	Update webapi test suite to test invalid PUT/POST operations. The API resource implementation previously prevented POST requests to item resources, and with a new change to Djblets, will also prevent PUT requests to list resources. However, neither of these was properly tested in the test suite. This change updates our API test mixins to test these scenarios where appropriate. Most of the individual test suites needed to add some setup methods to handle this appropriately. Unfortunately, the naming of the setup methods isn't ideal. `setup_http_not_allowed_item_test` should really be `setup_http_not_allowed_put_test`, and `setup_http_not_allowed_list_test` should be `setup_http_not_allowed_post_test`. These also do not currently support similar test cases for when there is a local site (though that shouldn't be a problem). Rather than trying to rename these methods now, I've just gone with the old ones, and we can address the naming later. For this reason I'm also holding off on adding missing docstrings or typing, merely trying to match it to the individual style of each existing test case. Testing Done: Ran unit tests.	2ec11767ae0728f938a75ac2b36721265ea66a2a

Diff:

Revision 3 (+1214 -72)

Show changes

	reviewboard/webapi/tests/mixins.py
	reviewboard/webapi/tests/test_api_token.py
	reviewboard/webapi/tests/test_default_reviewer.py
	reviewboard/webapi/tests/test_diff.py
	reviewboard/webapi/tests/test_diff_file_attachment.py
	reviewboard/webapi/tests/test_diffcommit.py
	reviewboard/webapi/tests/test_draft_diff.py
	reviewboard/webapi/tests/test_draft_diffcommit.py
	32 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/webapi/tests/mixins.py (Diff revision 3)

The issue has been resolved. Show all issues

Can we maybe combine the logic into:

if is_list or 'PUT' not in resource.allowed_methods:
    mixins = (BasicPutNotAllowedTestsMixin,)
else:
    ...

We can do a similar thing for the POST tests above.

reviewboard/webapi/tests/test_repository_group.py (Diff revision 3)
The issue has been resolved. Show all issues
```
str
```
reviewboard/webapi/tests/test_repository_user.py (Diff revision 3)
The issue has been resolved. Show all issues
```
str
```

Commits:

	Summary	ID
	Update webapi test suite to test invalid PUT/POST operations. The API resource implementation previously prevented POST requests to item resources, and with a new change to Djblets, will also prevent PUT requests to list resources. However, neither of these was properly tested in the test suite. This change updates our API test mixins to test these scenarios where appropriate. Most of the individual test suites needed to add some setup methods to handle this appropriately. Unfortunately, the naming of the setup methods isn't ideal. `setup_http_not_allowed_item_test` should really be `setup_http_not_allowed_put_test`, and `setup_http_not_allowed_list_test` should be `setup_http_not_allowed_post_test`. These also do not currently support similar test cases for when there is a local site (though that shouldn't be a problem). Rather than trying to rename these methods now, I've just gone with the old ones, and we can address the naming later. For this reason I'm also holding off on adding missing docstrings or typing, merely trying to match it to the individual style of each existing test case. Testing Done: Ran unit tests.	2ec11767ae0728f938a75ac2b36721265ea66a2a
	Update webapi test suite to test invalid PUT/POST operations. The API resource implementation previously prevented POST requests to item resources, and with a new change to Djblets, will also prevent PUT requests to list resources. However, neither of these was properly tested in the test suite. This change updates our API test mixins to test these scenarios where appropriate. Most of the individual test suites needed to add some setup methods to handle this appropriately. Unfortunately, the naming of the setup methods isn't ideal. `setup_http_not_allowed_item_test` should really be `setup_http_not_allowed_put_test`, and `setup_http_not_allowed_list_test` should be `setup_http_not_allowed_post_test`. These also do not currently support similar test cases for when there is a local site (though that shouldn't be a problem). Rather than trying to rename these methods now, I've just gone with the old ones, and we can address the naming later. For this reason I'm also holding off on adding missing docstrings or typing, merely trying to match it to the individual style of each existing test case. Testing Done: Ran unit tests.	fe63a0f3bcee7eee39c3fe08d7b0c5697dcb4950

Diff:

Revision 4 (+1164 -32)

Show changes

	reviewboard/webapi/tests/mixins.py
	reviewboard/webapi/tests/test_api_token.py
	reviewboard/webapi/tests/test_default_reviewer.py
	reviewboard/webapi/tests/test_diff.py
	reviewboard/webapi/tests/test_diff_file_attachment.py
	reviewboard/webapi/tests/test_diffcommit.py
	reviewboard/webapi/tests/test_draft_diff.py
	reviewboard/webapi/tests/test_draft_diffcommit.py
	32 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-6.x (2365763)