IPython - Enhanced Interactive Python

Merge lp:~villemvainio/ipython/ip0-unittests-1 into lp:ipython/0.11

Proposed by Ville M. Vainio on 2008-09-18
Status: Rejected
Rejected by: Ville M. Vainio on 2009-08-01
Proposed branch: lp:~villemvainio/ipython/ip0-unittests-1
Merge into: lp:ipython/0.11
To merge this branch: bzr merge lp:~villemvainio/ipython/ip0-unittests-1
Reviewer Review Type Date Requested Status
Fernando Perez Needs Fixing on 2009-08-01
Brian Granger Needs Fixing on 2008-11-20
Review via email: mp+1075@code.launchpad.net
To post a comment you must log in.
1158. By Ville M. Vainio on 2008-09-18

add test_magiclist

1159. By Ville M. Vainio on 2008-09-18

added inspection magic tests

1160. By Ville M. Vainio on 2008-09-18

add a few trivial tests

Brian Granger (ellisonbg) wrote :

Overall these changes look fine. It is great for us to begin moving these tests into IPython/tests where they will be run regularly by nose.

However, when I run iptest with this branch, I get a failure and some noise to stdout. This stuff should be cleaned up before we merge. Once these things are fixed, this can be merged.

brian-grangers-macbook:~ bgranger$ iptest
..........S.....................................................................................................................................................................................................................................................................................................................................................................................................................S....S>f2("a b c")
>f1("a", "b", "c")
>f1(1,2,3)
>f2(4)
.------------------------------------------------------------
Traceback (most recent call last):
  File "<ipython console>", line 1, in <module>
  File "<ipython console>", line 9, in test
AssertionError

F........... os.path.isdir(path)
.....
======================================================================
FAIL: %cd magic, %bookmark
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/bgranger/Library/Python/2.5/site-packages/nose-0.10.3-py2.5.egg/nose/case.py", line 182, in runTest
    self.test(*self.arg)
  File "/Users/bgranger/Documents/Computation/IPython/branches/ip0-unittests-1/IPython/tests/test_cd.py", line 35, in test_cd
    assert _ip.user_ns['success'] == 1
AssertionError:
>> assert _ip.user_ns['success'] == 1

-------------------- >> begin captured stdout << ---------------------
/
/private/tmp

--------------------- >> end captured stdout << ----------------------

----------------------------------------------------------------------
Ran 440 tests in 58.122s

FAILED (SKIP=3, failures=1)

review: Needs Fixing
1161. By Ville M. Vainio on 2008-11-26

test_cd: do not attempt if /tmp or /usr are symlinks

Ville M. Vainio (villemvainio) wrote :

You probably have /tmp as symlink. I now skip test_cd if /tmp or /usr are symlinks, can you test it?

Brian Granger (ellisonbg) wrote :

Yes, tmp on OS X is symlinked to private/tmp. I will retest shortly.

Brian

On Wed, Nov 26, 2008 at 9:30 AM, Ville M. Vainio <email address hidden> wrote:
> You probably have /tmp as symlink. I now skip test_cd if /tmp or /usr are symlinks, can you test it?
> --
> https://code.launchpad.net/~villemvainio/ipython/ip0-unittests-1/+merge/1075
> You are subscribed to branch lp:ipython.
>

--
Brian E. Granger, Ph.D.
Assistant Professor of Physics
Cal Poly State University, San Luis Obispo
<email address hidden>
<email address hidden>

Brian Granger (ellisonbg) wrote :

The latest change now solves this problem and all tests pass. However, I am still getting the noise on stdout (see my previous comment). We definitely need to track this down and get rid of it.

Ville M. Vainio (villemvainio) wrote :

On Sun, Nov 30, 2008 at 6:09 AM, Brian Granger <email address hidden> wrote:

> The latest change now solves this problem and all tests pass. However, I am still getting the noise on stdout (see my previous comment). We definitely need to track this down and get rid of it.
> --

I didn't think it matters - the test case does output to stdout by
design, since it tests interactive stuff (autocall conversion). I
don't think it makes sense to redirect stdout here, since nose already
does it.

Obviously, it can be done, especially since it's the only test that
does it so far - but yet it's also among the first tests that
excercises interactive stuff, and I don't think we should set a
precedent about not allowing stdout for tests.

--
Ville M. Vainio
http://tinyurl.com/vainio

Brian Granger (ellisonbg) wrote :

Ville,

> I didn't think it matters - the test case does output to stdout by
> design, since it tests interactive stuff (autocall conversion). I
> don't think it makes sense to redirect stdout here, since nose already
> does it.

What test is failing, I want to have a look.

I don't think we want to get into the habit of allowing tests to write
to stdout...Ideally we will be adding more and more tests of the
interactive stuff, and if more of them do this, our test suite will be
a huge mess when it gets run.

We now actually have quite a few tests and it is really important to
leave no doubts in folks mind what tests are passing and which are
failing. When I see random noise (that doesn't even say what test it
is associated with), things *look* broken, even if there are not. If
we write more tests that have noise on stdout, it will be very
difficult to sort out what is going on. So, I am still not conviced
we want to allow this type of thing.

But, I would like to have a look at this particular test, so if you
could points me in the right direction, that would be great.

> Obviously, it can be done, especially since it's the only test that
> does it so far - but yet it's also among the first tests that
> excercises interactive stuff, and I don't think we should set a
> precedent about not allowing stdout for tests.

I would like to know Fernando's thoughts on this - maybe there is a
way that we can have nose handle this stuff automatically. I would be
fine with that. Fernando would know if this is possible and could
help us implement it.

Cheers,

Brian

> --
> Ville M. Vainio
> http://tinyurl.com/vainio
> https://code.launchpad.net/~villemvainio/ipython/ip0-unittests-1/+merge/1075
> You are subscribed to branch lp:ipython.
>

--
Brian E. Granger, Ph.D.
Assistant Professor of Physics
Cal Poly State University, San Luis Obispo
<email address hidden>
<email address hidden>

Fernando Perez (fdo.perez) wrote :
Download full text (3.2 KiB)

Howdy,

On Sun, Nov 30, 2008 at 4:46 PM, Brian Granger <email address hidden> wrote:
> Ville,
>
>> I didn't think it matters - the test case does output to stdout by
>> design, since it tests interactive stuff (autocall conversion). I
>> don't think it makes sense to redirect stdout here, since nose already
>> does it.
>
> What test is failing, I want to have a look.
>
> I don't think we want to get into the habit of allowing tests to write
> to stdout...Ideally we will be adding more and more tests of the
> interactive stuff, and if more of them do this, our test suite will be
> a huge mess when it gets run.
>
> We now actually have quite a few tests and it is really important to
> leave no doubts in folks mind what tests are passing and which are
> failing. When I see random noise (that doesn't even say what test it
> is associated with), things *look* broken, even if there are not. If
> we write more tests that have noise on stdout, it will be very
> difficult to sort out what is going on. So, I am still not conviced
> we want to allow this type of thing.
>
> But, I would like to have a look at this particular test, so if you
> could points me in the right direction, that would be great.
>
>> Obviously, it can be done, especially since it's the only test that
>> does it so far - but yet it's also among the first tests that
>> excercises interactive stuff, and I don't think we should set a
>> precedent about not allowing stdout for tests.
>
> I would like to know Fernando's thoughts on this - maybe there is a
> way that we can have nose handle this stuff automatically. I would be
> fine with that. Fernando would know if this is possible and could
> help us implement it.

What's strange about those particular tests is that somehow they are
getting to stdout even after nose's capturing. I have plenty of
tests already in my branch that do interactive things but you only see
their output if you run nose with -s, while in this branch there's
still some output. Consider:

- Trunk:
bic128[~]> iptest
----------------------------------------------------------------------
Ran 420 tests in 29.305s

OK (SKIP=4)

- Ville's branch:
bic128[~]> iptest
>f2("a b c")
>f1("a", "b", "c")
>f1(1,2,3)
>f2(4)
 os.path.isdir(path)
----------------------------------------------------------------------
Ran 434 tests in 31.652s

OK (SKIP=4)

I fully agree with Brian that, at least when run *without* -s, the
test suite should be silent. Scipy got into the habit of having a
hideously noisy test suite from the start (years ago) and now people
are going through a lot of pain trying to clean it up. Let's not start
down the wrong path ourselves.

There's another problem: in this branch, '-v' or '-s' currently don't
work for iptest:

bic128[~]> iptest -v
WARNING:
Error in Arguments: "Illegal option '-v'"

bic128[~]> iptest -s
[... snip ipython help screen... ]
WARNING:
Error in Arguments: "Ambiguous option '-s'; matches ['screen_length',
'sl', 'separate_in', 'si', 'separate_out', 'so', 'separate_out2',
'so2', 'system_verbose']"

Since this works in trunk, it's likely just missing merges from trunk
on this branch.

I'll continue looking at this branch until we get i...

Read more...

Fernando Perez (fdo.perez) wrote :

See my comments in the reply thread to Brian's.

review: Needs Fixing
Fernando Perez (fdo.perez) wrote :

Ville, any news on this one after our feedback?

Ville M. Vainio (villemvainio) wrote :

On Sun, Mar 15, 2009 at 4:09 AM, Fernando Perez <email address hidden> wrote:

> Ville, any news on this one after our feedback?

Nope. I haven't tried with your trunk-dev branch though.

--
Ville M. Vainio
http://tinyurl.com/vainio

Fernando Perez (fdo.perez) wrote :

On Sun, Mar 15, 2009 at 2:03 AM, Ville M. Vainio <email address hidden> wrote:
> On Sun, Mar 15, 2009 at 4:09 AM, Fernando Perez <email address hidden> wrote:
>
>> Ville, any news on this one after our feedback?
>
> Nope. I haven't tried with your trunk-dev branch though.

OK. One option would be to have a look at my work and merge that in
first, then merge that trunk back into yours and see again. Since
much of what I did improves the testing support, it might make things
easier for you.

But for that, I need someone to review my changes. Wink, wink :)

f

Brian Granger (ellisonbg) wrote :

Any further progress on this?

Fernando Perez (fdo.perez) wrote :

Just a reminder (I left it also in the other branch): merge with trunk before continuing, since so much has happened there.

Fernando Perez (fdo.perez) wrote :

Ville, can you update to trunk and see what status this is in? If it's in reasonable shape, we should go ahead and try to get it ready for merging, but it seems you haven't pulled any of the trunk into this in a while.

Ville M. Vainio (villemvainio) wrote :

I merged from trunk (conflicts were trivial). However, now I get:

In [8]: !./iptest
SS.............SSSSSSSSS.S..S......
----------------------------------------------------------------------
Ran 35 tests in 0.635s

OK (SKIP=13)

What's up with this? Didn't this run a zillion tests before?

1162. By Ville M. Vainio <ville@debian> on 2009-04-15

merge from trunk, resolve conflicts

Fernando Perez (fdo.perez) wrote :

On Wed, Apr 15, 2009 at 2:52 AM, Ville M. Vainio <email address hidden> wrote:
> I merged from trunk (conflicts were trivial). However, now I get:
>
> In [8]: !./iptest
> SS.............SSSSSSSSS.S..S......
> ----------------------------------------------------------------------
> Ran 35 tests in 0.635s
>
> OK (SKIP=13)
>
> What's up with this? Didn't this run a zillion tests before?
> --
> https://code.launchpad.net/~villemvainio/ipython/ip0-unittests-1/+merge/1075
> You are reviewing the proposed merge of lp:~villemvainio/ipython/ip0-unittests-1 into lp:ipython.
>

I wonder if this is related to the problem Jorgen is seeing on the
list... Could you try with -vvs to get more info?

Cheers,

f

Fernando Perez (fdo.perez) wrote :

I'm up to date on this branch, but I see other testing problems. Using

iptest

there's a crash on output (with full ipython crash report), and using

iptest -vvs

I get

WARNING:
Error in Arguments: "Illegal option '-vvs'"

This doesn't happen on trunk for me.

Fernando Perez (fdo.perez) wrote :
Download full text (9.6 KiB)

There has been no progress on this, so we can't for now merge this branch. The comments we made above are still unresolved, and in particular, the full test suite reports errors in IPython.tests, and running

 iptest IPython.tests

produces this on my system:

uqbar[junk]> iptest IPython.tests
Could not open file <IPython.tests> for safe execution.
>f2("a b c")
>f1("a", "b", "c")
>f1(1,2,3)
>f2(4)
 os.path.isdir(path)
ERROR: File `refbug.py` not found.
ERROR: File `refbug.py` not found.
ERROR: File `tclass.py` not found.
ERROR: File `tclass.py` not found.
ERROR: File `tclass.py` not found.
======================================================================
ERROR: Check that %run doesn't damage __builtins__
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py", line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/testing/decorators.py", line 171, in skipper_func
    return f(*args, **kwargs)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_magic.py", line 261, in test_builtins_id
    bid1 = id(_ip.user_ns['__builtins__'])
KeyError: '__builtins__'

======================================================================
FAIL: Test that prompts correctly generate after %run
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/local/lib/python2.5/site-packages/nose-0.10.4-py2.5.egg/nose/case.py", line 182, in runTest
    self.test(*self.arg)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/testing/decorators.py", line 171, in skipper_func
    return f(*args, **kwargs)
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_magic.py", line 284, in test_prompts
    nt.assert_equals(p2[:3], '...')
AssertionError: '\x01\x1b[' != '...'
>> raise self.failureException, \
          (None or '%r != %r' % ('\x01\x1b[', '...'))

======================================================================
FAIL: Doctest: IPython.tests.test_magic.doctest_hist_f
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/testing/plugin/ipdoctest.py", line 430, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for IPython.tests.test_magic.doctest_hist_f
  File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_magic.py", line 71, in doctest_hist_f

----------------------------------------------------------------------
File "/home/fperez/usr/lib/python2.5/site-packages/IPython/tests/test_magic.py", line 78, in IPython.tests.test_magic.doctest_hist_f
Failed example:
    _ip.magic("hist -n -f $tfile 3")
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python2.5/doctest.py", line 1228, in __run
        compileflags, 1) in test.globs
      File "<doctest IPython.tests.test_magic.doctest_hist_f[2]>", line 1, in <module>
        _i...

Read more...

review: Needs Fixing
Ville M. Vainio (villemvainio) wrote :

On Sat, Aug 1, 2009 at 3:31 AM, Fernando Perez<email address hidden> wrote:

> Review: Needs Fixing
> There has been no progress on this, so we can't for now merge this branch.  The comments we made above are still unresolved, and in particular, the full test suite reports errors in IPython.tests, and running

Yeah, this one had decayed badly & not attended at all.

I removed the merge request and abandoned the branch.

--
Ville M. Vainio
http://tinyurl.com/vainio

Fernando Perez (fdo.perez) wrote :

On Sat, Aug 1, 2009 at 12:18 AM, Ville M. Vainio<email address hidden> wrote:
> Yeah, this one had decayed badly & not attended at all.
>
> I removed the merge request and abandoned the branch.

OK, thanks!

Unmerged revisions

1162. By Ville M. Vainio <ville@debian> on 2009-04-15

merge from trunk, resolve conflicts

1161. By Ville M. Vainio on 2008-11-26

test_cd: do not attempt if /tmp or /usr are symlinks

1160. By Ville M. Vainio on 2008-09-18

add a few trivial tests

1159. By Ville M. Vainio on 2008-09-18

added inspection magic tests

1158. By Ville M. Vainio on 2008-09-18

add test_magiclist

1157. By Ville M. Vainio on 2008-09-18

rm sys.path manipulation from ipapi test

1156. By Ville M. Vainio on 2008-09-18

rm sys.path manipulation from wildcard test case

1155. By Ville M. Vainio on 2008-09-18

move test_wildcard to nose tests

1154. By Ville M. Vainio on 2008-09-18

add test_completer and test_ipapi to nose tests

1153. By Ville M. Vainio on 2008-09-18

fix test_autocall.py

Subscribers

People subscribed via source and target branches