https://pantsbuild.org/ logo
Join Slack
Powered by
minor oopsie: post <https://github.com/pantsbuild/...
# development
w
minor oopsie: post https://github.com/pantsbuild/pants/pull/11557, pytest tests don’t fail.
investigating now.
h
I just noticed this too
w
(but i’ve got to get on the road in about 45 minutes, so unless it’s a quick fix someone else will need to grab)
some 
VenvPexProcess
instances do though… at least linting and typechecking will fail
h
I'm gonna try to look more into why docformatter is failing *to resolve, but can take over your investigation after
w
thanks!
iiinteresting. it does fail when run directly inside the sandbox via 
--no-process-execution-cleanup-local-dirs
👀 1
that’s unexpected.
i wonder if i messed up my bisect.
OHO. it fails the first time, and succeeds the second time.
e
So we set an entrypoint of pytest:main which I left unchanged in my PR. That appears to be bogus. Thats just a function that does no sys.exit.
We whould be using an entrypoint of just 
pytest
the module.
🚀 1
w
hang tight
e
The fact that you can't specify a script (-c in Pex) in Pants is broken
You should be able to use entrypoints or console scripts just like you can with Pex or anything else.
w
https://gist.github.com/stuhood/885491f6bb29498811424c36bb4b2e07 : returns 1 the first time it is run, and 0 the second time
so probably the return code in the venv wrapper script
e
... that's what my above addresses.
We set an entrypoint of pytest:main and thats just a function that returns an int
Pex cannot know to take the return value of an entrypoint function and exit with it.
So ... we should be using 
pytest
as the entrypoint
w
why the inconsistency on first run vs second?
e
You might try that change quick.
No clue.
w
k… will see if i repro the inconsistency.
…hm, yea. you’re right. i think the inconsistency was a red herring.
ok, trying the entrypoint change.
e
Yeah, so PEX blindly takes the return value of an entrypoint function an does a sys.exit(rv) ... no check if it's an int or anything. So that's broken but works for pytest:main 😕
w
yea, that was likely it.
e
The pex venv script does no such thing and I think it should likely not.
Ok. I'm going to file an issue here and add a user warning to PEX any time it does this to deprecate and kill that bogus behavior. I'll also file a Pants issue to track supporting -c since thats the right way in general to interface with a tool. The console script is the public api for mains.
💯 1
w
Copy code
$ git diff -- src/python/pants/backend/python/goals/pytest_runner.py
diff --git a/src/python/pants/backend/python/goals/pytest_runner.py b/src/python/pants/backend/python/goals/pytest_runner.py
index 39b170045..a09c951ab 100644
--- a/src/python/pants/backend/python/goals/pytest_runner.py
+++ b/src/python/pants/backend/python/goals/pytest_runner.py
@@ -172,7 +172,7 @@ async def setup_pytest_for_target(
         PexRequest(
             output_filename="pytest_runner.pex",
             interpreter_constraints=interpreter_constraints,
-            entry_point="pytest:main",
+            entry_point="pytest",
             internal_only=True,
             pex_path=[pytest_pex, requirements_pex],
         ),
fixes it: thanks John. i won’t have time to get the fix out with a test before i leave though.
e
Thanks for help debugging.
👍 1
Yeah - I'll handle a PR. Thank you.
❤️ 2
w
i’m really surprised that 
src/python/pants/backend/python/goals/pytest_runner_integration_test.py
didn’t catch this. perhaps it’s failing in a way that raises rather than returning?
e
I am too. Fiddled a good bit but still can't get it to fail whereas command line tests quickly show before / after. Not happy, but I'll send the un-automated-test fix up here presently to ensure we keep master green for real.
OK, finally got around to researching this in depth on the Pex side: https://github.com/pantsbuild/pex/issues/1241 I'll be fixing the 
pex
venv script to behave like 
PEX
and just documenting how we handle entrypoint functions.
🤘 1
But better, proposing this: https://github.com/pantsbuild/pants/issues/11583 We should not be binding tools to python module or functions but console scripts since we call them as tools and not use them as APIs.
Open in Slack
PreviousNext
Powered by