Add a conftest target type#10613
Conversation
# 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]
I'm not sure I follow this logic. Especially since we are describing this change as "basically sugar", it feels to me like a macro would map more specifically to my mental model of this. Do we expect to be writing rules at some point that have to specifically process |
The issue is that a macro defined in Instead, here, we make sure that We will also be able to document in https://www.pantsbuild.org/v2.0/docs/python-test-goal about using a
Unlikely. But that was an explicit design goal for the Target API. You can create new target types that fit your domain and still have those be understood by Pants with zero changes. For example, some repos might want to create a |
|
This goes too far in favor of purity over ergonomics. Closing in favor of #10619. Turns out, |
…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]
Problem
Some Python repositories (like ours) colocate source files with tests.
In #10603, we stopped having
conftest.pydefault to apython_teststarget and to instead default topython_library. This was a good conceptual change, and also necessary now that we always run tests one-file-at-a-time.However, #10603 does not work well in repositories where tests are colocated with source files. Previously, the
python_librarywould solely refer to production code, but now it's including a test config file without users realizing it. This means, for example, that test's inferred dependencies will be included in thepython_library(), so, whenever a user depends on:library(rather than file dep), they'll pull in more than they expected.For example, in Toolchain's Django repo, we have to explicitly declare several dynamic dependencies for our
conftest.py. To avoid polluting the production targets, this means that we need to declare this in our BUILD files:Beyond being clunky, this is subtle. It will be very easy for us to forget to declare two separate targets and to not use default source globs.
Solution
Leverage the Target API's design to create a new
conftest()target. This behaves like apython_library(), and is basically sugar forpython_library(sources=["conftest.py"]).Update
python_library()to excludeconftest.pyby default, so that it doesn't mix production and test code.Note that a macro could solve the problem for an individual codebase, but we want to solve this problem for all our users.
Result
Now, Toolchain's BUILD file could look like this:
A user can still use a
python_library()target for theirconftest.py, of course. They will still usepython_library()for test utils in general. This is about our default globs doing the right thing.[ci skip-rust]
[ci skip-build-wheels]