installing elisa breaks the unit tests of unrelated Python packages

[Zooko Wilcox-O'Hearn]

I maintain a Python application -- http://allmydata.org -- and I had a user report to me that when he runs my unit tests he gets this error message:

    raise DistutilsSetupError("test_suite must be a list")

I thought this was pretty weird, because actually test_suite must be a *string*. Investigation showed that the elisa 0.3.5 installed on my user's Ubuntu Hardy system has reached into the distutils module and overridden its check_test_suite() method with a mutually incompatible requirement! This means, as far as I can tell, that any Python package that uses the distutils test_suite feature cannot run its unit tests when that package is installed on the system.

The work-around is to ask my users to apt-get remove elisa. A better fix would probably be to backport some sort of patch to elisa-0.3.5 to stop this behavior? I would be willing to contribute a bit of time to make a patch which allows either list or string -- that would provide compatibility.

It appears that the current elisa trunk no longer has this behavior in it.

[Zooko Wilcox-O'Hearn (zooko)]

Here is the conversation on the distutils-sig mailing list:

http://mail.python.org/pipermail/distutils-sig/2008-August/009923.html

[Alessandro Decina (alessandro.decina)]

Thanks for looking into this.
Well, the solution would be to upgrade to 0.5 as 0.3 is not supported anymore. Packages are in our PPA.
If that's not an option, we could cook a patch and check with ubuntu if it's possible to push it through
hardy-updates.

[Zooko Wilcox-O'Hearn (zooko)]

Well, we need an Ubuntu expert to weigh in on this. Leaving the package in Ubuntu Hardy, breaking the unit tests of unrelated Python packages, seems pretty crummy. Upgrading elisa to modern version in Ubuntu Hardy backports sounds fine to me, but I'm not an Ubuntu developer, so I don't know how that fits in with their practices. Patching the version (0.3.5) in Hardy to stop breaking unit tests would be fine, and I'm willing to contribute a patch. I know what needs to be changed.

But I would prefer the "Upgrade elisa in Hardy" approach (less work for me, one fewer patch that is in Ubuntu but not in upstream to worry about).

[Loïc Minier (lool)]

Could someone please indicate which exact binary package installs the problematic distutils and where?

Thanks,

[Zooko Wilcox-O'Hearn (zooko)]

Well, I downloaded this orig.tar.gz from the Debian project:

http://packages.debian.org/lenny/elisa

http://ftp.de.debian.org/debian/pool/main/e/elisa/elisa_0.3.5.orig.tar.gz

And ran:

HACM yukyuk:/tmp/tmp$ grep -r "test_suite must be a list" .
./elisa-0.3.5/elisa/core/utils/dist.py: raise DistutilsSetupError("test_suite must be a list")

Then I installed the binary on Ubuntu Hardy (64 bit):

Ah. I see that it is the package "python-elisa":

HACL yukyuk:~/playground/pyutil/pyutil$ dpkg --listfiles python-elisa | xargs grep -A2 -B2 "test_suite must be a list"
/usr/share/pyshared/elisa/core/utils/dist.py-def override_check_test_suite(dist, attr, value):
/usr/share/pyshared/elisa/core/utils/dist.py- if not isinstance(value, list):
/usr/share/pyshared/elisa/core/utils/dist.py: raise DistutilsSetupError("test_suite must be a list")
/usr/share/pyshared/elisa/core/utils/dist.py-
/usr/share/pyshared/elisa/core/utils/dist.py-setuptools.dist.check_test_suite = override_check_test_suite
HACL yukyuk:~/playground/pyutil/pyutil$ apt-cache policy python-elisa
python-elisa:
  Installed: 0.3.5-3
  Candidate: 0.3.5-3
  Version table:
 *** 0.3.5-3 0
        500 http://us.archive.ubuntu.com hardy/main Packages
        100 /var/lib/dpkg/status

A patch that probably fixes this without causing any other problems is:

HACL yukyuk:/usr/share/pyshared/elisa/core/utils$ diff -u dist.py.orig dist.py
--- dist.py.orig 2008-09-02 14:33:18.000000000 -0600
+++ dist.py 2008-09-02 14:33:34.000000000 -0600
@@ -181,9 +181,8 @@
              'clean': Clean, 'sdist': SDist}

 def override_check_test_suite(dist, attr, value):
- if not isinstance(value, list):
- raise DistutilsSetupError("test_suite must be a list")
-
+ pass
+
 setuptools.dist.check_test_suite = override_check_test_suite

[Thomas E Jenkins (thomas-jenkins)]

I would think it would be better to comment out the line that sets setuptools.dist.check_test_suite = override_check_test_suite than to replace the body of the override_check_test_suite function. That would leave the original check_test_suite intact.

[Zooko Wilcox-O'Hearn (zooko)]

That would be an improvement, and it is a very simple change to make, but it might cause the elisa unit-tests to fail to run.

On the other hand, it is perhaps not important for people to run the elisa unit-tests from the elisa-0.3.5 package on Hardy. In any case, it is much more important that the package stops preventing unrelated Python projects from running their unit tests.

So I would not object to that way of fixing it either.

Note that my proposed patch of leaving the override in place but changing the new function to always pass would (I think) cause both elisa unit-tests and all other unit-tests to run okay. The drawback of it is just that it means you don't get a proper error message if you pass an incorrect type, such as "test_suite = 1" (an integer).

Oh, but probably my patch would need to be extended to make it so that the elisa-installed Test Command accepts both lists and strings. I know how to write such a patch, but if people would accept the simpler proposal by mort then I would be just as happy with that.

[Thomas E Jenkins (thomas-jenkins)]

I looked into why elisa specific code was breaking the build of unrelated python packages. Elisa advertises a 'egg_info.writers' entry point of 'elisa_infos.txt = elisa.core.utils.dist:write_dict' which means elisa.core.utils.dist is imported every time someone calls setup.py bdist_egg. And because the setuptools.dist.check_test_suite = override_check_test_suite is done at the module level in elisa.core.utils.dist it will be evaluated for every bdist_egg.

The correct fix probably involves moving the override of setuptools.dist.check_test_suite elsewhere in the elisa build but another fix would be to pull write_dict out of elisa.core.utils.dist into another module that doesn't mess with setuptools internals.

This problem exists in Intrepid as well.

[Philippe Normand (philn)]

Thanks mort for investigating on this. So this is relevant in 0.5 as well and should be fixed ASAP.

Anyway in elisa.core we always had a single test suite so I think we can remove that supid override hack.

[Zooko Wilcox-O'Hearn (zooko)]

That's right -- elisa never actually used this feature of having a list of test suite strings instead of a single string, so they (elisa) can remove the override hack and change their test_suite= parameter to be just the string. That same patch could be easily backported to elisa 0.3.5 and deployed into Intrepid and Hardy. This would definitely fix all problems both for running the elisa tests themselves and for the tests of other python packages.

[Zooko Wilcox-O'Hearn (zooko)]

I examined this patch, and it looks right: http://bazaar.launchpad.net/~philn/elisa/elisa_fix_test_suite/revision/678?start_revid=678

Please apply it to the 0.3 branch of Elisa. As a reminder, the reason why anyone cares about a simple issue like this in an ancient release like elisa 0.3 is that it break the ability of *other* unrelated packages to run their own unit tests!

[Zooko Wilcox-O'Hearn (zooko)]

P.S. The reason I'm noticing this bug again is that I'm working on packaging a new version of my "zbase32" package, and I have to keep this ugly workaround code in my setup.py to work-around this elisa-0.3.5 bug. (Even though zbase32 has nothing to do with elisa.) This work-around code is currently getting even more complicated because it interacts with another packaging feature I'm trying to add. Of course, other people who don't know about this and don't maintain this workaround in their setup.py will just have a bizarre exception when they try to run their own unit tests.

[Olivier Tilloy (osomon)]

I just adapted the patch to the latest development branch and sent it for review.

@Loïc: can we backport the patch to elisa 0.3.5 in hardy as explained by Zooko, and to elisa 0.5.9 in intrepid (and actually include the patch in elisa 0.5.28 in jaunty too)?

[Zooko Wilcox-O'Hearn (zooko)]

This issue just bit a user of Tahoe-LAFS on Intrepid:

http://allmydata.org/trac/tahoe/ticket/805

[zooko (zooko-tahoe-trac)]

This is a bug in Elisa in Intrepid: https://bugs.launchpad.net/ubuntu/+source/elisa/+bug/263697

[zooko (zooko-tahoe-trac)]

(The work-around is to uninstall Elisa.)

[Alex Valavanis (valavanisalex)]

Intrepid Ibex reached end-of-life on 30 April 2010 so I am closing the
report. The bug is still marked as confirmed in later versions of Ubuntu.

[Zooko Wilcox-O'Hearn (zooko)]

Okay, if we're not going to backport a fix for this into Hardy, then let's close this ticket so everyone knows that we're not going to, and so there are fewer open tickets. I just attempted to set the status of this bug in hardy to "wontfix", but apparently I lack the privilege of setting it to that setting. Could someone give me that privilege please? (Or else just set it yourself...)

[Olivier Tilloy (osomon)]

I can’t set it to "Won’t Fix" either, this status is disabled for Hardy.
Alternatively, I could set it to "Invalid", but that wouldn’t reflect correctly the actual status.

[dino99 (9d9)]

The latest free moovida 1.09 does not get any maintenance since a while. Now moovidadb.com is supporting Linux and support can be found at : http://www.fluendo.com/faq/