Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pex exits with correct code when using entrypoint #605

Merged
merged 4 commits into from
Oct 19, 2018
Merged

Pex exits with correct code when using entrypoint #605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
23b11eb
Pex exits with correct code when using entrypoint
illicitonion Oct 19, 2018
4bbfade
Add test
illicitonion Oct 19, 2018
ea1f6bc
Fix test
illicitonion Oct 19, 2018
54edd97
realpath?
illicitonion Oct 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions pex/pex.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,7 @@ def patch_all(path, path_importer_cache, modules):

def _wrap_coverage(self, runner, *args):
if not self._vars.PEX_COVERAGE and self._vars.PEX_COVERAGE_FILENAME is None:
runner(*args)
return
return runner(*args)

try:
import coverage
Expand All @@ -296,7 +295,7 @@ def _wrap_coverage(self, runner, *args):
cov.start()

try:
runner(*args)
return runner(*args)
finally:
TRACER.log('Stopping coverage')
cov.stop()
Expand All @@ -310,8 +309,7 @@ def _wrap_coverage(self, runner, *args):

def _wrap_profiling(self, runner, *args):
if not self._vars.PEX_PROFILE and self._vars.PEX_PROFILE_FILENAME is None:
runner(*args)
return
return runner(*args)

pex_profile_filename = self._vars.PEX_PROFILE_FILENAME
pex_profile_sort = self._vars.PEX_PROFILE_SORT
Expand Down Expand Up @@ -348,7 +346,9 @@ def execute(self):
self.patch_sys(pex_inherit_path)
working_set = self._activate()
self.patch_pkg_resources(working_set)
self._wrap_coverage(self._wrap_profiling, self._execute)
exit_code = self._wrap_coverage(self._wrap_profiling, self._execute)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was never correct. Who knows is the entry point function even returns an int? The real problem was ever using the pytest:main entrypoint. We should have been using -c pytest or else -m pytest. Pex needs to provide some guidance in its --help and docs, but using an entrypoint function (foo.bar:baz) implies you know the function does its own sys.exits to indicate failure or else raises. If it just returns some value, whether an int or a string or a what have you, PEX really can't just presume and pas that to sys.exit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The right path may be to keep this behavior but actually document it. Pex could say "If you pass an entrypoint function and it returns anything, we'll assume that's an int that represents the exit code of the process that should be used."

It looks like sys.exit behavior is a bit whacky and gracefully supports the present use :/:

$ python -c 'import sys; sys.exit("Hello")'; echo $?
Hello
1
$ python -c 'import sys; sys.exit([])'; echo $?
[]
1
$ python -c 'import sys; sys.exit(["a"])'; echo $?
['a']
1
$ python -c 'import sys; sys.exit(True)'; echo $?
1
$ python -c 'import sys; sys.exit(False)'; echo $?
0

if exit_code:
sys.exit(exit_code)
except Exception:
# Allow the current sys.excepthook to handle this app exception before we tear things down in
# finally, then reraise so that the exit status is reflected correctly.
Expand Down
20 changes: 20 additions & 0 deletions tests/test_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,26 @@ def test_pex_manylinux_runtime():
assert out.strip() == '[1, 2, 3]'


def test_pex_exit_code_propagation():
"""Tests exit code propagation."""
test_stub = dedent(
"""
def test_fail():
assert False
"""
)

with temporary_content({'tester.py': test_stub}) as output_dir:
pex_path = os.path.join(output_dir, 'test.pex')
tester_path = os.path.join(output_dir, 'tester.py')
results = run_pex_command(['pytest==3.9.1',
'-e', 'pytest:main',
'-o', pex_path])
results.assert_success()

assert subprocess.call([pex_path, os.path.realpath(tester_path)]) == 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lovely, eh?



@pytest.mark.skipif(NOT_CPYTHON27)
def test_platform_specific_inline_egg_resolution():
with temporary_dir() as td:
Expand Down