Skip to content

Conversation

@dhermes
Copy link
Contributor

@dhermes dhermes commented Oct 5, 2016

This took a lot of wrangling, but eventually I found a solution: make unit_tests a namespace package and then collect from site-packages. This way we only run py.test once instead of 15 times, a huge speedup.

In order to do this I needed to do some renames to avoid collisions (e.g. unit_tests.test_client would have been installed for every sub-package).

The first 3 (of 4 of 5) commits are for the rename. The first two are automated (i.e. I didn't make changes to any files myself) and the third is just a tiny lint cleanup based on the automated changes.

Also relevant: https://testrun.org/tox/latest/example/pytest.html#known-issues-and-limitations

To avoid collision had to move most tests into a uniquely
named directory. For example the tests in bigquery/unit_tests needed
to move to bigquery/unit_tests/bigquery to avoid any name collision.

Done via: https://gist.github.com/dhermes/397c731966674da1b42612b723e64895
Done via:

$ git grep -l 'from unit_tests._fixtures import' -- vision |
> xargs sed -i s/'from unit_tests._fixtures import'/'from unit_tests.vision._fixtures import'/g
$
$ git grep -l 'from unit_tests._fixtures import' -- speech |
> xargs sed -i s/'from unit_tests._fixtures import'/'from unit_tests.speech._fixtures import'/g
$
$ git grep -l 'from unit_tests._testing import ' -- bigtable |
> xargs sed -i s/'from unit_tests._testing import '/'from unit_tests.bigtable._testing import '/g
This requires manually collecting from the installed
site-packages.

See: https://testrun.org/tox/latest/example/pytest.html
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 5, 2016
@dhermes
Copy link
Contributor Author

dhermes commented Oct 5, 2016

@tseaver The Travis failure is due to #2486. The change needed to filter for files that exist.

candidates, _ = get_affected_files()
python_files = [
    candidate for candidate in candidates 
    if candidate.endswith('.py') and os.path.exists(candidate)]

@daspecster
Copy link
Contributor

unit_tests/handlers/transports/__init__.py → bigquery/unit_tests/bigquery/__init__.py, that doesn't seem right does it?

python_files = [
candidate for candidate in candidates if candidate.endswith('.py')]
candidate for candidate in candidates
if candidate.endswith('.py') and os.path.exists(candidate)]

This comment was marked as spam.

This comment was marked as spam.

@tseaver
Copy link
Contributor

tseaver commented Oct 5, 2016

I'm grinding my teeth about the issues caused from moving the unit tests out-of-tree: I still think they belong there, which would remove the need to mess with the unittests/ pseudo-package at all.

@daspecster
Copy link
Contributor

So from a selfish standpoint, I like having the unit_tests for a package in the package. It makes for less mental tracking while working on things. Might just be me though.

@dhermes
Copy link
Contributor Author

dhermes commented Oct 6, 2016

@daspecster RE:

unit_tests/handlers/transports/__init__.py → bigquery/unit_tests/bigquery/__init__.py, that doesn't seem right does it?

That is just git not really caring where the file went. The __init__.py files are all identical

@dhermes
Copy link
Contributor Author

dhermes commented Oct 6, 2016

Chatted with @tseaver and @jonparrott about this and will be putting the unit tests back.

@dhermes dhermes closed this Oct 6, 2016
@dhermes dhermes deleted the move-tests-side-by-side branch October 6, 2016 16:00
@dhermes dhermes restored the move-tests-side-by-side branch October 6, 2016 20:50
@dhermes dhermes reopened this Oct 6, 2016
@dhermes dhermes closed this Oct 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement. testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants