★ wanayoo — archive 1999 https://github.com/uber/Python-Sample-Application/pull/13Nouvelle recherche | Portail wanayoo
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

Improve code coverage by including missing tests #13

Merged
merged 6 commits into from Aug 22, 2014
Merged

Improve code coverage by including missing tests #13

merged 6 commits into from Aug 22, 2014

Conversation

@kaushal
Copy link
Contributor

@kaushal kaushal commented Aug 22, 2014

No description provided.

kaushal added 4 commits Aug 22, 2014
@coveralls
Copy link

@coveralls coveralls commented Aug 22, 2014

Coverage Status

Coverage increased (+12.5%) when pulling a07de3b on coverage into e5904a4 on master.

app.py Outdated
@@ -223,10 +223,10 @@ def get_redirect_uri(request):
"""Return OAuth redirect URI."""
parsed_url = urlparse(request.url)
if parsed_url.hostname == 'localhost':
return 'http://{hostname}:{port}/submit'.format(
return 'https://{hostname}:{port}/submit'.format(

This comment has been minimized.

@zheller

zheller Aug 22, 2014
Contributor

are you sure we want to do this?

This comment has been minimized.

@kaushal

kaushal Aug 22, 2014
Author Contributor

Yes it needs to happen because of all the ssl stuff @ai did

This comment has been minimized.

@ascandella

ascandella Aug 22, 2014
Contributor

do you mean me? all i did was fix the test by enabling debug mode. prior to that, literally every test was failing

This comment has been minimized.

@ascandella

ascandella Aug 22, 2014
Contributor

also, i only enabled it for the test run, so it should not affect normal behavior

app.py Outdated
hostname=parsed_url.hostname, port=parsed_url.port
)
return 'https://{hostname}/submit'.format(hostname=parsed_url.hostname)

if __name__ == '__main__':
app.run(port=7000)
app.run(port=7000, ssl_context='adhoc')

This comment has been minimized.

@zheller

zheller Aug 22, 2014
Contributor

what does this mean?

This comment has been minimized.

@kaushal

kaushal Aug 22, 2014
Author Contributor

http://kracekumar.com/post/54437887454/ssl-for-flask-local-development

It's just local development mode. We use the secret key from line 14

@zheller
Copy link
Contributor

@zheller zheller commented Aug 22, 2014

These should probably be two separate pull requests.

@coveralls
Copy link

@coveralls coveralls commented Aug 22, 2014

Coverage Status

Coverage increased (+12.5%) when pulling dafff2a on coverage into e5904a4 on master.

session['access_token'] = test_auth_token
with Betamax(app.requests_session).use_cassette('me_success'):
response = client.get('/me')
self.assertEquals(response.status_code, 200)

This comment has been minimized.

@zheller

zheller Aug 22, 2014
Contributor

we should assert something about the body

session['access_token'] = test_auth_token
with Betamax(app.requests_session).use_cassette('submit_success'):
response = client.get('/submit?code=%s' % test_auth_code)
self.assertEquals(response.status_code, 200)

This comment has been minimized.

@zheller

zheller Aug 22, 2014
Contributor

we should assert something about the body

This comment has been minimized.

@kaushal

kaushal Aug 22, 2014
Author Contributor

(thumbsup)

@coveralls
Copy link

@coveralls coveralls commented Aug 22, 2014

Coverage Status

Coverage increased (+12.5%) when pulling c7be4a4 on coverage into e5904a4 on master.

kaushal added a commit that referenced this pull request Aug 22, 2014
Improve code coverage by including missing tests, and assertions
@kaushal kaushal merged commit 5c416e8 into master Aug 22, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.