Change conftest.py to default to python_library rather than python_tests#10603
Merged
Eric-Arellano merged 2 commits intopantsbuild:masterfrom Aug 13, 2020
Merged
Change conftest.py to default to python_library rather than python_tests#10603Eric-Arellano merged 2 commits intopantsbuild:masterfrom
conftest.py to default to python_library rather than python_tests#10603Eric-Arellano merged 2 commits intopantsbuild:masterfrom
Conversation
…on_tests` # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
benjyw
approved these changes
Aug 13, 2020
Eric-Arellano
added a commit
to Eric-Arellano/pants
that referenced
this pull request
Aug 15, 2020
…an `python_tests` (pantsbuild#10603)" This reverts commit 255f9e0. [ci skip-rust] [ci skip-build-wheels]
Eric-Arellano
added a commit
to Eric-Arellano/pants
that referenced
this pull request
Aug 15, 2020
…s` again # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
Eric-Arellano
added a commit
that referenced
this pull request
Aug 15, 2020
…10619) In #10603, we moved `conftest.py` from `python_tests` to `python_library`. This is because Pants now runs file-at-a-time for Pytest thanks to the model change in #10511. This causes `conftest.py` to error because it doesn't have any test files. We decided in #10603 that `conftest.py` should be under `python_library` because it is not actual tests. For example, test util code goes under a `python_library` target already. However, `python_library` owning a `conftest.py` by default ended up being problematic, as it caused test code to be mixed with production code, which is generally not desired. See #10613 for an approach to fix this. Instead of adding a `conftest` target, this PR instead moves back `conftest.py` to `python_tests` and special cases `pytest_runner.py` to skip `conftest.py`. Even though this is less correct, it's simpler for users and it avoids making 1.x users having to change a bunch of things. Conceptually, `conftest.py` can be seen as "config" for tests, rather than traditional "test utils" like you'd have in a `python_library`. [ci skip-rust]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
conftest.pyis (typically) a util file with no tests in it. Now that we always run tests one-file-at-a-time thanks to generated subtargets, this causes Pytest to try to run onconftest.py, which fails because there are no tests.More philosophically, test util code has always meant to go into
python_library, rather thanpython_tests. A target is "metadata for some files" - it doesn't make sense to say something liketimeout=120onconftest.py.Solution
Change default source globs so that
conftest.pywill show up inpython_library()now.Result
Pytest no longer will error if you have a
conftest.pyand were using default source globs.If you already had a
python_library()target in the directory where theconftest.pywas defined—and you have--python-infer-conftestsenabled (the default)—then you will not need to make any changes for this to work.If you only had a
python_tests()target—and you have--python-infer-conftests—you simply need to addpython_library(name='conftest')to your BUILD file.If you only had a
python_teststarget—and do not have--python-infer-conftests—you will need to addpython_library(name='conftest')and also update yourpython_tests()to explicitly depend on that new target.For new users, their BUILD file will now look something like this in a folder that solely has tests +
conftest.py:[ci skip-rust]
[ci skip-build-wheels]