So using file addresses (aka generated subtargets) always runs into some issues with the file

conftest.py

. We end up generating a

python_tests()

target for the

conftest.py

file, then try running

./pants test conftest.py

, which causes Pytest to fail because there are no tests inside. This is an issue with

conftest.py

, but also any util file in a

python_tests()

target. Some possible solutions: 1. Stop having

conftest.py

belong to

python_tests()

and have it belong to

python_library()

.

python_tests()

should only have files with actual tests in them. This is conceptually pure, but also not very ergonomic, especially if your tests live in their own folders and you only had

python_tests()

targets before. 2. Special case

pytest_runner.py

to no-op if the file name is

conftest.py

. But this would not catch if you had other util files in your

python_tests()

target, and perhaps you could actually have a test in your

conftest.py

? 3. Change

pytest_runner.py

to not fail if the tests were empty. I argue that this failure is a feature, though - it’s caught issues for me in the past. And Pants should generally emulate Pytest as much as possible.

yea, i agree on #1. it’s already the case that any non-test code in the directory requires the lib

reading through this i would be much more comfortable with #1 as it seems like the correct thing to do and we can possibly figure out a nicer syntax for it later

yea, i think that the example BUILD file is valid and working. it’s just that folks might not have done that in all cases

it’s just that folks might not have done that in all cases

Yeah, this is precisely the issue. If your tests lived in their own dedicated folder, it’s common to only put this in your BUILD file:

python_tests(
  dependences=[...]
)

Now you have to go manually update every BUILD file to do this:

python_tests(
   dependencies=[':conftest', ...]
)

python_library(name="conftest")

Although, actually, if you have

--python-infer-conftests

on (the default), you would solely need to add the

python_library(name="conftest")

target 🙂 You woulnd’t need to update the

dependencies

of your

python_tests

target because we’ll infer it. So, still not great, but not as horrible as before

i would definitely want to have an explicit target covering the conftest file in any case. this could also be a job for reviving https://github.com/pantsbuild/pants/pull/9434 for BUILD file introspection (if we wanted to automate adding this)

I am not aware of any convention regarding tests.py file in django projects...

Okay no worries. Per https://docs.djangoproject.com/en/3.1/topics/testing/overview/#writing-tests, the info block “Where should the tests live?“, it looks like tests do live there. Only wanted to verify with you, but I think that’s what it’s for.

The issue here is both that conftest.py is a util file (does not have tests) and is named *test.py - right? You need both things to hit the error.

We went out of our way to include

conftest.py

in the default globs for

python_tests()

. If we remove that explicit entry, then it no longer will belong by default to a

python_tests

target

that kind of makes sense. i’m not sure that someone reading a repo is necessarily going to have the intuition that one is a test with an underscore and the other not, but i’m not trying to reopen that discussion

I still don't really follow all this but it seems to make sense if possible to respect how pytest works, which is to infer sibling or ancestor conftests.py files as test dependencies.

i’ll read up on that so i can understand this

which is to infer sibling or ancestor conftests.py files as test dependencies.

Ah, there have been some changes to do exactly that in the past month, thanks to

--python-infer-conftests

option

Ok, then I really don't understand this thread at all. Carry on!

Sort of.

--python-infer-conftests

would generate a subtarget from whatever the owner was. It doesn’t care if it’s a

python_library

vs. a

python_tests

target; it will generate the new target from whatever the original was The real issue is the enormous model change in https://github.com/pantsbuild/pants/pull/10511. Now, we run tests one-file-at-a-time, over every file belonging to the

python_tests

target. Because

python_tests

defaults to

conftest.py

being in its default globs, it was common for that to be included in the target.

How does this impact non-python resources, like json blobs, used for tests? Would these still be defined as resources and added as a dependency for a

python_test

target type?

Yeah, either resources or files targets. See https://www.pantsbuild.org/docs/python-test-goal#depending-on-test-utilities-resources-and-built-packages