https://pantsbuild.org/ logo
#development
Title
# development
w

witty-crayon-22786

02/17/2021, 7:13 PM
minor oopsie: post https://github.com/pantsbuild/pants/pull/11557, pytest tests don’t fail.
investigating now.
h

hundreds-father-404

02/17/2021, 7:13 PM
I just noticed this too
w

witty-crayon-22786

02/17/2021, 7:14 PM
(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

hundreds-father-404

02/17/2021, 7:19 PM
I'm gonna try to look more into why docformatter is failing *to resolve, but can take over your investigation after
w

witty-crayon-22786

02/17/2021, 7:20 PM
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

enough-analyst-54434

02/17/2021, 7:26 PM
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

witty-crayon-22786

02/17/2021, 7:27 PM
hang tight
e

enough-analyst-54434

02/17/2021, 7:27 PM
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

witty-crayon-22786

02/17/2021, 7:28 PM
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

enough-analyst-54434

02/17/2021, 7:29 PM
... 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

witty-crayon-22786

02/17/2021, 7:30 PM
why the inconsistency on first run vs second?
e

enough-analyst-54434

02/17/2021, 7:30 PM
You might try that change quick.
No clue.
w

witty-crayon-22786

02/17/2021, 7:31 PM
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

enough-analyst-54434

02/17/2021, 7:33 PM
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

witty-crayon-22786

02/17/2021, 7:33 PM
yea, that was likely it.
e

enough-analyst-54434

02/17/2021, 7:33 PM
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

witty-crayon-22786

02/17/2021, 7:35 PM
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

enough-analyst-54434

02/17/2021, 7:35 PM
Thanks for help debugging.
👍 1
Yeah - I'll handle a PR. Thank you.
❤️ 2
w

witty-crayon-22786

02/17/2021, 7:37 PM
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

enough-analyst-54434

02/17/2021, 8:27 PM
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.