New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add __init__.py for tests/python directories #4193

Merged
merged 1 commit into from Jan 21, 2017

Conversation

Projects
None yet
3 participants
@peiyuwang
Contributor

peiyuwang commented Jan 20, 2017

We have most of our test directories containing an __init__.py, a few probably we forgot. The consequence of that is pytest expected either all w/ __init__ or all without, when mixed together the behavior is assumed by looking at the first test file, whether the directory it's in contains __init__.py, the assumed behavior may not work for the other, this causes flakiness when input files ordering changes.

As an example, turning on v2 changes the list ordering and sometimes happens to put tests/python/pants_test/invalidation/test_strict_deps_invalidation_integration.py as the first test file to collect, tests/python/pants_test/invalidation/__init__.py does not exist. For the rest tests we see "ImportError: No module named test_export_classpath_integration [1]

The alternative is to remove __init__.py [2] entirely for all tests/python. I tried that, the issue is our tests directories are not organized in 1:1:1 style, those non-test packages under the same directories won't be recognized properly.

So we either adding __init__.py which is what this review does or do the refactoring of test libraries.

[1] https://travis-ci.org/peiyuwang/pants/jobs/193144087#L3859
[2] http://doc.pytest.org/en/latest/goodpractices.html "avoid “init.py” files in your test directories"

@peiyuwang peiyuwang requested review from stuhood, jsirois, benjyw and kwlzn Jan 20, 2017

@kwlzn

kwlzn approved these changes Jan 20, 2017

nice find!

@peiyuwang peiyuwang merged commit 6fd00dc into pantsbuild:master Jan 21, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

lenucksi added a commit to lenucksi/pants that referenced this pull request Apr 25, 2017

Add __init__.py for tests/python directories (#4193)
We have most of our test directories containing an `__init__.py`, a few probably we forgot. The consequence of that is `pytest` expected either all w/ `__init__` or all without, when mixed together the behavior is assumed by looking at the first test file, whether the directory it's in contains `__init__.py`, the assumed behavior may not work for the other, this causes flakiness when input files ordering changes.

As an example, turning on v2 changes the list ordering and sometimes happens to put `tests/python/pants_test/invalidation/test_strict_deps_invalidation_integration.py` as the first test file to collect, `tests/python/pants_test/invalidation/__init__.py` does not exist. For the rest tests we see "ImportError: `No module named test_export_classpath_integration` [1]

The alternative is to remove `__init__.py` [2] entirely for all tests/python. I tried that, the issue is our tests directories are not organized in `1:1:1` style, those non-test packages under the same directories won't be recognized properly.

So we either adding `__init__.py` which is what this review does or do the refactoring of test libraries.

[1] https://travis-ci.org/peiyuwang/pants/jobs/193144087#L3859
[2] http://doc.pytest.org/en/latest/goodpractices.html "avoid “__init__.py” files in your test directories"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment