#506 closed defect (fixed)

don't attempt to find trial until twisted is installed

Reported by: zooko Owned by: somebody
Priority: critical Milestone: 1.3.0
Component: packaging Version: 1.2.0
Keywords: Cc:
Launchpad Bug:

Description

Currently our Makefile invokes a script named misc/find_trial.py to look for trial on the path and emit an error message if it can't be found. Our setup.py file specifies that twisted is required at setup time, so that trial will be installed. Unfortunately, the former is executed before the latter is! That means that if Twisted is not already installed, then when the user runs "make test" they will get an error message saying that trial couldn't be found. On the other hand if they run make && make test then it will work. (install.html currently instructs the user to run make and then to run make test.

To fix this would probably mean rewriting the invocation of find_trial.py in the Makefile from being Make language which gets executed when the Makefile is executed into being a shell script which gets executed as a command to satisfy a rule (namely the "test" rule).

See also #505 (wanted: a setuptools plugin to make unit tests be executed with trial instead of with pyunit), which would obviate the need for this fix.

Change History (6)

comment:1 Changed at 2008-08-28T20:59:38Z by zooko

  • Milestone changed from undecided to 1.2.1
  • Priority changed from major to critical

Whoops, actually this doesn't work -- when you run make it stops with an error because it tried to find trial and couldn't. We could make it so that the find_trial part of the Makefile proceeds even if trial can't be found. Then make && make test would work again. Or we could do as this ticket suggests: move the find_trial part into a command (in shell), instead of being part of the makefile's execution. This would fix the current failure in which make doesn't work at all (when Twisted isnt' installed) and also allow make test to work by itself without first requiring a make. Or we could do as #505 suggests and make a setuptools plugin, which would also fix it so that make test would work (which would then just be a simple wrapper around ./setup.py test or possibly ./setup.py trial).

Marking this as Priority: critical and Milestone: 1.2.1.

comment:2 Changed at 2008-08-28T22:39:56Z by warner

What is find_trial.py for anyways?

Perhaps we should be content with saying that if you want to run unit tests, you must already have twisted installed in some reasonable way. #505 seems like a noble goal to me, maybe it's ok to require more effort on the part of developers until we get #505 done.

comment:3 Changed at 2008-08-28T23:58:18Z by zooko

find_trial.py is so people can run unit tests without first manually installing twisted. It already works in http://allmydata.org/trac/tahoe/browser/Makefile?annotate=True&tag=allmydata-tahoe-1.2.0#L66 -- the version of our Makefile that shipped in allmydata-tahoe-1.2.0 Makefile in Tahoe-1.2.0, but this patch 960a648d5bc3b720 accidentally broke it by adding the feature of stopping with an error message if trial isn't found. (The fact that this breakage wasn't discovered until today shows that we lack test coverage of the install process -- #434 (automate testing of installation).)

I value minimizing the number of steps of the install process -- http://allmydata.org/source/tahoe/trunk/docs/install.html -- very highly, and I also value users running the unit tests, so I'm glad to say that it will be possible to fix this issue -- if only by reverting that part of 960a648d5bc3b720 -- for allmydata-tahoe-1.3.0.

But I would like to leave it up to cgalvan to choose which of the various ways to fix this issue he wants to do for the 1.3.0 release, if that's okay with you.

P.S. check out that URL -- http://allmydata.org/trac/tahoe/browser/Makefile?annotate=True&tag=allmydata-tahoe-1.2.0#L66 -- isn't that cool? :-)

comment:4 Changed at 2008-09-03T22:55:48Z by warner

I like the 3fab96493ef8802d change. We could even change it to set TRIALCMD=omg-you-seem-to-have-no-trial-please-install-twisted , so that 1) developers don't notice a problem until/unless they attempt to run unit tests, and 2) the problem that shows up ("no such program omg-etc") can guide them to a solution.

I wouldn't mind having the #505 work go into 1.3.0, but I don't want to hold up the release for it. I think 3fab96493ef8802d and/or the omg-etc change would make me happy with the state of #506 in 1.3.0 .

comment:5 Changed at 2008-09-12T05:49:30Z by warner

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

This is now fixed: the new 'setup.py trial' command I just finished adding imports trial and runs it as a subroutine (after sys.path is updated to handle our support/lib/ scheme), instead of invoking trial as a child process.

comment:6 Changed at 2009-03-09T16:34:03Z by zooko

  • Milestone changed from 1.3.1 to 1.3.0

This was fixed long ago -- probably already in tahoe 1.2.0, but definitely already in tahoe 1.3.0.

Note: See TracTickets for help on using tickets.