TST: add future unicode_literals test (#786) #790

Merged
merged 3 commits into from Sep 15, 2011

Conversation

Projects
None yet
2 participants
@olivierverdier
Contributor

olivierverdier commented Sep 14, 2011

add simple test to check that the bug #786 is indeed fixed in master

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Sep 14, 2011

Member

I'm not quite sure if this is reliably cross-platform. On Windows, \xe9 may be equivalent to the single byte \xe9 (although due to limitations in Python, it may not be). Can we use an isinstance test instead of len?

Also, we're aiming for Python 3 compatibility for 0.12, so we need to be careful with this test. We probably need to add a skipif_python3 decorator for this. Have a look at IPython.testing.decorators (or I can add that, if you prefer).

Member

takluyver commented Sep 14, 2011

I'm not quite sure if this is reliably cross-platform. On Windows, \xe9 may be equivalent to the single byte \xe9 (although due to limitations in Python, it may not be). Can we use an isinstance test instead of len?

Also, we're aiming for Python 3 compatibility for 0.12, so we need to be careful with this test. We probably need to add a skipif_python3 decorator for this. Have a look at IPython.testing.decorators (or I can add that, if you prefer).

TST: simpler check for unicode literals (#790)
check using isinstance instead for the length of the string
@olivierverdier

This comment has been minimized.

Show comment
Hide comment
@olivierverdier

olivierverdier Sep 15, 2011

Contributor

Ok, I used isinstance instead of length, which is indeed more elegant. I'm not sure how to skip the test in Python 3, so I let you do that.

Contributor

olivierverdier commented Sep 15, 2011

Ok, I used isinstance instead of length, which is indeed more elegant. I'm not sure how to skip the test in Python 3, so I let you do that.

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Sep 15, 2011

Member

Great, thanks, that's definitely neater. In fact, I think it would now pass after conversion to Python 3, but I'll probably add a decorator to skip it anyway.

One minor niggle: the second argument to assert is the message that shows up if the assertion fails, and you've worded it as a description of what happens if it passes. I think it makes more sense for the message to say what went wrong, e.g.

assert isinstance(ip.user_ns['unicode_str'], unicode), 'strings literals are not being interpreted as unicode'
Member

takluyver commented Sep 15, 2011

Great, thanks, that's definitely neater. In fact, I think it would now pass after conversion to Python 3, but I'll probably add a decorator to skip it anyway.

One minor niggle: the second argument to assert is the message that shows up if the assertion fails, and you've worded it as a description of what happens if it passes. I think it makes more sense for the message to say what went wrong, e.g.

assert isinstance(ip.user_ns['unicode_str'], unicode), 'strings literals are not being interpreted as unicode'
TST: remove assert messages (#790)
remove ambiguous assert messages for unicode_literals test
@olivierverdier

This comment has been minimized.

Show comment
Hide comment
@olivierverdier

olivierverdier Sep 15, 2011

Contributor

ok, you're right about the assert message. Since I find it a bit awkward, I remove them altogether.

Contributor

olivierverdier commented Sep 15, 2011

ok, you're right about the assert message. Since I find it a bit awkward, I remove them altogether.

takluyver added a commit that referenced this pull request Sep 15, 2011

Merge pull request #790 from olivierverdier/future_unicode_test
TST: add future unicode_literals test (#786)

@takluyver takluyver merged commit 54e67c3 into ipython:master Sep 15, 2011

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Sep 15, 2011

Member

Excellent. Thanks, Olivier - it's merged.

Member

takluyver commented Sep 15, 2011

Excellent. Thanks, Olivier - it's merged.

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

TST: simpler check for unicode literals (#790)
check using isinstance instead for the length of the string

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

TST: remove assert messages (#790)
remove ambiguous assert messages for unicode_literals test

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Merge pull request #790 from olivierverdier/future_unicode_test
TST: add future unicode_literals test (#786)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment