Merge lp://staging/~nejucomo/divmod.org/fix-nevow-setup into lp://staging/divmod.org

This would be an easier change to accept if it included automated test coverage.

> This would be an easier change to accept if it included automated test
> coverage.

What would constitute an acceptable testing strategy? Note that in order to run tests you have to successfully build, in which case this code was necessarily executed and returned some version (not necessarily the correct one, admittedly). I suppose you can unit-test the function in {{{setup.py}}} though. Or, you could check that the version returned by {{{pkg_resources}}} is the same as the one you get by importing {{{nevow._version}}} (remember to skip the test if {{{pkg_resources}}} is not importable).

I think testing the function in setup.py would probably be sufficient in this scenario.

Where would the automated test code live?

If it's inside Nevow proper, does that mean it would import setup.py in a unit test module?

Should we move the parsing code inside of Nevow, then import that in setup.py? That difficult without re-introducing the problem this patch is trying to solve. It would also be in danger of regressing https://bugs.launchpad.net/nevow/+bug/812537.

Should the setup.py module itself could have unittests?

All of these options sound ridiculous to me. Is there another approach I've missed?

It seems like the right way to test this is to have package infrastructure tests somewhere that test installing packages (and then running the unittests or other verification on the installation).

Does Nevow have such a system? If not, I can try one of the approaches above, and I'd appreciate guidance on which is the least insane.

There's no problem with the tests living inside of nevow. The *code* has to live in setup.py to avoid regressing, but there's no problem with importing setup.py as a module to run some functions in it. Why does that seem ridiculous to you?

> There's no problem with the tests living inside of nevow. The *code* has to
> live in setup.py to avoid regressing, but there's no problem with importing
> setup.py as a module to run some functions in it. Why does that seem
> ridiculous to you?

Perhaps "ridiculous" is too strong. It depends on the expected use case of unittests. Importing "setup" is a problem if the unittests are intended to run from the installed code.

I tend to prefer unittests which install with the application code, such that at any time one may run the test suite for an installed package as a sanity check. For example: "pip install $PKG; trial $PKG"

If, OTOH, we decide unittests can only be run from a revision control checkout (what about unpacked sdists?), then this would not be an issue, except in so far as future maintainers don't realize this policy and violate it.

A middle ground is to have "application" unittests which are installed with the package and can be run on the installed package, and have separate "packaging" unittests.

I think I prefer this "middle ground" approach. Unless someone raises an issue with it, next chance I'll take this approach:

Add a new single python module called something like "nevow_setup_unittests.py" which does not get installed, and place the test code in there, then perhaps tweak "runtests" to pass that to trial. (I'm assuming "runtests" is only for development and is unrelated to the installation.)

I prefer this separate module approach versus a conditional import inside Nevow *application* unittest code because it seems to easy to import the *wrong* setup.py.

I'm curious what other packages have done to address this issue. I know that Tahoe-LAFS, for example has an extremely complex setup.py and importing it has all manner of side effects. I wouldn't be surprised if it modifies sys.path outside of setuptools behavior.

The approach there has been to test all packaging code with test code which lives outside the repository and is currently messily spread about some buildbot configs.

I'd like to see the packaging test code which is specific to a revision live within the repository, yet be separate from application unittest code.

BTW- as a followup, I just created a fresh virtualenv and ran "pip install twisted && pip install nevow && trial nevow".

The twisted install prior to the nevow install is required because of the ticket this branch attempts to address.

The result is that everything and unittests ran [1], so I believe that importing setup.py in the unittests would be changing a policy, whether that policy is explicit or not.

[1] Unfortunately the test result counts were:

FAILED (skips=28, expectedFailures=2, failures=1, errors=12, successes=711)

Attaching your test output and log somewhere would possibly be helpful, since Nevow doesn't have any CI right now...

I have no idea about the tests etc but I can at least verify the approach will fix the issues I see with Nevow's setup.py.

I am dusting this off again, in order to write unittests for the setup.py changes.

Before I begin changing tests, I ran `trial nevow` in the source root directory *without a nevow* install, and see the following error:

{{{
nevow.test.test_url.Serialization.test_rfc1808
===============================================================================
[ERROR]
Traceback (most recent call last):
Failure: twisted.internet.utils._UnexpectedErrorOutput: got stderr: 'Could not create the Java virtual machine.\n'
}}}

Ah, I misread the error output. It was unrelated to `nevow.test.test_url.Serialization.test_rfc1808` but instead occurred in `nevow.test.test_consolejstest.JSGenerationTestCase.test_generateTestScript`.

I'm now investigating if a js interpreter is needed, and how it is detected. I have node on my $PATH, but it was not detected...

> Ah, I misread the error output. It was unrelated to
> `nevow.test.test_url.Serialization.test_rfc1808` but instead occurred in
> `nevow.test.test_consolejstest.JSGenerationTestCase.test_generateTestScript`.
>
> I'm now investigating if a js interpreter is needed, and how it is detected.
> I have node on my $PATH, but it was not detected...

I would guess it is looking for a JS implementation written in Java, e.g. Rhino. But that's unrelated to this ticket.

Currently the JavaScript tests require the SpiderMonkey interpreter, installed as `js` or `smjs` in your $PATH (iirc). I'm guessing you have another JS intepreter installed as `js`, which is a) not working (producing the "Could not create the Java virtual machine" error), and b) not SpiderMonkey, thus the code that drives it will most likely fail to run the tests anyway.

It's probably not that important to be able to run the JavaScript tests in this case; if SpiderMonkey can't be found, the tests would normally be skipped anyway.

I've added a comment on https://bugs.launchpad.net/nevow/+bug/812537 saying that this issue is causing Tahoe-LAFS users to be unable to install it.

My intention is now to move Nevow to github (and therefore git) and use Brian Warner's python-versioneer project to remove the need for this Python parsing regexp in order to get the version information in setup.py.