Skip to content

Commit

Permalink
env: do not mix stdout and stderr when running Python scripts and par…
Browse files Browse the repository at this point in the history
…sing the output (#6665)

Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com>
  • Loading branch information
jleclanche and radoering authored Feb 15, 2023
1 parent e92de61 commit 012fcb9
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 18 deletions.
35 changes: 23 additions & 12 deletions src/poetry/utils/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -463,13 +463,16 @@ class EnvCommandError(EnvError):
def __init__(self, e: CalledProcessError, input: str | None = None) -> None:
self.e = e

message = (
f"Command {e.cmd} errored with the following return code {e.returncode},"
f" and output: \n{decode(e.output)}"
)
message_parts = [
f"Command {e.cmd} errored with the following return code {e.returncode}"
]
if e.output:
message_parts.append(f"Output:\n{decode(e.output)}")
if e.stderr:
message_parts.append(f"Error output:\n{decode(e.stderr)}")
if input:
message += f"input was : {input}"
super().__init__(message)
message_parts.append(f"Input:\n{input}")
super().__init__("\n\n".join(message_parts))


class NoCompatiblePythonVersionFound(EnvError):
Expand Down Expand Up @@ -1503,7 +1506,14 @@ def run_pip(self, *args: str, **kwargs: Any) -> int | str:

def run_python_script(self, content: str, **kwargs: Any) -> int | str:
return self.run(
self._executable, "-I", "-W", "ignore", "-", input_=content, **kwargs
self._executable,
"-I",
"-W",
"ignore",
"-",
input_=content,
stderr=subprocess.PIPE,
**kwargs,
)

def _run(self, cmd: list[str], **kwargs: Any) -> int | str:
Expand All @@ -1513,23 +1523,24 @@ def _run(self, cmd: list[str], **kwargs: Any) -> int | str:
call = kwargs.pop("call", False)
input_ = kwargs.pop("input_", None)
env = kwargs.pop("env", dict(os.environ))
stderr = kwargs.pop("stderr", subprocess.STDOUT)

try:
if input_:
output = subprocess.run(
cmd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
stderr=stderr,
input=encode(input_),
check=True,
**kwargs,
).stdout
elif call:
return subprocess.call(cmd, stderr=subprocess.STDOUT, env=env, **kwargs)
else:
output = subprocess.check_output(
cmd, stderr=subprocess.STDOUT, env=env, **kwargs
return subprocess.call(
cmd, stdout=subprocess.PIPE, stderr=stderr, env=env, **kwargs
)
else:
output = subprocess.check_output(cmd, stderr=stderr, env=env, **kwargs)
except CalledProcessError as e:
raise EnvCommandError(e, input=input_)

Expand Down
66 changes: 60 additions & 6 deletions tests/utils/test_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -947,35 +947,89 @@ def test_run_with_called_process_error(
tmp_dir: str, tmp_venv: VirtualEnv, mocker: MockerFixture
):
mocker.patch(
"subprocess.run", side_effect=subprocess.CalledProcessError(42, "some_command")
"subprocess.run",
side_effect=subprocess.CalledProcessError(
42, "some_command", "some output", "some error"
),
)
with pytest.raises(EnvCommandError):
with pytest.raises(EnvCommandError) as error:
tmp_venv.run("python", "-", input_=MINIMAL_SCRIPT)
subprocess.run.assert_called_once()
assert "some output" in str(error.value)
assert "some error" in str(error.value)


def test_call_with_input_and_called_process_error(
tmp_dir: str, tmp_venv: VirtualEnv, mocker: MockerFixture
):
mocker.patch(
"subprocess.run", side_effect=subprocess.CalledProcessError(42, "some_command")
"subprocess.run",
side_effect=subprocess.CalledProcessError(
42, "some_command", "some output", "some error"
),
)
kwargs = {"call": True}
with pytest.raises(EnvCommandError):
with pytest.raises(EnvCommandError) as error:
tmp_venv.run("python", "-", input_=MINIMAL_SCRIPT, **kwargs)
subprocess.run.assert_called_once()
assert "some output" in str(error.value)
assert "some error" in str(error.value)


def test_call_no_input_with_called_process_error(
tmp_dir: str, tmp_venv: VirtualEnv, mocker: MockerFixture
):
mocker.patch(
"subprocess.call", side_effect=subprocess.CalledProcessError(42, "some_command")
"subprocess.call",
side_effect=subprocess.CalledProcessError(
42, "some_command", "some output", "some error"
),
)
kwargs = {"call": True}
with pytest.raises(EnvCommandError):
with pytest.raises(EnvCommandError) as error:
tmp_venv.run("python", "-", **kwargs)
subprocess.call.assert_called_once()
assert "some output" in str(error.value)
assert "some error" in str(error.value)


def test_check_output_with_called_process_error(
tmp_dir: str, tmp_venv: VirtualEnv, mocker: MockerFixture
):
mocker.patch(
"subprocess.check_output",
side_effect=subprocess.CalledProcessError(
42, "some_command", "some output", "some error"
),
)
with pytest.raises(EnvCommandError) as error:
tmp_venv.run("python", "-")
subprocess.check_output.assert_called_once()
assert "some output" in str(error.value)
assert "some error" in str(error.value)


def test_run_python_script_called_process_error(
tmp_dir: str, tmp_venv: VirtualEnv, mocker: MockerFixture
):
mocker.patch(
"subprocess.run",
side_effect=subprocess.CalledProcessError(
42, "some_command", "some output", "some error"
),
)
with pytest.raises(EnvCommandError) as error:
tmp_venv.run_python_script(MINIMAL_SCRIPT)
assert "some output" in str(error.value)
assert "some error" in str(error.value)


def test_run_python_script_only_stdout(tmp_dir: str, tmp_venv: VirtualEnv):
output = tmp_venv.run_python_script(
"import sys; print('some warning', file=sys.stderr); print('some output')"
)
assert "some output" in output
assert "some warning" not in output


def test_create_venv_tries_to_find_a_compatible_python_executable_using_generic_ones_first( # noqa: E501
Expand Down

0 comments on commit 012fcb9

Please sign in to comment.