#505 closed enhancement (fixed)

wanted: a setuptools plugin to make unit tests be executed with trial instead of with pyunit

Reported by: zooko Owned by: cgalvan
Priority: major Milestone: undecided
Component: packaging Version: 1.2.0
Keywords: Cc:
Launchpad Bug:

Description

Currently we use a script misc/find_trial.py to find a trial executable and then the Makefile executes it. It would be nicer if ./setup.py test would use trial instead of pyunit to run tests. See also #179 (rewrite our Makefile in Python instead of GNUmake).

Attachments (6)

zooko._setup_py.log.txt (74.2 KB) - added by zooko at 2008-08-31T02:31:17Z.
result of "./setup.py trial"
zooko.make_test_.log.txt (56.1 KB) - added by zooko at 2008-08-31T02:32:39Z.
result of "make test" (everything passes in this file, not in the "./setup.py test" file)
twistd_exe.patch (1.0 KB) - added by cgalvan at 2008-09-01T22:32:42Z.
Patch to fall back on finding twisted executables in source/bin.
zooko._setup_py.log.2.txt (39.7 KB) - added by zooko at 2008-09-02T21:47:36Z.
stderr and stdout from running "./setup.py trial -s allmydata.test.test_runner.RunNode?.test_client"
twistd.log (2.5 KB) - added by zooko at 2008-09-02T21:50:16Z.
_trial_temp/test_runner/RunNode/test_client/c1/logs/twistd.log
use_setuptools_trial.patch (93.1 KB) - added by cgalvan at 2008-11-21T21:08:28Z.
Modify setup.py to use the setuptools_trial plugin

Download all attachments as: .zip

Change History (31)

comment:1 Changed at 2008-08-28T22:45:50Z by warner

I'm ok with this as long as I can still exercise as much control over the test process as I have now. That means:

  • which tests get run
  • add things to PYTHONPATH that will override any automatically acquired local dependencies (specifically alternate versions of foolscap and twisted)
  • make it run quickly, with no spurious rebuilding of dependencies or slow regeneration of version numbers

Something like ./setup.py test allmydata.test.test_mutable, or PYTHONPATH=~/my-foolscap ./setup.py test.

If/when this gets in, I'll change the Makefile so that 'make test' is just an alias for ./setup.py test .

One question is how much automatic-dependency-management this step ought to do. I recommend doing nothing.

comment:2 Changed at 2008-08-29T02:31:29Z by zooko

Our Makefile currently has the test target compute dependencies first and the quicktest target which just executes the test. We added the latter because measurements showed that it took on the order of 5 seconds to compute the dependencies, and you (Brian) have a very fast edit-test cycle which cannot stand a 5-second delay in the middle of it. We preserved the version that also computed the dependencies because new users who just run make test need dependencies built and because I don't like the weird version skew issues if I pull patches which change the dependencies and then I run tests afterward without first running make.

Would a similar pair of "quick test" and "dependency-building test" commands in the setuptools plugin satisfy you?

comment:3 Changed at 2008-08-29T19:04:53Z by warner

Yes. Maybe something like "setup.py test --no-deps" for the quick version.. once I get it into my command history (or I add it to a Makefile alias), I'll be ok.

The argument to control which tests get run is important too, that'll probably just be the remaining sys.argv elements after the options are pulled out.

comment:4 Changed at 2008-08-30T03:32:32Z by cgalvan

  • Owner changed from somebody to cgalvan

comment:5 Changed at 2008-08-30T20:55:35Z by cgalvan

I have a plugin hosted here: http://code.google.com/p/trialtest/

It currently adds a 'trial' command to distutils, which is like 'test' only it uses trial instead of pyunit. It doesn't appear that I can replace the existing 'test' command using a plugin. If we want 'setup.py test' to use the trial functionality, then we'll just have to override it in tahoe's setup.py with a cmdclass.

comment:6 Changed at 2008-08-31T02:00:22Z by zooko

How do we add to tahoe's setup.py a cmdclass to make it use trialtest for the test command?

comment:7 Changed at 2008-08-31T02:26:29Z by zooko

Okay now we've got to make sure that all of Brian's requirements are met.

First of all, how can we make it not resolve dependencies when running tests? This might be tricky. Actually Brian's requirement isn't that it leaves dependencies unresolved, but rather his requirement is that it launches the tests very quickly -- something like ... let's see how fast make quicktest TEST=allmydata.test.test_base62.T.test_ende_0x00 currently works on my machine...

Hm. Let's say we have a budget of 800 milliseconds to launch the test. I don't believe it will be possible to do the setuptools dependency checking and still meet that budget, which means we need to have a --no-deps option.

comment:8 Changed at 2008-08-31T02:30:44Z by zooko

Cool! So it sort of works! Here's what I get here, attached.

The problems I have is probably because ./setup.py trial doesn't set the PYTHONPATH and the PATH the way make test does.

Changed at 2008-08-31T02:31:17Z by zooko

result of "./setup.py trial"

Changed at 2008-08-31T02:32:39Z by zooko

result of "make test" (everything passes in this file, not in the "./setup.py test" file)

comment:9 Changed at 2008-08-31T17:36:44Z by cgalvan

The problem with those tests failing was not with the PATH's not being set, but because the reactor wasn't being set to poll. I reproduced this by running:

./trial --rterrors allmydata  - which yields the extra errors
./trial --rterrors --reactor=poll allmydata  - which runs just fine

So I am currently working on adding an option to the TrialTest? command to choose the reactor, but I haven't yet figured out how to pass this option through the trial script itself.

comment:10 Changed at 2008-08-31T18:16:27Z by zooko

No, I'm pretty sure the problem on my machine is that somehow it is trying to invoke "trial" and "trial" is not on the path. Here's the excerpt from my zooko._setup_py.log.txt:

    test_baddir ...                                                        [OK]
    test_client ... Can't find twistd (it comes with Twisted).  Aborting.
                                                    [ERROR]
    test_introducer ... Can't find twistd (it comes with Twisted).  Aborting.
                                                [ERROR]

Changed at 2008-09-01T22:32:42Z by cgalvan

Patch to fall back on finding twisted executables in source/bin.

comment:11 Changed at 2008-09-02T21:46:40Z by zooko

Okay, after applying the twistd_exe.patch, I realized that our setup.py is not setup_require'ing twisted as it needs to do in order to make this ticket work. Commenting-in that setup_require's line yields the following result from ./setup.py trial:

[FAIL]: allmydata.test.test_client.Basic.test_versions

Traceback (most recent call last):
  File "/home/zooko/playground/allmydata/tahoe/trunk-pristine/src/allmydata/test/test_client.py", line 130, in test_versions
    self.failIfEqual(str(allmydata.__version__), "unknown")
twisted.trial.unittest.FailTest: 'unknown' == 'unknown'
===============================================================================
[FAIL]: allmydata.test.test_runner.RunNode.test_client

Traceback (most recent call last):
  File "/home/zooko/playground/allmydata/tahoe/trunk-pristine/src/allmydata/test/test_runner.py", line 224, in _start
    self.failUnlessEqual(rc, 0, errstr)
twisted.trial.unittest.FailTest: rc=1, OUT: '', ERR: 'client node probably not started
'
===============================================================================
[FAIL]: allmydata.test.test_runner.RunNode.test_introducer

Traceback (most recent call last):
  File "/home/zooko/playground/allmydata/tahoe/trunk-pristine/src/allmydata/test/test_runner.py", line 129, in _start
    self.failUnlessEqual(rc, 0, errstr)
twisted.trial.unittest.FailTest: rc=1, OUT: '', ERR: 'introducer node probably not started
'
-------------------------------------------------------------------------------
Ran 414 tests in 200.428s

FAILED (expectedFailures=2, failures=3, successes=409)

So then I re-ran it like this to run just one of the failing tests:

./setup.py trial -s allmydata.test.test_runner.RunNode.test_client

The complete stdout and stderr from this will be attached to this ticket, and here is the main result:

Running 1 tests.
allmydata.test.test_runner
  RunNode
    test_client ... 
Failed to load application: No module named allmydata
                                                     [FAIL]

===============================================================================
[FAIL]: allmydata.test.test_runner.RunNode.test_client

Traceback (most recent call last):
  File "/home/zooko/playground/allmydata/tahoe/trunk-pristine/src/allmydata/test/test_runner.py", line 224, in _start
    self.failUnlessEqual(rc, 0, errstr)
twisted.trial.unittest.FailTest: rc=1, OUT: '', ERR: 'client node probably not started
'
-------------------------------------------------------------------------------
Ran 1 tests in 0.454s

FAILED (failures=1)

real    4m17.077s
user    3m17.598s
sys     0m19.316s

Changed at 2008-09-02T21:47:36Z by zooko

stderr and stdout from running "./setup.py trial -s allmydata.test.test_runner.RunNode?.test_client"

comment:12 Changed at 2008-09-02T21:49:53Z by zooko

Here is an excerpt from the log file produced by the test node in this test:

2008/09/02 15:47 -0600 [-] Loading tahoe-client.tac...
2008/09/02 15:47 -0600 [-] Traceback (most recent call last):
2008/09/02 15:47 -0600 [-]   File "/usr/lib/python2.5/site-packages/twisted/application/app.py", line 218, in getApplication
2008/09/02 15:47 -0600 [-]     application = service.loadApplication(filename, style, passphrase)
2008/09/02 15:47 -0600 [-]   File "/usr/lib/python2.5/site-packages/twisted/application/service.py", line 341, in loadApplication
2008/09/02 15:47 -0600 [-]     application = sob.loadValueFromFile(filename, 'application', passphrase)
2008/09/02 15:47 -0600 [-]   File "/usr/lib/python2.5/site-packages/twisted/persisted/sob.py", line 215, in loadValueFromFile
2008/09/02 15:47 -0600 [-]     exec fileObj in d, d
2008/09/02 15:47 -0600 [-]   File "tahoe-client.tac", line 4, in <module>
2008/09/02 15:47 -0600 [-]     from allmydata import client
2008/09/02 15:47 -0600 [-] ImportError: No module named allmydata

the full log of the test node (which is named "_trial_temp/test_runner/RunNode/test_client/c1/logs/twistd.log") will be attached...

Changed at 2008-09-02T21:50:16Z by zooko

_trial_temp/test_runner/RunNode/test_client/c1/logs/twistd.log

comment:13 Changed at 2008-09-02T22:03:02Z by zooko

I don't know how we are going to accomplish the ./setup.py trial --no-deps option -- my guess is that it requires some help from setuptools itself. Fortunately, it occurs to me that Brian (who requires no-deps) can keep using the makefile's quicktest target, which will not go through the setuptools_trial plugin at all, but will continue doing its current thing (invoking trial directly).

I don't want Brian's method of invoking tests to diverge from other people's in the long term, so I would want to make progress on the ./setup.py trial --no-deps option in parallel, possibly by submitting a patch to setuptools if need be.

comment:14 Changed at 2008-09-08T02:50:11Z by warner

sounds good to me. The extra routine in setup.py will manage sys.path and locate trial, right?

If so I'll apply this in a few days.

thanks!

comment:15 Changed at 2008-09-08T12:39:02Z by zooko

It occurs to me that the most recent patch from Chris -- twistd_exe.patch -- finds the twisted executables and adds their directory to the sys.path, but does not also add them to the os.environ['PATH']. That means that subprocesses won't have that on their path. However, it doesn't seem like that could explain the error I get: ImportError: No module named allmydata.

comment:16 Changed at 2008-09-09T00:07:14Z by warner

zooko: are the transcripts you are attaching the result of using Chris' patch? Or his plugin? Or are they the result of some other change to setup.py? You say "It sort of works" but I'm not sure which "It" you're referring to :)

comment:17 Changed at 2008-09-09T12:09:09Z by zooko

The most recent transcripts I posted are the result of using Chris's plugin and his find_twistd.exe patch, and editing our setup.py to have "twisted" in its setup_requires parameter. Note that this is a second attempt after the earlier attempt in which I wrote "it sort of works" -- Chris write the find_twistd.exe patch and I applied it in order to get past the problem of that earlier attempt -- lack of twistd on the PATH.

comment:18 Changed at 2008-09-10T23:41:22Z by warner

ok, I'm going to see if there's a way to copy Chris's plugin into our setup.py, as well as including the find_twistd.exe patch. Tell me if I'm wrong, but I *think* this will result in a simpler solution than trying to incorporate a setuptools plugin (I don't know how setuptools plugins work).

comment:19 Changed at 2008-09-11T04:47:22Z by cgalvan

Using the setuptools plugin actually makes it simpler and reusable. There are other projects who are also interested in being able to use this plugin to run their unit tests with trial. BTW, I ended up being busier this week than I thought, so I probably won't be able to help out until Friday :/

comment:20 Changed at 2008-09-12T05:46:42Z by warner

In a fit of activity, I pushed a bunch of changes that took care of this. Specifically, it added a 'setup.py trial' command, which interacts with a number of other tahoe-specific changes that I made to simplify our Makefile drastically.

This probably isn't general enough to satisfy the original goals of this ticket, but it makes Tahoe work much better (all of the shell-quoting from the Makefile is gone, replaced by python code in setup.py). The code from setup.py could probably be extracted into a separate plugin, but I'm not sure it's worth it.

The code is here: http://allmydata.org/trac/tahoe/browser/setup.py?rev=77d7f63e39d5968d#L316 .

  • the --reactor -adding code is tahoe-specific (well, it is a workaround to a bug that is probably invisible to folks outside the tahoe/foolscap world)
  • the sys.path/PYTHONPATH/where-the-heck-is-twistd functionality depends upon other changes that I made ([here http://allmydata.org/trac/tahoe/browser/setup.py?rev=77d7f63e39d5968d#L23]), so it probably couldn't be used in a project which depends upon setuptools-driven automatic dependency building without copying some of that code over too

That said, I love to see code reused, so I'd like to hear everybody's thoughts on turning this stuff into a plugin of some sort.

comment:21 Changed at 2008-09-17T17:18:35Z by zooko

So Chris Galvan has already made a setuptools plugin which mostly works. It sounds like if he just examines your patch and copies into his plugin any parts of your patch which are sufficiently non-Tahoe-specific, then we can probably switch to using his plugin.

Changed at 2008-11-21T21:08:28Z by cgalvan

Modify setup.py to use the setuptools_trial plugin

comment:22 Changed at 2008-11-21T21:11:26Z by cgalvan

I have attached a patch that modifies the setup.py so that it uses the setuptools_trial plugin. The plugin itself allows you to pass different arguments such as reporter, reactor, etc..., but in tahoe's setup.py I sub-class the class in the plugin so that we can set certain arguments to default(i.e. set the reactor to poll on certain platforms).

  • Note: This patch should not be applied until the setuptools_trial package has been uploaded to PyPi?, which will probably be done today once Zooko can apply my other patch :)

comment:23 Changed at 2008-11-25T20:01:49Z by zooko

  • Owner changed from cgalvan to warner

Thanks for the patch, Chris! We might want to bundle setuptools_trial in which the Tahoe slim distribution, the way we do with setuptools_darcs and darcsver.

Brian said he would have a look at this soon.

comment:24 Changed at 2009-01-14T20:37:02Z by zooko

  • Owner changed from warner to cgalvan

Re-assigning to Chris.

comment:25 Changed at 2009-01-22T21:55:13Z by cgalvan

  • Resolution set to fixed
  • Status changed from new to closed

We have been using the setuptools_trial plugin for awhile now and seems to be working pretty well, so I'm going to close this ticket.

Note: See TracTickets for help on using tickets.