Summary

Fix regressions in the rbintegrations test suites.

Review Request #11198 — Created Sept. 23, 2020 and submitted Oct. 30, 2020, 2:24 p.m.

Information

Owner

chipx86

Repository

rbintegrations

Branch

release-1.0.x

Bugs

Depends On

Reviewers

Groups

rbintegrations

People

Description

Some recent changes broke a few of the rbintegrations tests. These
affected CircleCI, Travis CI, Slack, and Mattermost.

The CircleCI and Travis CI tests attempt an authorization on GitHub, but
the changes to move to token-based authentication resulted in some
changes to the HTTP methods performed to the GitHub API. The unit tests
weren't spying on the new methods, causing our tests to hit the API and
fail. Now, we spy on both methods, allowing compatibility with a larger
range of Review Board versions.

They've also been updated to provide two versions of the HTTP headers,
in order to fix address case normalization issues.

Travis CI got some fixes for reloading models. Newer unit tests added
calls to refresh_from_db(), which isn't available for Review Board 3.0.

Slack and Mattermost were both creating a review request, changing the
ID, and then saving, which wasn't particularly safe, and led to some
issues with RelationCounterField in a test. We now set the ID when
creating the review requests.

Testing Done

Unit tests pass for Review Board 3.0. Some work is still required for
4.0, which will be fixed separately.

Commits

Summary	ID
Fix regressions in the rbintegrations test suites. Some recent changes broke a few of the rbintegrations tests. These affected CircleCI, Travis CI, Slack, and Mattermost. The CircleCI and Travis CI tests attempt an authorization on GitHub, but the changes to move to token-based authentication resulted in some changes to the HTTP methods performed to the GitHub API. The unit tests weren't spying on the new methods, causing our tests to hit the API and fail. Now, we spy on both methods, allowing compatibility with a larger range of Review Board versions. They've also been updated to provide two versions of the HTTP headers, in order to fix address case normalization issues. Travis CI got some fixes for reloading models. Newer unit tests added calls to `refresh_from_db()`, which isn't available for Review Board 3.0. Slack and Mattermost were both creating a review request, changing the ID, and then saving, which wasn't particularly safe, and led to some issues with `RelationCounterField` in a test. We now set the ID when creating the review requests.	a418db3badf5d4b086574c1de3d43b2dfe99f869

Summary

Fix regressions in the rbintegrations test suites.

Some recent changes broke a few of the rbintegrations tests. These affected CircleCI, Travis CI, Slack, and Mattermost. The CircleCI and Travis CI tests attempt an authorization on GitHub, but the changes to move to token-based authentication resulted in some changes to the HTTP methods performed to the GitHub API. The unit tests weren't spying on the new methods, causing our tests to hit the API and fail. Now, we spy on both methods, allowing compatibility with a larger range of Review Board versions. They've also been updated to provide two versions of the HTTP headers, in order to fix address case normalization issues. Travis CI got some fixes for reloading models. Newer unit tests added calls to `refresh_from_db()`, which isn't available for Review Board 3.0. Slack and Mattermost were both creating a review request, changing the ID, and then saving, which wasn't particularly safe, and led to some issues with `RelationCounterField` in a test. We now set the ID when creating the review requests.

a418db3badf5d4b086574c1de3d43b2dfe99f869

Issues

Description	From	Last Updated
See my comment on Hailan's change--the capitalization seems to be dependent on Python version, not Review Board version.	david	Sept. 25, 2020, 1:15 a.m.

flake8 passed.

JSHint passed.

rbintegrations/slack/tests.py (Diff revision 1)

I'm curious what this does

chipx86 Sept. 23, 2020, 6:40 p.m.

To help manage our extensive test suite, we have a number of helper functions in the past TestCase (defined in reviewboard/testing/testcase.py) to create common objects, such as review requests (create_review_request()). This provides defaults for fields and a lot of default behavior (e.g., making it easy to create an associated repository). One of the things it allows us to do is to explicitly provide an ID (seen as an id or pk field) for the database (this patch's review request's id would be 11198, for example).

The older code was creating a review request in the database, then changing the ID and creating a new one. This led to two entries, which wasn't correct, and broke some internal state management that some fields have. So this particular change is just fixing the code to set the correct ID on creation.

rbintegrations/circleci/tests.py (Diff revision 1)

The issue has been resolved. Show all issues

See my comment on Hailan's change--the capitalization seems to be dependent on Python version, not Review Board version.

chipx86 Sept. 23, 2020, 11:16 p.m.

I'm about to post a change fixing this in a more reliable way in Review Board. I'll be undoing this part after that's in.

Change Summary:

Removed the Review Board 4.0 variations of HTTP headers, since it won't be relevant with /r/11199 and /r/11200.

Commits:

	Summary	ID
	Fix regressions in the rbintegrations test suites. Some recent changes broke a few of the rbintegrations tests. These affected CircleCI, Travis CI, Slack, and Mattermost. The CircleCI and Travis CI tests attempt an authorization on GitHub, but the changes to move to token-based authentication resulted in some changes to the HTTP methods performed to the GitHub API. The unit tests weren't spying on the new methods, causing our tests to hit the API and fail. Now, we spy on both methods, allowing compatibility with a larger range of Review Board versions. They've also been updated to provide two versions of the HTTP headers, in order to fix address case normalization issues. Travis CI got some fixes for reloading models. Newer unit tests added calls to `refresh_from_db()`, which isn't available for Review Board 3.0. Slack and Mattermost were both creating a review request, changing the ID, and then saving, which wasn't particularly safe, and led to some issues with `RelationCounterField` in a test. We now set the ID when creating the review requests.	957e2d19b155da58b88465f74937d1095465ce7e
	Fix regressions in the rbintegrations test suites. Some recent changes broke a few of the rbintegrations tests. These affected CircleCI, Travis CI, Slack, and Mattermost. The CircleCI and Travis CI tests attempt an authorization on GitHub, but the changes to move to token-based authentication resulted in some changes to the HTTP methods performed to the GitHub API. The unit tests weren't spying on the new methods, causing our tests to hit the API and fail. Now, we spy on both methods, allowing compatibility with a larger range of Review Board versions. They've also been updated to provide two versions of the HTTP headers, in order to fix address case normalization issues. Travis CI got some fixes for reloading models. Newer unit tests added calls to `refresh_from_db()`, which isn't available for Review Board 3.0. Slack and Mattermost were both creating a review request, changing the ID, and then saving, which wasn't particularly safe, and led to some issues with `RelationCounterField` in a test. We now set the ID when creating the review requests.	a418db3badf5d4b086574c1de3d43b2dfe99f869

Diff:

Revision 2 (+68 -24)

Show changes

	rbintegrations/circleci/tests.py
	rbintegrations/mattermost/tests.py
	rbintegrations/slack/tests.py
	rbintegrations/travisci/tests.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-1.0.x (7bc3fdf)