-
Notifications
You must be signed in to change notification settings - Fork 1
Python 3 compatibility #11
base: master
Are you sure you want to change the base?
Conversation
- Work is heavily dependent on string/binary/unicode so installed and used six - Unittests work in both python 2.7 and 3.7 - Tests needed to be butchered because we're expecting wsgi to return strings(unicode) when in fact, it deals with bytes so comparison can't be made between versions correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall to me. The tests don't appear to be running in circleci - can we resolve that?
self.failUnless(response.ok) | ||
self.assertEqual('@©', page.request(1).body) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we do something like this?
self.assertEqual('@©'.encode('utf-8'), page.request(1).body)
with self.server: | ||
response = self.post('page', 'sample data string in request') | ||
self.failUnless(response.ok) | ||
self.assertEqual('sample data string in request', page.request(1).body) | ||
self.assertEqual('@©', page.content) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've also removed this assert. Can we use encode
here instead?
@@ -23,35 +24,33 @@ def test_get_page(self): | |||
with self.server: | |||
response = self.get('page') | |||
self.failUnless(response.ok) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
failUnless
is a deprecated alias for assertTrue
(https://docs.python.org/2/library/unittest.html#deprecated-aliases).
self.failUnless(response.ok) | |
self.assertTrue(response.ok) |
Circle didn't run this because it's on a fork. I've enabled running for forks for this repo - could you see if you can re-push the branch? Not quite sure what would trigger a build. |
used six
strings(unicode) when in fact, it deals with bytes so comparison can't
be made between versions correctly