Summary

Fix up CircleCI/TravisCI tests and webapi token generation.

Review Request #9467 — Created Jan. 1, 2018 and submitted Jan. 8, 2018, 9:15 p.m.

Information

Owner

david

Repository

rbintegrations

Branch

release-0.5.x

Bugs

Depends On

Commit

a6122bc...

Reviewers

Groups

rbintegrations

People

Description

A few API changes in djblets and kgb caused a couple breakages in the
rbintegrations tests. This change fixes those up.

In review, it was also pointed out that assertions inside callbacks that
come from something using SignalHook won't be recorded properly,
because the extension sandboxing swallows them. I've moved code around
so that we do all the assertions inside the test case code instead of in
deep callbacks.

Testing Done

Ran unit tests.

Issues

Description	From	Last Updated
F841 local variable 'url' is assigned to but never used	reviewbot	Jan. 2, 2018, 2:13 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Jan. 2, 2018, 2:13 p.m.
These will stick around for every test. We should store it in a local dictionary variable. Same with the others.	chipx86	Jan. 3, 2018, 4:02 p.m.

flake8 passed.

JSHint passed.

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

The asserts inside a method swapped by kgb don't work when called from SignalHook callbacks - the default sandbox_errors=True catches all exceptions. This is why the unexpected keyword argument error for _urlopen didn't cause the tests to fail before, and any failed asserts inside the method won't be seen either.

Is there some way to possibly use kgb to change the default for sandbox_errors in unit tests? Or perhaps another more general solution?

david Jan. 2, 2018, 1:17 p.m.

Hmm, that's a good point. I think perhaps what we need to do is record the failure inside the callback and then assert outside of it.

Summary:

-	Fix up CircleCI tests and webapi token generation.
+	Fix up CircleCI/TravisCI tests and webapi token generation.

Description:

		A few API changes in djblets and kgb caused a couple breakages in the
		rbintegrations tests. This change fixes those up.
	+
	+	In review, it was also pointed out that assertions inside callbacks that
	+	come from something using `SignalHook` won't be recorded properly,
	+	because the extension sandboxing swallows them. I've moved code around
	+	so that we do all the assertions inside the test case code instead of in
	+	deep callbacks.

Commit:

-	2912eb6f856b53b7afe876dede0367ae66b7bbb9
+	3ea03085ccf5f301e3d8282574d8ab9ecb6cd25c

Diff:

Revision 2 (+82 -62)

Show changes

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

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbintegrations/circleci/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
F841 local variable 'url' is assigned to but never used
```
rbintegrations/travisci/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
E501 line too long (80 > 79 characters)
```

Commit:

-	3ea03085ccf5f301e3d8282574d8ab9ecb6cd25c
+	fbdbb70e88f0c8eea57b74ca0c546c1aa6f3aacc

Diff:

Revision 3 (+81 -62)

Show changes

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

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

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

The issue has been resolved. Show all issues

These will stick around for every test. We should store it in a local dictionary variable.

Same with the others.

Commit:

-	fbdbb70e88f0c8eea57b74ca0c546c1aa6f3aacc
+	a6122bc2a5b9e21ab94b33322a770e06ef8ceeca

Diff:

Revision 4 (+89 -62)

Show changes

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

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-0.5.x (c03045a)