Tests on Python 3 #845

Merged
merged 18 commits into from Oct 9, 2011

Conversation

Projects
None yet
3 participants
@takluyver
Member

takluyver commented Oct 8, 2011

This gets the bulk of the test suite passing on Python 3 - there's just a few isolated cases where it still doesn't.

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Oct 8, 2011

Member

I've added context managers for AssertPrints and AssertNotPrints, removing the need for manually switching stdout (or stderr) in each location we need to check that something is being printed.

Member

takluyver commented Oct 8, 2011

I've added context managers for AssertPrints and AssertNotPrints, removing the need for manually switching stdout (or stderr) in each location we need to check that something is being printed.

IPython/core/page.py
@@ -68,6 +68,55 @@ def page_dumb(strng, start=0, screen_lines=25):
last_escape = esc_list[-1]
print >>io.stdout, last_escape + os.linesep.join(screens[-1])
+def _detect_screen_size(use_curses, screen_lines_def):
+ if (TERM=='xterm' or TERM=='xterm-color') and sys.platform != 'sunos5':

This comment has been minimized.

@fperez

fperez Oct 9, 2011

Member

Now that this is a function, it should have a docstring explaining its call signature.

@fperez

fperez Oct 9, 2011

Member

Now that this is a function, it should have a docstring explaining its call signature.

This comment has been minimized.

@fperez

fperez Oct 9, 2011

Member

It should also be possible to add a small test for this guy, if nothing else at least a smoke test that validates its call form. Even simple tests like that do help catch regressions and api changes.

@fperez

fperez Oct 9, 2011

Member

It should also be possible to add a small test for this guy, if nothing else at least a smoke test that validates its call form. Even simple tests like that do help catch regressions and api changes.

This comment has been minimized.

@takluyver

takluyver Oct 9, 2011

Member

I'll do a docstring, but the reason I pulled it out like this was that the code was throwing an exception in the test suite (Python 3 termios didn't seem to like sys.stdout not being an actual OS stream), so I wanted to put a try/except around it. So I'm not sure how we can really test it in isolation.

@takluyver

takluyver Oct 9, 2011

Member

I'll do a docstring, but the reason I pulled it out like this was that the code was throwing an exception in the test suite (Python 3 termios didn't seem to like sys.stdout not being an actual OS stream), so I wanted to put a try/except around it. So I'm not sure how we can really test it in isolation.

This comment has been minimized.

@fperez

fperez Oct 9, 2011

Member

Oh, the test could be just

def test_smoke():
  try:
   call_func(args)
  except ExpectedException:
    pass

at least that will call it and make sure that if we change the signature or a different exception is thrown, we see it.

As I said, it's just smoke testing so trivial to implement, but better than nothing.

@fperez

fperez Oct 9, 2011

Member

Oh, the test could be just

def test_smoke():
  try:
   call_func(args)
  except ExpectedException:
    pass

at least that will call it and make sure that if we change the signature or a different exception is thrown, we see it.

As I said, it's just smoke testing so trivial to implement, but better than nothing.

This comment has been minimized.

@takluyver

takluyver Oct 9, 2011

Member

OK, done one like that.

@takluyver

takluyver Oct 9, 2011

Member

OK, done one like that.

IPython/testing/tools.py
+
+notprinted_msg = """Did not find {0!r} in printed output (on {1}):
+{2!r}"""
+class AssertPrints(object):

This comment has been minimized.

@fperez

fperez Oct 9, 2011

Member

blank lines missing

@fperez

fperez Oct 9, 2011

Member

blank lines missing

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Oct 9, 2011

Member

Other than the small comments above, go for it. The code looks clean, thanks!

Member

fperez commented Oct 9, 2011

Other than the small comments above, go for it. The code looks clean, thanks!

@fperez

This comment has been minimized.

Show comment
Hide comment
@fperez

fperez Oct 9, 2011

Member

Great, ran the test suite on oneiric and all looks good. Merging now; excellent work.

Member

fperez commented Oct 9, 2011

Great, ran the test suite on oneiric and all looks good. Merging now; excellent work.

fperez added a commit that referenced this pull request Oct 9, 2011

Merge pull request #845 from takluyver/py3-tests
This gets the bulk of the test suite passing on Python 3 - there's just a few isolated cases where it still doesn't.

@fperez fperez merged commit 9b734ea into ipython:master Oct 9, 2011

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Oct 9, 2011

Member

Great, thanks Fernando.

Member

takluyver commented Oct 9, 2011

Great, thanks Fernando.

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Oct 12, 2011

Member

@takluyver - why are you disabling our warning mechanism? Have you removed the entire utils.warn module? And if not, why only in utils.path?

Member

minrk commented on 765ba4e Oct 12, 2011

@takluyver - why are you disabling our warning mechanism? Have you removed the entire utils.warn module? And if not, why only in utils.path?

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Oct 12, 2011

Member

I'm not disabling it, just switching to the standard warnings module. utils.warn's docstring says "Shoudn't we just use the built in warnings module." I came upon it because it caused some unicode/bytes friction with capturing stdout into a StringIO in the test suite. I've since resolved that friction another way, but it seemed sensible to switch to the stdlib warnings system while I had the code open.

If we're happy with using the standard library warning system, I'll look round to see where else utils.warn is used. If not, it's easy enough to change this back.

Member

takluyver replied Oct 12, 2011

I'm not disabling it, just switching to the standard warnings module. utils.warn's docstring says "Shoudn't we just use the built in warnings module." I came upon it because it caused some unicode/bytes friction with capturing stdout into a StringIO in the test suite. I've since resolved that friction another way, but it seemed sensible to switch to the stdlib warnings system while I had the code open.

If we're happy with using the standard library warning system, I'll look round to see where else utils.warn is used. If not, it's easy enough to change this back.

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

Merge pull request #845 from takluyver/py3-tests
This gets the bulk of the test suite passing on Python 3 - there's just a few isolated cases where it still doesn't.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment