Our method of running the test suite (iptest) is broken

Bug #362142 reported by Brian Granger
2
Affects Status Importance Assigned to Milestone
IPython
Fix Released
Critical
Fernando Perez

Bug Description

Currently we run the IPython test suite using a custom command line program "iptest" that calls nosetests with the right options. The problem with this is that the entire approach is very fragile. A couple of problems exist:

* The integration with twisted.trial seems to be buggy at best. But, we need to use twisted.trial to test things in kernel and frontend.
* We are getting lots of odd errors/failures that are impossible to track down.
* Different tests have very different and at times conflicting dependencies. For instance, we have code that uses wx, qt, twisted, etc. All of these things should not be tested in the same process.

We need a new way of running tests that:

* Run's certain tests in subprocesses.
* Isolates tests based on dependencies.
* Runs twisted using tests using trial.
* Allow packages/modules to declared their deps without being imported so it can be decided how they will be run.

The difficulty with this is that our test dependencies are not well isolated into package or even modules. A perfect example of this is that frontend.asyncfrontendbase uses twisted. Other tests on frontend could easily use wx and twisted. Another problem is that the twisted using parts of our code can't use nose things like decorators or the additional testing syntax that nose supports. Argh!

Changed in ipython:
assignee: nobody → ellisonbg
importance: Undecided → Critical
milestone: none → 0.10
status: New → Confirmed
Revision history for this message
Brian Granger (ellisonbg) wrote :

As a start we are doing the following:

* For all modules that nose should skip because twisted is needed, we have defined a __test__ = {} attribute.

* For all modules that use twisted, we no longer protect the twisted import using a try/except. The assumption is that because these
tests use trial to be run, that twisted must be installed.

* We have written new trial compatible skip decorators in testing/decorators_trial.py. These have an API just like those in decorators.py

Revision history for this message
Brian Granger (ellisonbg) wrote :

We have a new version of iptest that works for now. In 0.11 we need to have a better solution.

Changed in ipython:
assignee: ellisonbg → fdo.perez
milestone: 0.10 → 0.11
status: Confirmed → Triaged
Revision history for this message
Fernando Perez (fdo.perez) wrote :

I'm not closing this one yet because I still think we can do better during 0.11, but things as of the 0.10 release are actually pretty good. We don't get a unified report (that's what I want to fix) but at least we have proper isolation, dependency handling and graceful fallback when dependencies are missing.

Changed in ipython:
status: Triaged → In Progress
Revision history for this message
Fernando Perez (fdo.perez) wrote :

With recent work in my trunk-dev branch, pretty much all the points in the original report are dealt with. The solutions aren't super-clean, and we still have lots of room for improvement on iptest (mainly its options handling and verbosity), but by and large the core problems reported above are fixed.

Brian, what do you think? We're never going to get off of the nose/twisted pair of tools because twisted.trial is very limited in critical areas compared to nose, yet needed for managing the reactor. So unless twisted.trial were to massively improve (extensible doctest management, full test discovery of non-unittest tests, etc), it's not a realistic option.

I'm inclined to close this bug now, leaving the remaining iptest problems to their own bug reports so we can act on them with more specificity.

OK with that?

Revision history for this message
Brian Granger (ellisonbg) wrote : Re: [Bug 362142] Re: Our method of running the test suite (iptest) is broken
Download full text (5.0 KiB)

> With recent work in my trunk-dev branch, pretty much all the points in
> the original report are dealt with.  The solutions aren't super-clean,
> and we still have lots of room for improvement on iptest (mainly its
> options handling and verbosity), but by and large the core problems
> reported above are fixed.

Yes, I think some of the original issues have been addresses, but not
all of them.
But we should probably create, new more focused tickets about these
isssues. These
issues may overlap with other iptest tickets you have created...

Issue 1
======

It is still super easy for a single developer to break the test
suite by making their new module depend on something (twisted, wx, etc.).
For that dev, they always have the dep. installed, so they never see
the failure, so all their
new tests will run fine. This is particularly insidious (still) for
twisted because iptest does not
have a clean way of declaring test dependencies and run methods for
individual files.
We are trying to do this by subpackage, but that is quite inflexible -
what if only a single
file in a subpackage depends on twisted - then the rest of the
subpackage can't be tested
using nose.

We need a simple, declarative API for "declaring" the testing related
information on a per subpackage or per
module basis. This includes:

* Dependencies
* Method of running the test (nose+iptest/trial)
* Thing like "test this subpackage this way, but these two modules are
different" We run into this with frontend, which
uses twisted in a few places, but not everywhere.

I don't think this needs to be in place for 0.11, but because some of
these issues are quite subtle, I don't want us to forget
them. Thus, I think it makes sense to keep a ticket around for this.

Issue 2
======

Our test suite is still quite fragile and difficult to debug when
things go wrong (case in point, your recent bug report about
the history being dumped to the screen). If we can get the dpendency
issues worked out (to isolate twisted and nose)
I think most of this fragility is localized to the nose extension that
handles the one time IPython shell. I think our test
infrastructure has to be completely independent of our main IPython code
(the testing infrastucture should never do "from IPython import ...").
 At some point, we figured out how we could enable
the writing of IPython doctests without the nose extension (do you
remember how this can be done?). As I remember we talked
about using this idea along with pexpect to isolate the running
IPython to another process.

Can we keep a ticket for this and fill in some of the details that we
talked about. I honestly don't remember what
solution we came up with, but it was somehow related to the new
parametric testing you came up with.

Again, I don't think this is 0.11 critical.

> Brian, what do you think?  We're never going to get off of the
> nose/twisted pair of tools because twisted.trial is very limited in
> critical areas compared to nose, yet needed for managing the reactor.
> So unless twisted.trial were to massively improve (extensible doctest
> management, full test discovery of non-unittest tests, etc), it's not a
> realistic option.
>
> I'm in...

Read more...

Revision history for this message
Jörgen Stenarson (jorgen-stenarson) wrote : Re: [Bug 362142] Re: Our method of running the test suite (iptest) is broken

Brian Granger skrev 2010-01-17 20:48:
> remember how this can be done?). As I remember we talked
> about using this idea along with pexpect to isolate the running
> IPython to another process.
>
I think using pexpect requires some research if you want to keep the the
tests running on windows.

/Jörgen

Revision history for this message
Brian Granger (ellisonbg) wrote : Re: [Bug 362142] Re: Our method of running the test suite (iptest) is broken

Yes, this is a huge problem - pexpect doesn't work on Windows...

We should check with the Sage folks, I know at one point, they were working on
a windows port of pexpect called wexpect.

Twisted does have many of the same capabilities that pexpect has for
working with
external processes. That might be our best bet.

Cheers,

Brian

On Sun, Jan 17, 2010 at 3:10 PM, Jörgen Stenarson
<email address hidden> wrote:
> Brian Granger skrev 2010-01-17 20:48:
>> remember how this can be done?).  As I remember we talked
>> about using this idea along with pexpect to isolate the running
>> IPython to another process.
>>
> I think using pexpect requires some research if you want to keep the the
> tests running on windows.
>
> /Jörgen
>
> --
> Our method of running the test suite (iptest) is broken
> https://bugs.launchpad.net/bugs/362142
> You received this bug notification because you are a member of IPython
> Developers, which is subscribed to IPython.
>
> Status in IPython - Enhanced Interactive Python: In Progress
>
> Bug description:
> Currently we run the IPython test suite using a custom command line program "iptest" that calls nosetests with the right options.  The problem with this is that the entire approach is very fragile.  A couple of problems exist:
>
> * The integration with twisted.trial seems to be buggy at best.  But, we need to use twisted.trial to test things in kernel and frontend.
> * We are getting lots of odd errors/failures that are impossible to track down.
> * Different tests have very different and at times conflicting dependencies.  For instance, we have code that uses wx, qt, twisted, etc.  All of these things should not be tested in the same process.
>
> We need a new way of running tests that:
>
> * Run's certain tests in subprocesses.
> * Isolates tests based on dependencies.
> * Runs twisted using tests using trial.
> * Allow packages/modules to declared their deps without being imported so it can be decided how they will be run.
>
> The difficulty with this is that our test dependencies are not well isolated into package or even modules.  A perfect example of this is that frontend.asyncfrontendbase uses twisted.  Other tests on frontend could easily use wx and twisted.  Another problem is that the twisted using parts of our code can't use nose things like decorators or the additional testing syntax that nose supports.  Argh!
>
>
>

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

Revision history for this message
Fernando Perez (fdo.perez) wrote :

I've closed it as 'fix released' just so it's easy to filter it out, and in a sense we have released solutions, just that they are unsatisfactory.

I've made two new more specific reports with the new points brought up here:

https://bugs.launchpad.net/ipython/+bug/508970
https://bugs.launchpad.net/ipython/+bug/508971

so we can continue to track each topic better.

Changed in ipython:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.