env: do not mix stdout and stderr when running Python scripts and parsing the output (#6665)

jleclanche · radoering · web-flow · commit 012fcb99b8b7 · 2023-02-15T17:22:21.000+01:00
Co-authored-by: Randy Döring &lt;30527984+radoering@users.noreply.github.com&gt;
diff --git a/src/poetry/utils/env.py b/src/poetry/utils/env.py
@@ -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):
@@ -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:
@@ -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_)
 
diff --git a/tests/utils/test_env.py b/tests/utils/test_env.py
@@ -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
