Summary

Fix transaction errors when running review group tests

Review Request #9177 — Created Sept. 7, 2017 and submitted Sept. 8, 2017, 2:44 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

Commit

4f2a4a4...

Reviewers

Groups

reviewboard

People

Description

When running unit tests, if an IntegrityError error is raised (even if
it is caught), it will break the transaction the unit test runs in.
Therefore, we have to run the potentially exception-raising code in its
own atomic block to avoid this happening.

This also changes a mistaken reference to self.self.local_site_name.

Testing Done

Ran unit tests.

Issues

Description	From	Last Updated
Could we use get_or_create()? That way, even though there's one additional query, we don't at all impact the transaction in …	chipx86	Sept. 7, 2017, 5:31 p.m.
F401 'django.db.transaction' imported but unused	reviewbot	Sept. 7, 2017, 5:32 p.m.
F401 'django.db.IntegrityError' imported but unused	reviewbot	Sept. 7, 2017, 5:32 p.m.
I'm confused. Why are we creating a local site if with_local_site is False?	david	Sept. 8, 2017, 2:42 p.m.

flake8 passed.

JSHint passed.

reviewboard/webapi/tests/test_review_group.py (Diff revision 1)

The issue has been resolved. Show all issues

Could we use get_or_create()? That way, even though there's one additional query, we don't at all impact the transaction in any form.

Actually, we get that query back if we pass in the returned LocalSite to create_review_group() instead of using with_local_site=. In fact, we save an additional query with the subsequent call. So I think this is the ideal way for multiple reasons.

Change Summary:

Addressed Christian's issues

Commit:

-	d757f862c994e11d8bf231dbb67cb80272449747
+	84a2d2228261d87de59124d6dfb513a45003af80

Diff:

Revision 2 (+19 -13)

Show changes

reviewboard/webapi/tests/test_review_group.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/webapi/tests/test_review_group.py (Diff revision 2)
The issue has been resolved. Show all issues
```
F401 'django.db.transaction' imported but unused
```
reviewboard/webapi/tests/test_review_group.py (Diff revision 2)
The issue has been resolved. Show all issues
```
F401 'django.db.IntegrityError' imported but unused
```

Commit:

-	84a2d2228261d87de59124d6dfb513a45003af80
+	4f2a4a488a8c47742496f233935aa341efdf44b8

Diff:

Revision 3 (+18 -12)

Show changes

reviewboard/webapi/tests/test_review_group.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

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

The issue has been dropped. Show all issues

I'm confused. Why are we creating a local site if with_local_site is False?

brennie Sept. 8, 2017, 2:43 p.m.

with_local_site indicates whether or not the created review group is bound to the local site. We always create a review group bound to the local site and none not bound to it so we can check we are returning the correct groups.

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (4d9ac67)