Add additional info to failure messages #29

Merged
merged 1 commit into from Mar 27, 2013

Conversation

Projects
None yet
5 participants
Collaborator

bobsilverberg commented Mar 27, 2013

This addresses issue #28

Contributor

superawesome commented Mar 27, 2013

I'm unqualified to review this, leaving for others

@@ -21,3 +21,11 @@ def _head_request(self, url, user_agent=_user_agent_firefox, locale='en-US', par
def _parse_response(self, content):
return BeautifulSoup(content)
+
+ def response_info_failure_message(self, url, param, response):
@m8ttyB

m8ttyB Mar 27, 2013

Collaborator

What does param correspond to? Is that the expected result?

+
+ def response_info(self, response):
+ url = response.url
+ x_backend_server = response.headers['X-Backend-Server']
@m8ttyB

m8ttyB Mar 27, 2013

Collaborator

perhaps add a code comment about why we're interested in this particular header.

Collaborator

m8ttyB commented Mar 27, 2013

Nice job cleaning up the code! lgtm and purrs (aka runs) like a kitten doing avalanche control work in the high mountains.

py.test -n 4 tests/test_redirects.py --baseurl=http://download.mozilla.org
==================== 273 passed, 2 xfailed in 6.67 seconds =====================

py.test -n 5 tests/test_redirects.py --baseurl=http://download.allizom.org
==================== 272 passed, 3 xfailed in 18.97 seconds ====================

kitten

retornam added a commit that referenced this pull request Mar 27, 2013

Merge pull request #29 from bobsilverberg/moar_info
Add additional info to failure messages

@retornam retornam merged commit e505f0c into mozilla:master Mar 27, 2013

- (url, param, response.url))
- Assert.equal(parsed_url.netloc, urlparse(url).netloc, 'Failed on %s \nUsing %s' % (url, param))
- Assert.equal(parsed_url.query, urlencode(param), 'Failed on %s \nUsing %s' % (url, param))
+ 'Failed by redirected to incorrect scheme %s. \n %s' %
@m8ttyB

m8ttyB Mar 27, 2013

Collaborator

Suggestion:
s/Failed by redirected to incorrect scheme/Failed: redirected to incorrect scheme/

- Redirected to %s' % \
- (response.status_code, url, param, response.url))
- Assert.equal(parsed_url.scheme, 'http', 'Failed on %s \nUsing %s' % (url, param))
+ 'Redirect failed with HTTP status %s. \n %s' %
@m8ttyB

m8ttyB Mar 27, 2013

Collaborator

Suggestion:
s/Redirect failed with HTTP status/Failed: redirect failed with HTTP status/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment