Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only run oauth tests if a connection succeeds #908

Closed
wants to merge 1 commit into from

Conversation

murraybd
Copy link

In the autopkgtest infrastructure for Ubuntu we use a squid proxy and were not receiving an OSError in _oauth_request but instead were receiving a 403 status code. Regardless, checking for the status_code of the session.get() call seems like a good idea in general.

In the autopkgtest infrastructure for Ubuntu we use a squid proxy and were not receiving an OSError in _oauth_request but instead were receiving a 403 status code. Regardless, checking for the status_code of the session.get() call seems like a good idea in general.
Comment on lines +105 to +108
if connection.status_code == 200:
return connection.text
else:
raise unittest.SkipTest()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if connection.status_code == 200:
return connection.text
else:
raise unittest.SkipTest()
connection.raise_for_status()
return connection.text

We should just call raise_for_status() here to avoid code duplication.

It might also be sensible to raise unittest.SkipTest() unconditionally at the beginning of _oauth_request() since the public test server at term.ie is not available anymore and hasn't been for some time.

@mikf mikf closed this in 422e69f Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants