Skip to content
This repository has been archived by the owner on Jun 12, 2023. It is now read-only.

Python 3 compatibility #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dekomote
Copy link

  • 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

- 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
Copy link

@LilyFoote LilyFoote left a 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)

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)

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)

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).

Suggested change
self.failUnless(response.ok)
self.assertTrue(response.ok)

@veryhappythings
Copy link

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants