Stop installing the test suite with setup.py install #255

Merged
merged 3 commits into from Jul 14, 2011

Conversation

Projects
None yet
4 participants
Contributor

gholms commented Jul 12, 2011

''setup.py install'' currently installs the test suite in site-packages/tests, which conflicts with other Python modules that do that as well, probably incorrectly so. The test suite can run out of the source tree, but AFAIK ''setup.py install'' doesn't need to install it on target systems. This commit makes setup.py stop doing that.

Contributor

jtriley commented Jul 12, 2011

Better yet, we should probably move the tests package to boto.test. This removes the problem given that the tests will then be under boto's namespace and also has the benefit that tests can be executed interactively. We could also add a convenience method "test()" to boto/init.py for easy interactive testing.

Contributor

gholms commented Jul 13, 2011

If we want to go down that path instead then I took a first stab at it in my mvtestsuite branch with commits gholms@6c6644b and gholms@ad51f91 . What do you folks think?

Owner

garnaat commented Jul 13, 2011

Hmm. The tests used to be under boto.tests. I'm sure they were moved for a reason but I can't remember what it could have been. Seems to make much more sense to have them as a package under boto.

Contributor

jtriley commented Jul 13, 2011

@gholms: Looks good to me. Seems to work fine but I haven't tried actually running the tests with credentials yet. IMHO I'd say close this request and open another from your mvtestsuite branch but @garnaat should make the final call.

Contributor

jtriley commented Jul 13, 2011

@gholms: p.s. also added some minor tweaks to your mvtestsuite branch. I just sent you a pull request for the changes (gholms#1)

Contributor

gholms commented Jul 13, 2011

Looks good. I sent a new pull request [0] that instead moves the test suite to boto.testsuite and boto.test. If you prefer that solution then just close this request instead. ;-)

[0] #257

Owner

garnaat commented Jul 13, 2011

I'm confused by this. Many of the Python repos on github that I have checked (e.g. Django, Flask, pinax, etc.) place their test directory at the top-level just as boto does. The documentation for distutils is not totally clear on this but it certainly states that it will, by default, look for a tests directory at the top-level.

So, what exactly is the issue and why are we changing it? Is there some precedent that you can point me at to explain why this approach is better or more "standard"?

Contributor

jtriley commented Jul 13, 2011

The original issue was that boto's setup.py had "tests.*" in the package list which caused distutils/setuptools to install boto's tests outside of its namespace (ie $PYTHON/site-packages/tests) which is not in general where tests should be installed. Bottom line is if tests are going to be installed for a package they should really live under the given package's namespace which is where pull request #257 comes in.

If we don't care about installing the tests along with boto, then we could certainly just leave the tests directory where it is and only remove the "test.*" entries from the package list in setup.py as is the case in this pull request. However, IMO it is nice to include the tests along with a given package so that it's easy to run the suite interactively and on demand in addition to the standard "python setup.py test". Both NumPy and SciPy projects do this for convenient testing:

import numpy
numpy.test()

import scipy
scipy.test()

I'm also doing this in StarCluster...

I don't have strong feelings about this one way or the other; it really just boils down to whether we want to install the tests with boto or not. If so, pull request #257 solves that. If not, this pull request (#255) will take care not to install the tests in their current form when boto is installed.

Contributor

jtriley commented Jul 13, 2011

@garnaat: also, can you point me to the docs for distutils you're talking about with the "tests" directory? All I can find is that it will include things like tests/test*.py when packaging a source distribution. This isn't needed with #257 given that the tests will remain in the package list in setup.py and therefore be included with the source package anyway. AFAIK distutils isn't really test-aware and doesn't support things like "python setup.py test"; only setuptools does via a "test_suite" kwarg in setup() which has been changed to boto.testsuite in #257

Owner

garnaat commented Jul 13, 2011

@jtriley: I was just referring to the mention of the tests directory being included by default. Perhaps I gave that more weight than it deserves.

I really don't have a strong opinion on this either. I'm just aware of the fact that the tests directory once resided within the package itself and people lobbied to move it to the top-level and now we are talking about moving it back. I guess the bottom line is there is no "right" way to do this.

Contributor

jtriley commented Jul 13, 2011

@garnaat I'm interested in reading/hearing the arguments for moving tests outside of the boto package in the past. I have yet to see a downside and it's nice to be able to run the tests using the installed package rather than having to download and unpack the original source. I tried searching the old google groups posts but so far I haven't found anything...

At the very least merging #255 is worth while since it doesn't change anything but prevents boto from incorrectly installing its tests into a "tests" module in the user's site-packages directory.

@kopertop kopertop added a commit that referenced this pull request Jul 14, 2011

@kopertop kopertop Merge pull request #255 from gholms/master
Stop installing the test suite with setup.py install
5260b7d

@kopertop kopertop merged commit 5260b7d into boto:master Jul 14, 2011

@msabramo msabramo pushed a commit to msabramo/boto that referenced this pull request Nov 28, 2012

@kopertop kopertop Merge pull request #255 from gholms/master
Stop installing the test suite with setup.py install
692da21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment