https://pantsbuild.org/ logo
Join Slack
Powered by
A collegue send me this some minutes ago. This err...
# general
m
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
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
u mean that the python_tests is in the wrong file, right?
thanks
c
in the wrong field, yes.
✅ 1
h
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
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
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
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
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
h
"yes" to which?
😂 1
Ah, so you cannot put 
conftest.py
in there even explicitly
h
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
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
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
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
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
Fair enough. I just don't want us to go too far down the path of constraining the user.
4 Views
Open in Slack
PreviousNext
Powered by