Summary

Fix CircleCI build not running on personal GitHub repositories.

Review Request #11190 — Created Sept. 19, 2020 and submitted Dec. 13, 2020, 3:41 a.m.

Information

Owner

Repository

rbintegrations

Branch

master

Bugs

Depends On

Reviewers

Groups

People

Description

Fix CircleCI build not running on personal GitHub repositories.

CircleCI pipelines were not running for personal GitHub repositories when
a review request is made because the GitHub username was being extracted
from the wrong place.

RBIntegrations was pulling the wrong key from extra_data and it's been
updated to grab the associated HostingService's username.

This patch depends on /r/11198 and /r/11204 for the unit tests to pass.

Testing Done

Manual testing: A personal public and private repository was set up and

integrated with CircleCI. They are confirmed to be running without using

Review Board. Then, a review request was made for both the repositories.

Using the CircleCI web UI, the pipelines were triggered.
Unit tests were created to verify that the username is being pulled

from the correct place for the 4 different GitHub repository plans.

These were ran on Review Board 3.0 and Review Board 4.0 and their

supported versions of Python.

Commits

Summary	ID
Fix CircleCI not running on personal GitHub repos.	00999ad311dbeb178fa70a100da3d5cd265cc44d
Add CircleCI different GitHub repository plan unit tests.	e788898d1f3a7f977a9b57d3504a63f6faa705df
Refactor CircleCI tests to reduce duplication.	7a08a8b7e2a3e2622112a3d95484174e5f0cdc04
Fix types in CircleCI docstrings to Python types.	0ebb174f668ab7352f4c27454a78f1720c76537a

Issues

Description	From	Last Updated
The change summary should make it more clear which CI we're talking about. There's 3 in rbintegrations. It'd be best …	chipx86	Sept. 20, 2020, 9:42 a.m.
We generally avoid shortening words, because not everybody's coming in with the same level of knowledge. "repositories" would be better …	chipx86	Sept. 20, 2020, 9:42 a.m.
Change descriptions should wrap at around 75 characters. This goes for the description and testing.	chipx86	Sept. 20, 2020, 9:42 a.m.
The description is helpful, but it doesn't really say what the problem really was and how it was fixed. This …	chipx86	Sept. 20, 2020, 9:46 a.m.
Your testing mentions "manual testing," but doesn't mention anything about unit tests. Since you caught a bug manually that hadn't …	chipx86	Sept. 23, 2020, 10:24 p.m.
Doing a quick grep through the codebase, I can see that the Travis CI support has the same problem. Maybe …	chipx86	Sept. 23, 2020, 11:30 p.m.
W293 blank line contains whitespace	reviewbot	Oct. 8, 2020, 10:20 p.m.
W292 no newline at end of file	reviewbot	Oct. 8, 2020, 10:20 p.m.
The type name here should be unicode, rather than string.	david	Oct. 12, 2020, 2:33 p.m.
Type here should be unicode rather than str.	david	Oct. 12, 2020, 2:33 p.m.
json isn't a type in Python. This is returning dict.	david	Oct. 12, 2020, 2:33 p.m.

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues

The change summary should make it more clear which CI we're talking about. There's 3 in rbintegrations. It'd be best to specifically say "CircleCI".

The issue has been resolved. Show all issues

We generally avoid shortening words, because not everybody's coming in with the same level of knowledge. "repositories" would be better than "repos" in the summary/description.

The issue has been resolved. Show all issues

Change descriptions should wrap at around 75 characters. This goes for the description and testing.

The issue has been resolved. Show all issues

The description is helpful, but it doesn't really say what the problem really was and how it was fixed. This is good information to have so I don't have to dig into code.

You don't need to go into every little detail, but saying that it was pulling out the wrong key from extra_data and it's been updated to grab the associated HostingServiceAccount's username would be good.

The issue has been resolved. Show all issues

Your testing mentions "manual testing," but doesn't mention anything about unit tests. Since you caught a bug manually that hadn't been noticed in tests, it's the right time to add some tests to make sure that all of those types of hosting setups do what they're expected to do. One test per setup.

This is going to be longer than the fix, but since unit tests will be a big part of what you end up doing with any new code you write, now's a great time to learn the tricks and procedures for writing them :)

The issue has been dropped. Show all issues

Doing a quick grep through the codebase, I can see that the Travis CI support has the same problem. Maybe this change should focus on fixing both?

MarcusBoay Sept. 23, 2020, 11:30 p.m.

I'm running into authorization errors for the TravisCI tests. Let's make this a separate patch so that we can land this fix.

Summary:

-	Fix CI build not running on personal GitHub repos.
+	Fix CircleCI build not running on personal GitHub repositories.

Description:

~		Fix CI build not running on personal GitHub repos.
	~	Fix CircleCI build not running on personal GitHub repositories.

~		CircleCI pipelines were not running for personal GitHub repositories when a review request is made because the GitHub username was being extracted from the wrong place.
	~	CircleCI pipelines were not running for personal GitHub repositories when
	+	a review request is made because the GitHub username was being extracted
	+	from the wrong place.
	+
	+	RBIntegrations was pulling the wrong key from `extra_data` and it's been
	+	updated to grab the associated `HostingService`'s `username`.

Testing Done:

~		Manual testing. A personal public and private repository was set up and integrated with CircleCI. They are confirmed to be running without using Review Board. Then, a review request was made for both the repositories. Using the CircleCI web UI, the pipelines were triggered.
	~	Manual testing. A personal public and private repository was set up and
	+	integrated with CircleCI. They are confirmed to be running without using
	+	Review Board. Then, a review request was made for both the repositories.
	+	Using the CircleCI web UI, the pipelines were triggered.

Testing Done:

~		Manual testing. A personal public and private repository was set up and
	~	Manual testing: A personal public and private repository was set up and
		integrated with CircleCI. They are confirmed to be running without using
		Review Board. Then, a review request was made for both the repositories.
		Using the CircleCI web UI, the pipelines were triggered.
	+
	+	Unit tests were created to verify that the `username` is being pulled
	+	from the correct place for the 4 different GitHub repository plans.

Commits:

	Summary	ID
	Fix CI build not running on personal GitHub repos.	b04261844035ea88c6c773da76209ea7adafa9fa
	Fix CI build not running on personal GitHub repos.	b04261844035ea88c6c773da76209ea7adafa9fa
	Add CircleCI different GitHub repository plan tests.	1e1495d29454fa904253bc128d3a0ff58a44ed32

Diff:

Revision 2 (+292 -16)

	rbintegrations/circleci/integration.py
	rbintegrations/circleci/tests.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Commits:

	Summary	ID
	Fix CI build not running on personal GitHub repos.	b04261844035ea88c6c773da76209ea7adafa9fa
	Add CircleCI different GitHub repository plan tests.	1e1495d29454fa904253bc128d3a0ff58a44ed32
	Fix CI build not running on personal GitHub repos.	2db84fc46bff4ab8515381d6fb95b49e10221cd5
	Add CircleCI different GitHub repository plan tests.	6556251c5c9647f91348ec6b7ea4860e22493d9e

Diff:

Revision 3 (+298 -16)

	rbintegrations/circleci/integration.py
	rbintegrations/circleci/tests.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Description:

		Fix CircleCI build not running on personal GitHub repositories.

		CircleCI pipelines were not running for personal GitHub repositories when
		a review request is made because the GitHub username was being extracted
		from the wrong place.

		RBIntegrations was pulling the wrong key from `extra_data` and it's been
		updated to grab the associated `HostingService`'s `username`.
	+
	+	This patch depends on /r/11198 for the unit tests to pass.

Change Summary:

Update testing description with RB3 and RB4

Description:

		Fix CircleCI build not running on personal GitHub repositories.

		CircleCI pipelines were not running for personal GitHub repositories when
		a review request is made because the GitHub username was being extracted
		from the wrong place.

		RBIntegrations was pulling the wrong key from `extra_data` and it's been
		updated to grab the associated `HostingService`'s `username`.

~		This patch depends on /r/11198 for the unit tests to pass.
	~	This patch depends on /r/11198 and /r/11204 for the unit tests to pass.

Testing Done:

		Manual testing: A personal public and private repository was set up and
		integrated with CircleCI. They are confirmed to be running without using
		Review Board. Then, a review request was made for both the repositories.
		Using the CircleCI web UI, the pipelines were triggered.

		Unit tests were created to verify that the `username` is being pulled
~		from the correct place for the 4 different GitHub repository plans.
	~	from the correct place for the 4 different GitHub repository plans.
	+	These were ran on Review Board 3.0 and Review Board 4.0 and their
	+	supported versions of Python.

Change Summary:

Add missing documentation

