https://pantsbuild.org/ logo
#general
Title
# general
m

modern-wolf-36228

10/21/2021, 9:54 AM
A collegue send me this some minutes ago. This error message doesn't seem correct? 🤔
code:
Copy code
python_sources(
    dependencies=[
        "src/python/authenticator/resources",
        "src/python/authenticator/main.py",
        "//:python-multipart",
        "//:asyncpg"
    ]
)

python_tests(
    name="tests",
    timeout=120,
    sources=["tests/conftest.py"],
    dependencies=["src/python/authenticator/tests.py"],
    extra_env_vars=[
      "PROJECT_NAME=authenticator",
    ]
)

pex_binary(
    name="binary",
    entry_point="__main__.py",
    dependencies=["//:asyncpg"],
)
c

curved-television-6568

10/21/2021, 9:59 AM
That can happen if the selected target was filtered out for some reason. Not sure how things behave when listing the tests as dependencies rather than sources, though…
m

modern-wolf-36228

10/21/2021, 10:00 AM
u mean that the python_tests is in the wrong file, right?
thanks
c

curved-television-6568

10/21/2021, 10:02 AM
in the wrong field, yes.
1
h

happy-kitchen-89482

10/21/2021, 2:38 PM
That error message is terrible! Can you open a ticket for it at https://github.com/pantsbuild/pants/issues ? And let us know the BUILD file contents as well as what command the user was running?
Sorry for the inconvenience
But yeah, the problem is this:
Copy code
python_tests(
    name="tests",
    timeout=120,
    sources=["tests/conftest.py"],
    dependencies=["src/python/authenticator/tests.py"],
    extra_env_vars=[
      "PROJECT_NAME=authenticator",
    ]
)
Whereas you want the test files in
sources
of
python_tests
targets , not in
dependencies
Where is this BUILD file in relation to
src/python/authenticator
?
t

tall-truck-67859

10/21/2021, 3:24 PM
I'm @modern-wolf-36228 collegue. It's fixed, thanks for your help ! I had to put
python_tests
inside tests/BUILD file, and remove tests files from dependencies yes 🙂
👋 1
h

hundreds-father-404

10/21/2021, 3:25 PM
Benjy, fyi this situation won't be possible soon thanks to us deprecating allowing conftest.py to be in the sources field or python_tests I don't think we need a GitHub issue. This is because we special case conftest.py and tell Pytest to ignore it. Again, will be fixed soon
👍 3
c

curved-television-6568

10/22/2021, 7:09 AM
I’ve seen this message too before, and it isn’t great in those situations.. for me it was when running
publish
for a
docker_image
target, and that target has a
skip_push
. So the target isn’t off.. it’s just that it wasn’t applicable for the requested goal.
👍 1
h

happy-kitchen-89482

10/22/2021, 4:50 PM
Are we deprecating "allowing conftest.py to be in the sources field or python_tests" or just changing the defaults?
Not sure I understand "allowing" in this context
h

hundreds-father-404

10/22/2021, 4:51 PM
h

happy-kitchen-89482

10/22/2021, 5:04 PM
"yes" to which?
😂 1
Ah, so you cannot put
conftest.py
in there even explicitly
h

hundreds-father-404

10/22/2021, 5:07 PM
No, because it's a modeling error.
conftest.py
and
foo_test.pyi
are test utils, they're not tests themselves. What does
timeout
field mean on a type stub?
It was always a modeling error. I tried fixing in Pants 2.0, but we chose to intentionally use incorrect modeling for the sake of less verbose BUILD files because it worked "enough" with our weird
opt_out
hack The world is different now, with
./pants tailor
making the boilerplate less offensive,
python_tests
vs
python_test
targets +
./pants peek
making the weirdness more explicit, and multiple user lockfiles meaning this bad modeling is actually going to cause issues
h

happy-kitchen-89482

10/23/2021, 6:17 PM
Yeah but this is a very special case that relies on a naming convention
There is nothing in principle that stops me from having a test file called
conftest.py
(maybe it tests
conf.py
...)
And now there is no escape hatch for that
And on the other hand there is nothing to stop me from putting test_foo.py in a python_tests target when it doesn't contain tests, or putting
foo.py
in python_sources when it does contain tests.
So in general we don't stop you from doing the wrong thing, it seems odd to do it in this one special case
I get not doing the wrong thing by default, obviously we want to fix that
But straight up telling a user "you cannot name your test conftest.py" is odd
we don't do that anywhere else
maybe it's fine to do in this case because the special meaning of conftest.py is a strong pytest convention
But nonetheless, we don't enforce that kind of thing anywhere elsee
h

hundreds-father-404

10/23/2021, 6:33 PM
Reminder we have a dependency inference rule for files named
conftest.py
- that particular file name is already very very special-cased And then for type stubs like
test_foo.pyi
, it never really makes sense to run Pytest on a type stub file Those are the only two restrictions we put on
python_tests
. You could still put
foo.py
in it if you'd like
h

happy-kitchen-89482

10/23/2021, 11:43 PM
That can be turned off though
This is the first time, AFAICT, where we've made a strong assumption and offered no escape hatch
My point is that I'm not sure why it's necessary to do so when we generally don't do things like that
I'm not strongly opposed, but it does seem like an odd step to me
h

hundreds-father-404

10/24/2021, 12:14 AM
I don't think I buy that fully. We do validate things already like that file ends in
.py
or no file extension. We error on
.js
, even though it's possible to run Black on
.js
files:
Copy code
❯ black foo.js
All done! :sparkles: :cake: :sparkles:
1 file left unchanged.
That is, generally, we already are doing some eager validation for cases that are legal, but very unlikely to be what the user intended so that we can instead give helpful instructions towards what they likely wanted to do
offered no escape hatch
The escape hatch is to name it literally anything other than
conftest.py
! Like
conf_test.py
h

happy-kitchen-89482

10/24/2021, 8:55 PM
Fair enough. I just don't want us to go too far down the path of constraining the user.
4 Views