Moving unit tests into dedicated unit_tests/ directory

[dhermes]

Also moving two _testing.py modules and updating the relevant imports.

As pre-work, removed all unit test imports (i.e. unit tests that imported from another unit test).

See https://gist.github.com/dhermes/a36abffe9d921755d5566c2cb3930be2 for the script I used to generate the 2nd commit. (No code was manually edited, all done by the script.)

[dhermes]

@jonparrott What changes do I need to make to setup.py / MANIFEST.in to make sure Debian package maintainers are happy but also unit_tests/ doesn't end up installed in site-packages?

[theacodes]

@dhermes just add:

graft unit_tests

to MANIFEST.in.

When doing an sdist, setuptools will include that directory in the tar.gz. sdists still use setup.py, so even though the folder is there, it won't be included unless find_packages finds it, which it shouldn't. If it does, use the second argument to ignore it: find_pacakages('.', exclude=('unit_tests')). Note the bdists (including wheels) won't include this folder. This is intentional.

Also, you probably could've kept the situations where tests import other tests by making unit_tests it's own python package (that's just not installed).

[dhermes]

Also, you probably could've kept the situations where tests import other tests by making unit_tests it's own python package (that's just not installed).

That was mostly for good testing hygiene

[theacodes]

SGTM.

[theacodes]

This LGTM.

[tseaver]

I really dislike moving the tests out of line: I believe they should be present in the installed software, on the "fly what you test, test what you fly" principle.

[theacodes]

@tseaver I understand your concern, but it seems that several other major Python packages have tests in a separate directory. We're still providing them in the source distribution.

[dhermes]

@tseaver The "responsible" thing to do is to move to the best-in-class and actively maintained test-runner, rather than staying on the dead nose project.

I'm not entirely sure what "fly what you test, test what you fly" has to do with the location of the tests, or sure why it would matter where the tests exist, rather it should matter that they exist.

I don't have any interest in writing a test-runner framework. Provided we're using someone else's, we should do what they support. I can modify my script to go with the second option, which is a dedicated test/ dir (without an __init__.py) in each subpackage.

WDYT?

[tseaver]

The "fly what you test" implies shipping the tests with all the distributions, which means not moving them out of the package directory.

I'm -0 about switching to py.test: I can live with that a lot more easily than moving the tests out-of-line.

[theacodes]

@tseaver I can't think of a case where it makes a lot of sense for a binary distribution (wheel) to include tests, especially if we explicitly structure tests to be able to run against the installed library. The clear separation of the tests package from the installed package makes this more obvious - the tester doesn't have to wonder what's being tested.

That is to say, if there is a test at a/test_a.py that tests a.a, it's ambiguous (especially with nose) if the tests are exercising ./a/a.py or {env}/lib/python{x}.{y}/site-packages/a/a.py. By moving the tests to their own package, it's always explicit that the package-under-test is in site-packages.

[daspecster]

Sorry, I'm also confused about how the "Fly as you test, test as you fly" principle applies in this case?
ISTM that means having and adding systems tests as we build which to me would be our CI?

That is to say, if there is a test at a/test_a.py that tests a.a, it's ambiguous (especially with nose) if the tests are exercising ./a/a.py or {env}/lib/python{x}.{y}/site-packages/a/a.py.

In all honesty, I've been burned a couple times by exactly what @jonparrott pointed out there, so I'm game for trying out solutions.

[tseaver]

I want users who have installed a wheel to be able to test it without the sdist, e.g.:

  $ /path/to/virtualenv/bin/pip install google-cloud
  $ /path/to/virtualenv/bin/nosetests google.cloud

(or the py.test equvalent). This allows them to verify the installation is correct on their platform. Today, this works (assuming you can navigate borken dependencies yourself):

$ /path/to/virtualenv/bin/pip install googleapis-common-proto==1.2.0 # borken
$ /path/to/virtualenv/bin/pip install nose unittests2 gcloud
$ /path/to/virtualenv/bin/nosetests gcloud
$ /tmp/issue_2227/bin/nosetests gcloud
.................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................SSSSSSSSSS....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 2207 tests in 0.546s

OK (SKIP=10)

[theacodes]

@tseaver basically no other package does this. If that is a strong desire for the user then we should make it easy for them to use the tests in our source tree against an installed version (which is what all the other major packages I linked do), which is what I'm recommending we do.

[tseaver]

@jonparrott AYKM? I can point to hundreds which ship the tests inline in the source package.

[jgeewax]

Putting my preferences aside, it looks to me like we have two possible scenarios in the future:

1. Some standard body says "thou shalt put your tests in site-packages"
2. Some standard body says "thou shalt not put your tests in site-packages"

If we presume (1) and (2) happens instead, following the standard would mean that we break people who assume they can still type nosetests google.cloud. If we presume (2) and (1) happens instead, we add a feature to meet the standard...

Considering that we both have examples to point to, and a vote today among this group would end up with (2, don't ship tests), and it's the more future-proof option, it seems like we should just not put tests in site-packages and put it down as work to do should a standard be ratified.

Cool?

[dhermes]

@tseaver I'm going with consensus here and merging. We can revisit / we can always put these back. I just don't want to block the other work I'm doing to move to py.test / namespace changes.