Commits:

	Summary	ID
	Fix CI build not running on personal GitHub repos.	2db84fc46bff4ab8515381d6fb95b49e10221cd5
	Add CircleCI different GitHub repository plan tests.	6556251c5c9647f91348ec6b7ea4860e22493d9e
	Fix CI build not running on personal GitHub repos.	2db84fc46bff4ab8515381d6fb95b49e10221cd5
	Add CircleCI different GitHub repository plan unit tests.	ce02f30ca1d14bba62a866a287b44b9cd322660d

Diff:

Revision 4 (+310 -16)

	rbintegrations/circleci/integration.py
	rbintegrations/circleci/tests.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Refactor code to reduce duplication

Commits:

	Summary	ID
	Fix CI build not running on personal GitHub repos.	2db84fc46bff4ab8515381d6fb95b49e10221cd5
	Add CircleCI different GitHub repository plan unit tests.	ce02f30ca1d14bba62a866a287b44b9cd322660d
	Fix CI build not running on personal GitHub repos.	2db84fc46bff4ab8515381d6fb95b49e10221cd5
	Add CircleCI different GitHub repository plan unit tests.	ce02f30ca1d14bba62a866a287b44b9cd322660d
	Refactor CircleCI tests to reduce duplication.	d33504c8bbe0da9e5d7a2d45f3306919dc83761b

Diff:

Revision 5 (+331 -151)

	rbintegrations/circleci/integration.py
	rbintegrations/circleci/tests.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbintegrations/circleci/tests.py (Diff revision 5)
The issue has been resolved. Show all issues
```
W293 blank line contains whitespace
```
rbintegrations/circleci/tests.py (Diff revision 5)
The issue has been resolved. Show all issues
```
W292 no newline at end of file
```

Change Summary:

Resolve flake8 issue

Commits:

	Summary	ID
	Fix CI build not running on personal GitHub repos.	2db84fc46bff4ab8515381d6fb95b49e10221cd5
	Add CircleCI different GitHub repository plan unit tests.	ce02f30ca1d14bba62a866a287b44b9cd322660d
	Refactor CircleCI tests to reduce duplication.	d33504c8bbe0da9e5d7a2d45f3306919dc83761b
	Fix CI build not running on personal GitHub repos.	2db84fc46bff4ab8515381d6fb95b49e10221cd5
	Add CircleCI different GitHub repository plan unit tests.	ce02f30ca1d14bba62a866a287b44b9cd322660d
	Refactor CircleCI tests to reduce duplication.	40741f106259d7dad5e3e36d4dbb2797c3f851ea

Diff:

Revision 6 (+331 -151)

	rbintegrations/circleci/integration.py
	rbintegrations/circleci/tests.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Just a few simple comments about types in docstrings.

MarcusBoay Oct. 12, 2020, 2:33 p.m.

Thanks! I was confused as to what types I should put into the docstrings.

rbintegrations/circleci/integration.py (Diff revision 6)
The issue has been resolved. Show all issues
```
The type name here should be unicode, rather than string.
```
rbintegrations/circleci/tests.py (Diff revision 6)
The issue has been resolved. Show all issues
```
Type here should be unicode rather than str.
```
rbintegrations/circleci/tests.py (Diff revision 6)
The issue has been resolved. Show all issues
```
json isn't a type in Python. This is returning dict.
```

Commits:

	Summary	ID
	Fix CI build not running on personal GitHub repos.	2db84fc46bff4ab8515381d6fb95b49e10221cd5
	Add CircleCI different GitHub repository plan unit tests.	ce02f30ca1d14bba62a866a287b44b9cd322660d
	Refactor CircleCI tests to reduce duplication.	40741f106259d7dad5e3e36d4dbb2797c3f851ea
	Fix CircleCI not running on personal GitHub repos.	00999ad311dbeb178fa70a100da3d5cd265cc44d
	Add CircleCI different GitHub repository plan unit tests.	e788898d1f3a7f977a9b57d3504a63f6faa705df
	Refactor CircleCI tests to reduce duplication.	7a08a8b7e2a3e2622112a3d95484174e5f0cdc04
	Fix types in CircleCI docstrings to Python types.	0ebb174f668ab7352f4c27454a78f1720c76537a

Diff:

Revision 7 (+334 -154)

	rbintegrations/circleci/integration.py
	rbintegrations/circleci/tests.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-2.0.x (4a5a3c7)

Loading...