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

[zooko]

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).

[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.

[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?

[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.

[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.

[zooko]

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

[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.

[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.

[zooko]

result of "./setup.py trial"

[zooko]

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

[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.

[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]

[cgalvan]

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

[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

[zooko]

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

[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...

[zooko]

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

[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.

[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!

[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.

[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 :)

[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.

[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).

[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 :/

[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.

[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.

[cgalvan]

Modify setup.py to use the setuptools_trial plugin

[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 :)

[zooko]

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.

[zooko]

Re-assigning to Chris.

[cgalvan]

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.