diff --git a/changes/1141.bugfix.rst b/changes/1141.bugfix.rst new file mode 100644 index 000000000..f2de6cc2b --- /dev/null +++ b/changes/1141.bugfix.rst @@ -0,0 +1 @@ +When commands produce output that cannot be decoded to Unicode, Briefcase now writes the bytes as hex instead of truncating output or canceling the command altogether. diff --git a/src/briefcase/integrations/subprocess.py b/src/briefcase/integrations/subprocess.py index f8ec2af6e..b946bb184 100644 --- a/src/briefcase/integrations/subprocess.py +++ b/src/briefcase/integrations/subprocess.py @@ -221,25 +221,36 @@ def final_kwargs(self, **kwargs): "encoding", os.device_encoding(sys.__stdout__.fileno()) or "UTF-8" ) + # When text mode is enabled, subprocess defaults to "strict" + # handling for errors arising from decoding output to Unicode. + # To avoid Unicode exceptions from misbehaving commands, set + # `errors` so output that cannot be decoded for the specified + # encoding are replaced with hex of the raw bytes. + if any(kwargs.get(kw) for kw in ["text", "encoding", "universal_encoding"]): + kwargs.setdefault("errors", "backslashreplace") + # For Windows, convert start_new_session=True to creation flags if self.tools.host_os == "Windows": try: if kwargs.pop("start_new_session") is True: if "creationflags" in kwargs: raise AssertionError( - "Subprocess called with creationflags set and start_new_session=True.\n" - "This will result in CREATE_NEW_PROCESS_GROUP and CREATE_NO_WINDOW being " - "merged in to the creationflags.\n\n" - "Ensure this is desired configuration or don't set start_new_session=True." + "Subprocess called with creationflags set and " + "start_new_session=True.\nThis will result in " + "CREATE_NEW_PROCESS_GROUP and CREATE_NO_WINDOW " + "being merged in to the creationflags.\n\nEnsure " + "this is desired configuration or don't set " + "start_new_session=True." ) - # CREATE_NEW_PROCESS_GROUP: Makes the new process the root process - # of a new process group. This also disables CTRL+C signal handlers - # for all processes of the new process group. - # CREATE_NO_WINDOW: Creates a new console for the process but does not - # open a visible window for that console. This flag is used instead - # of DETACHED_PROCESS since the new process can spawn a new console - # itself (in the absence of one being available) but that console - # creation will also spawn a visible console window. + # CREATE_NEW_PROCESS_GROUP: Promotes the new process to the root + # process of a new process group. This also disables CTRL+C + # signal handlers for all processes of the new process group. + # CREATE_NO_WINDOW: Creates a new console for the process but + # does not open a visible window for that console. This flag + # is used instead of DETACHED_PROCESS since the new process + # can spawn a new console itself (in the absence of one being + # available) but that console creation will also spawn a + # visible console window. new_session_flags = ( self._subprocess.CREATE_NEW_PROCESS_GROUP | self._subprocess.CREATE_NO_WINDOW diff --git a/tests/conftest.py b/tests/conftest.py index 24a7b47d3..1f6954e5b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,6 @@ import inspect -from unittest import mock +import subprocess +from unittest.mock import ANY, MagicMock import git as git_ import pytest @@ -19,9 +20,8 @@ def pytest_sessionfinish(session, exitstatus): @pytest.fixture def mock_git(): - git = mock.MagicMock(spec_set=git_) + git = MagicMock(spec_set=git_) git.exc = git_exceptions - return git @@ -49,6 +49,27 @@ def no_print(monkeypatch): monkeypatch.setattr("builtins.print", monkeypatched_print) +@pytest.fixture +def sub_kw(): + """Default keyword arguments for all subprocess calls.""" + return dict(text=True, encoding=ANY, errors="backslashreplace") + + +@pytest.fixture +def sub_check_output_kw(sub_kw): + """Default keyword arguments for all subprocess.check_output calls.""" + return {**sub_kw, **dict(stderr=subprocess.STDOUT)} + + +@pytest.fixture +def sub_stream_kw(sub_kw): + """Default keyword arguments for all output streaming subprocess calls.""" + return { + **sub_kw, + **dict(stdout=subprocess.PIPE, stderr=subprocess.STDOUT, bufsize=1), + } + + @pytest.fixture def first_app_config(): return AppConfig( diff --git a/tests/integrations/docker/test_DockerAppContext__check_output.py b/tests/integrations/docker/test_DockerAppContext__check_output.py index a9b7162d5..226f62a96 100644 --- a/tests/integrations/docker/test_DockerAppContext__check_output.py +++ b/tests/integrations/docker/test_DockerAppContext__check_output.py @@ -1,13 +1,11 @@ import os -import subprocess import sys from pathlib import Path -from unittest.mock import ANY import pytest -def test_simple_call(mock_docker_app_context, tmp_path, capsys): +def test_simple_call(mock_docker_app_context, tmp_path, sub_check_output_kw, capsys): """A simple call will be invoked.""" assert mock_docker_app_context.check_output(["hello", "world"]) == "goodbye\n" @@ -24,14 +22,12 @@ def test_simple_call(mock_docker_app_context, tmp_path, capsys): "hello", "world", ], - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) assert capsys.readouterr().out == "" -def test_extra_mounts(mock_docker_app_context, tmp_path, capsys): +def test_extra_mounts(mock_docker_app_context, tmp_path, sub_check_output_kw, capsys): """A call can request additional mounts.""" assert ( mock_docker_app_context.check_output( @@ -61,9 +57,7 @@ def test_extra_mounts(mock_docker_app_context, tmp_path, capsys): "hello", "world", ], - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) assert capsys.readouterr().out == "" @@ -71,7 +65,7 @@ def test_extra_mounts(mock_docker_app_context, tmp_path, capsys): @pytest.mark.skipif( sys.platform == "win32", reason="Windows paths aren't converted in Docker context" ) -def test_cwd(mock_docker_app_context, tmp_path, capsys): +def test_cwd(mock_docker_app_context, tmp_path, sub_check_output_kw, capsys): """A call can use a working directory relative to the project folder.""" assert ( mock_docker_app_context.check_output( @@ -96,14 +90,17 @@ def test_cwd(mock_docker_app_context, tmp_path, capsys): "hello", "world", ], - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) assert capsys.readouterr().out == "" -def test_call_with_arg_and_env(mock_docker_app_context, tmp_path, capsys): +def test_call_with_arg_and_env( + mock_docker_app_context, + tmp_path, + sub_check_output_kw, + capsys, +): """Extra keyword arguments are passed through as-is; env modifications are converted.""" output = mock_docker_app_context.check_output( @@ -116,6 +113,7 @@ def test_call_with_arg_and_env(mock_docker_app_context, tmp_path, capsys): ) assert output == "goodbye\n" + sub_check_output_kw.pop("text") mock_docker_app_context.tools.subprocess._subprocess.check_output.assert_called_once_with( [ "docker", @@ -134,8 +132,7 @@ def test_call_with_arg_and_env(mock_docker_app_context, tmp_path, capsys): "world", ], universal_newlines=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) assert capsys.readouterr().out == "" @@ -144,7 +141,12 @@ def test_call_with_arg_and_env(mock_docker_app_context, tmp_path, capsys): sys.platform == "win32", reason="Windows paths aren't converted in Docker context", ) -def test_call_with_path_arg_and_env(mock_docker_app_context, tmp_path, capsys): +def test_call_with_path_arg_and_env( + mock_docker_app_context, + tmp_path, + sub_check_output_kw, + capsys, +): """Path-based arguments and environment are converted to strings and passed in as- is.""" output = mock_docker_app_context.check_output( @@ -176,9 +178,7 @@ def test_call_with_path_arg_and_env(mock_docker_app_context, tmp_path, capsys): "hello", os.fsdecode(tmp_path / "location"), ], - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) assert capsys.readouterr().out == "" @@ -186,7 +186,12 @@ def test_call_with_path_arg_and_env(mock_docker_app_context, tmp_path, capsys): @pytest.mark.skipif( sys.platform == "win32", reason="Windows paths aren't converted in Docker context" ) -def test_simple_verbose_call(mock_docker_app_context, tmp_path, capsys): +def test_simple_verbose_call( + mock_docker_app_context, + tmp_path, + sub_check_output_kw, + capsys, +): """If verbosity is turned out, there is output.""" mock_docker_app_context.tools.logger.verbosity = 2 @@ -205,9 +210,7 @@ def test_simple_verbose_call(mock_docker_app_context, tmp_path, capsys): "hello", "world", ], - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) assert capsys.readouterr().out == ( "\n" diff --git a/tests/integrations/docker/test_DockerAppContext__prepare.py b/tests/integrations/docker/test_DockerAppContext__prepare.py index 5ae5bd5c3..62023d7f4 100644 --- a/tests/integrations/docker/test_DockerAppContext__prepare.py +++ b/tests/integrations/docker/test_DockerAppContext__prepare.py @@ -1,6 +1,5 @@ import os import subprocess -from unittest.mock import ANY import pytest @@ -8,7 +7,7 @@ from briefcase.integrations.docker import DockerAppContext -def test_prepare(mock_tools, my_app, tmp_path): +def test_prepare(mock_tools, my_app, tmp_path, sub_stream_kw): """The Docker environment can be prepared.""" mock_docker_app_context = DockerAppContext(mock_tools, my_app) mock_docker_app_context.prepare( @@ -40,11 +39,7 @@ def test_prepare(mock_tools, my_app, tmp_path): "HOST_GID=42", os.fsdecode(tmp_path / "base" / "path" / "to" / "src"), ], - stdout=-1, - stderr=-2, - bufsize=1, - text=True, - encoding=ANY, + **sub_stream_kw, ) assert mock_docker_app_context.app_base_path == tmp_path / "base" @@ -54,7 +49,7 @@ def test_prepare(mock_tools, my_app, tmp_path): assert mock_docker_app_context.python_version == "3.X" -def test_prepare_failure(mock_docker_app_context, tmp_path): +def test_prepare_failure(mock_docker_app_context, tmp_path, sub_stream_kw): """If the Docker environment can't be prepared, an error is raised.""" # Mock a failure in docker build. mock_docker_app_context.tools.subprocess._subprocess.Popen.side_effect = ( @@ -91,9 +86,5 @@ def test_prepare_failure(mock_docker_app_context, tmp_path): "HOST_GID=42", os.fsdecode(tmp_path / "base" / "path" / "to" / "src"), ], - stdout=-1, - stderr=-2, - bufsize=1, - text=True, - encoding=ANY, + **sub_stream_kw, ) diff --git a/tests/integrations/docker/test_DockerAppContext__run.py b/tests/integrations/docker/test_DockerAppContext__run.py index 895cdbb3b..c05c27cbd 100644 --- a/tests/integrations/docker/test_DockerAppContext__run.py +++ b/tests/integrations/docker/test_DockerAppContext__run.py @@ -1,13 +1,11 @@ import os -import subprocess import sys from pathlib import Path -from unittest.mock import ANY import pytest -def test_simple_call(mock_docker_app_context, tmp_path, capsys): +def test_simple_call(mock_docker_app_context, tmp_path, sub_stream_kw, capsys): """A simple call will be invoked.""" mock_docker_app_context.run(["hello", "world"]) @@ -25,11 +23,7 @@ def test_simple_call(mock_docker_app_context, tmp_path, capsys): "hello", "world", ], - text=True, - encoding=ANY, - bufsize=1, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, + **sub_stream_kw, ) assert capsys.readouterr().out == ( "\n" @@ -41,7 +35,7 @@ def test_simple_call(mock_docker_app_context, tmp_path, capsys): ) -def test_interactive(mock_docker_app_context, tmp_path, capsys): +def test_interactive(mock_docker_app_context, tmp_path, sub_kw, capsys): """Docker can be invoked in interactive mode.""" mock_docker_app_context.run(["hello", "world"], interactive=True) @@ -60,8 +54,7 @@ def test_interactive(mock_docker_app_context, tmp_path, capsys): "hello", "world", ], - text=True, - encoding=ANY, + **sub_kw, ) assert capsys.readouterr().out == ( "\n" @@ -73,7 +66,7 @@ def test_interactive(mock_docker_app_context, tmp_path, capsys): ) -def test_extra_mounts(mock_docker_app_context, tmp_path, capsys): +def test_extra_mounts(mock_docker_app_context, tmp_path, sub_stream_kw, capsys): """A subprocess call can be augmented with mounts.""" mock_docker_app_context.run( @@ -101,11 +94,7 @@ def test_extra_mounts(mock_docker_app_context, tmp_path, capsys): "hello", "world", ], - text=True, - encoding=ANY, - bufsize=1, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, + **sub_stream_kw, ) assert capsys.readouterr().out == ( "\n" @@ -120,7 +109,7 @@ def test_extra_mounts(mock_docker_app_context, tmp_path, capsys): @pytest.mark.skipif( sys.platform == "win32", reason="Windows paths aren't converted in Docker context" ) -def test_cwd(mock_docker_app_context, tmp_path, capsys): +def test_cwd(mock_docker_app_context, tmp_path, sub_stream_kw, capsys): """A subprocess call can use a working directory relative to the project folder.""" mock_docker_app_context.run( @@ -143,11 +132,7 @@ def test_cwd(mock_docker_app_context, tmp_path, capsys): "hello", "world", ], - text=True, - encoding=ANY, - bufsize=1, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, + **sub_stream_kw, ) assert capsys.readouterr().out == ( "\n" @@ -159,7 +144,12 @@ def test_cwd(mock_docker_app_context, tmp_path, capsys): ) -def test_call_with_arg_and_env(mock_docker_app_context, tmp_path, capsys): +def test_call_with_arg_and_env( + mock_docker_app_context, + tmp_path, + sub_stream_kw, + capsys, +): """Extra keyword arguments are passed through as-is; env modifications are converted.""" @@ -172,6 +162,7 @@ def test_call_with_arg_and_env(mock_docker_app_context, tmp_path, capsys): universal_newlines=True, ) + sub_stream_kw.pop("text") mock_docker_app_context.tools.subprocess._subprocess.Popen.assert_called_once_with( [ "docker", @@ -190,10 +181,7 @@ def test_call_with_arg_and_env(mock_docker_app_context, tmp_path, capsys): "world", ], universal_newlines=True, - encoding=ANY, - bufsize=1, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, + **sub_stream_kw, ) assert capsys.readouterr().out == ( "\n" @@ -208,7 +196,12 @@ def test_call_with_arg_and_env(mock_docker_app_context, tmp_path, capsys): @pytest.mark.skipif( sys.platform == "win32", reason="Windows paths aren't converted in Docker context" ) -def test_call_with_path_arg_and_env(mock_docker_app_context, tmp_path, capsys): +def test_call_with_path_arg_and_env( + mock_docker_app_context, + tmp_path, + sub_stream_kw, + capsys, +): """Path-based arguments and environment are converted to strings and passed in as- is.""" @@ -240,11 +233,7 @@ def test_call_with_path_arg_and_env(mock_docker_app_context, tmp_path, capsys): "hello", os.fsdecode(tmp_path / "location"), ], - text=True, - encoding=ANY, - bufsize=1, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, + **sub_stream_kw, ) assert capsys.readouterr().out == ( "\n" @@ -260,7 +249,10 @@ def test_call_with_path_arg_and_env(mock_docker_app_context, tmp_path, capsys): sys.platform == "win32", reason="Windows paths aren't converted in Docker context" ) def test_interactive_with_path_arg_and_env_and_mounts( - mock_docker_app_context, tmp_path, capsys + mock_docker_app_context, + tmp_path, + sub_kw, + capsys, ): """Docker can be invoked in interactive mode with all the extras.""" mock_docker_app_context.run( @@ -302,8 +294,7 @@ def test_interactive_with_path_arg_and_env_and_mounts( "hello", os.fsdecode(tmp_path / "location"), ], - text=True, - encoding=ANY, + **sub_kw, ) assert capsys.readouterr().out == ( "\n" @@ -318,7 +309,7 @@ def test_interactive_with_path_arg_and_env_and_mounts( @pytest.mark.skipif( sys.platform == "win32", reason="Windows paths aren't converted in Docker context" ) -def test_simple_verbose_call(mock_docker_app_context, tmp_path, capsys): +def test_simple_verbose_call(mock_docker_app_context, tmp_path, sub_stream_kw, capsys): """If verbosity is turned out, there is output.""" mock_docker_app_context.tools.logger.verbosity = 2 @@ -337,11 +328,7 @@ def test_simple_verbose_call(mock_docker_app_context, tmp_path, capsys): "hello", "world", ], - text=True, - encoding=ANY, - bufsize=1, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, + **sub_stream_kw, ) assert capsys.readouterr().out == ( "\n" diff --git a/tests/integrations/subprocess/test_Subprocess__Popen.py b/tests/integrations/subprocess/test_Subprocess__Popen.py index b393542fb..b1ed6bf6b 100644 --- a/tests/integrations/subprocess/test_Subprocess__Popen.py +++ b/tests/integrations/subprocess/test_Subprocess__Popen.py @@ -8,34 +8,31 @@ @pytest.mark.parametrize("platform", ["Linux", "Darwin", "Windows"]) -def test_call(mock_sub, capsys, platform): +def test_call(mock_sub, capsys, platform, sub_kw): """A simple call will be invoked.""" mock_sub.tools.host_os = platform mock_sub.Popen(["hello", "world"]) - mock_sub._subprocess.Popen.assert_called_with( - ["hello", "world"], - text=True, - encoding=ANY, - ) + mock_sub._subprocess.Popen.assert_called_with(["hello", "world"], **sub_kw) assert capsys.readouterr().out == "" -def test_call_with_arg(mock_sub, capsys): +def test_call_with_arg(mock_sub, capsys, sub_kw): """Any extra keyword arguments are passed through as-is.""" mock_sub.Popen(["hello", "world"], universal_newlines=True) + sub_kw.pop("text") mock_sub._subprocess.Popen.assert_called_with( ["hello", "world"], universal_newlines=True, - encoding=ANY, + **sub_kw, ) assert capsys.readouterr().out == "" -def test_call_with_path_arg(mock_sub, capsys, tmp_path): +def test_call_with_path_arg(mock_sub, capsys, tmp_path, sub_kw): """Path-based arguments are converted to strings and passed in as-is.""" mock_sub.Popen(["hello", tmp_path / "location"], cwd=tmp_path / "cwd") @@ -43,8 +40,7 @@ def test_call_with_path_arg(mock_sub, capsys, tmp_path): mock_sub._subprocess.Popen.assert_called_with( ["hello", os.fsdecode(tmp_path / "location")], cwd=os.fsdecode(tmp_path / "cwd"), - text=True, - encoding=ANY, + **sub_kw, ) assert capsys.readouterr().out == "" @@ -73,27 +69,24 @@ def test_call_with_start_new_session( platform, start_new_session, popen_kwargs, + sub_kw, ): """start_new_session is passed thru on Linux and macOS but converted for Windows.""" mock_sub.tools.host_os = platform mock_sub.Popen(["hello", "world"], start_new_session=start_new_session) + final_kwargs = {**sub_kw, **popen_kwargs} if platform == "Windows": mock_sub._subprocess.Popen.assert_called_with( - ["hello", "world"], - text=True, - encoding=ANY, - **popen_kwargs, + ["hello", "world"], **final_kwargs ) assert capsys.readouterr().out == "" else: mock_sub._subprocess.Popen.assert_called_with( ["hello", "world"], start_new_session=start_new_session, - text=True, - encoding=ANY, - **popen_kwargs, + **final_kwargs, ) assert capsys.readouterr().out == "" @@ -122,21 +115,19 @@ def test_call_windows_with_start_new_session_and_creationflags( AssertionError, match="Subprocess called with creationflags set" ): mock_sub.Popen( - ["hello", "world"], start_new_session=True, creationflags=creationflags + ["hello", "world"], + start_new_session=True, + creationflags=creationflags, ) -def test_debug_call(mock_sub, capsys): +def test_debug_call(mock_sub, capsys, sub_kw): """If verbosity is turned up, there is output.""" mock_sub.tools.logger.verbosity = 2 - mock_sub.Popen(["hello", "world"]) - mock_sub._subprocess.Popen.assert_called_with( - ["hello", "world"], - text=True, - encoding=ANY, - ) + mock_sub._subprocess.Popen.assert_called_with(["hello", "world"], **sub_kw) + assert capsys.readouterr().out == ( "\n" ">>> Running Command:\n" @@ -146,7 +137,7 @@ def test_debug_call(mock_sub, capsys): ) -def test_debug_call_with_env(mock_sub, capsys, tmp_path): +def test_debug_call_with_env(mock_sub, capsys, tmp_path, sub_kw): """If verbosity is turned up, and injected env vars are included output.""" mock_sub.tools.logger.verbosity = 2 @@ -160,8 +151,7 @@ def test_debug_call_with_env(mock_sub, capsys, tmp_path): ["hello", "world"], env=merged_env, cwd=os.fsdecode(tmp_path / "cwd"), - text=True, - encoding=ANY, + **sub_kw, ) expected_output = ( @@ -173,18 +163,20 @@ def test_debug_call_with_env(mock_sub, capsys, tmp_path): ">>> Environment Overrides:\n" ">>> NewVar=NewVarValue\n" ) - assert capsys.readouterr().out == expected_output @pytest.mark.parametrize( "in_kwargs, kwargs", [ - ({}, {"text": True, "encoding": ANY}), - ({"text": True}, {"text": True, "encoding": ANY}), + ({}, {"text": True, "encoding": ANY, "errors": "backslashreplace"}), + ({"text": True}, {"text": True, "encoding": ANY, "errors": "backslashreplace"}), ({"text": False}, {"text": False}), ({"universal_newlines": False}, {"universal_newlines": False}), - ({"universal_newlines": True}, {"universal_newlines": True, "encoding": ANY}), + ( + {"universal_newlines": True}, + {"universal_newlines": True, "encoding": ANY, "errors": "backslashreplace"}, + ), ], ) def test_text_eq_true_default_overriding(mock_sub, in_kwargs, kwargs): diff --git a/tests/integrations/subprocess/test_Subprocess__check_output.py b/tests/integrations/subprocess/test_Subprocess__check_output.py index c4ded1cd2..4e4eecc4a 100644 --- a/tests/integrations/subprocess/test_Subprocess__check_output.py +++ b/tests/integrations/subprocess/test_Subprocess__check_output.py @@ -10,7 +10,7 @@ @pytest.mark.parametrize("platform", ["Linux", "Darwin", "Windows"]) -def test_call(mock_sub, capsys, platform): +def test_call(mock_sub, capsys, platform, sub_check_output_kw): """A simple call will be invoked.""" mock_sub.tools.host_os = platform @@ -18,28 +18,26 @@ def test_call(mock_sub, capsys, platform): mock_sub._subprocess.check_output.assert_called_with( ["hello", "world"], - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) assert capsys.readouterr().out == "" -def test_call_with_arg(mock_sub, capsys): +def test_call_with_arg(mock_sub, capsys, sub_check_output_kw): """Any extra keyword arguments are passed through as-is.""" mock_sub.check_output(["hello", "world"], universal_newlines=True) + sub_check_output_kw.pop("text") mock_sub._subprocess.check_output.assert_called_with( ["hello", "world"], universal_newlines=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) assert capsys.readouterr().out == "" -def test_call_with_path_arg(mock_sub, capsys, tmp_path): +def test_call_with_path_arg(mock_sub, capsys, tmp_path, sub_check_output_kw): """Path-based arguments are converted to strings and passed in as-is.""" mock_sub.check_output(["hello", tmp_path / "location"], cwd=tmp_path / "cwd") @@ -47,9 +45,7 @@ def test_call_with_path_arg(mock_sub, capsys, tmp_path): mock_sub._subprocess.check_output.assert_called_with( ["hello", os.fsdecode(tmp_path / "location")], cwd=os.fsdecode(tmp_path / "cwd"), - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) assert capsys.readouterr().out == "" @@ -78,29 +74,26 @@ def test_call_with_start_new_session( platform, start_new_session, check_output_kwargs, + sub_check_output_kw, ): """start_new_session is passed thru on Linux and macOS but converted for Windows.""" mock_sub.tools.host_os = platform mock_sub.check_output(["hello", "world"], start_new_session=start_new_session) + final_kwargs = {**check_output_kwargs, **sub_check_output_kw} + if platform == "Windows": mock_sub._subprocess.check_output.assert_called_with( ["hello", "world"], - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, - **check_output_kwargs, + **final_kwargs, ) assert capsys.readouterr().out == "" else: mock_sub._subprocess.check_output.assert_called_with( ["hello", "world"], start_new_session=start_new_session, - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, - **check_output_kwargs, + **final_kwargs, ) assert capsys.readouterr().out == "" @@ -135,7 +128,7 @@ def test_call_windows_with_start_new_session_and_creationflags( ) -def test_debug_call(mock_sub, capsys): +def test_debug_call(mock_sub, capsys, sub_check_output_kw): """If verbosity is turned up, there is output.""" mock_sub.tools.logger.verbosity = 2 @@ -143,9 +136,7 @@ def test_debug_call(mock_sub, capsys): mock_sub._subprocess.check_output.assert_called_with( ["hello", "world"], - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) expected_output = ( @@ -163,7 +154,7 @@ def test_debug_call(mock_sub, capsys): assert capsys.readouterr().out == expected_output -def test_debug_call_with_env(mock_sub, capsys, tmp_path): +def test_debug_call_with_env(mock_sub, capsys, tmp_path, sub_check_output_kw): """If verbosity is turned up, injected env vars are included in output.""" mock_sub.tools.logger.verbosity = 2 @@ -177,9 +168,7 @@ def test_debug_call_with_env(mock_sub, capsys, tmp_path): ["hello", "world"], env=merged_env, cwd=os.fsdecode(tmp_path / "cwd"), - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) expected_output = ( @@ -199,7 +188,7 @@ def test_debug_call_with_env(mock_sub, capsys, tmp_path): assert capsys.readouterr().out == expected_output -def test_debug_call_with_quiet(mock_sub, capsys, tmp_path): +def test_debug_call_with_quiet(mock_sub, capsys, tmp_path, sub_check_output_kw): """If quiet mode is on, calls aren't logged, even if verbosity is turned up.""" mock_sub.tools.logger.verbosity = 2 @@ -218,16 +207,14 @@ def test_debug_call_with_quiet(mock_sub, capsys, tmp_path): ["hello", "world"], env=merged_env, cwd=os.fsdecode(tmp_path / "cwd"), - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) # No output assert capsys.readouterr().out == "" -def test_debug_call_with_stderr(mock_sub, capsys, tmp_path): +def test_debug_call_with_stderr(mock_sub, capsys, tmp_path, sub_check_output_kw): """If stderr is specified, it is not defaulted to stdout.""" mock_sub.tools.logger.verbosity = 2 @@ -237,12 +224,12 @@ def test_debug_call_with_stderr(mock_sub, capsys, tmp_path): stderr=subprocess.DEVNULL, ) + sub_check_output_kw.pop("stderr") mock_sub._subprocess.check_output.assert_called_with( ["hello", "world"], cwd=os.fsdecode(tmp_path / "cwd"), - text=True, - encoding=ANY, stderr=subprocess.DEVNULL, + **sub_check_output_kw, ) expected_output = ( @@ -308,7 +295,7 @@ def test_calledprocesserror_exception_quiet(mock_sub, capsys): with pytest.raises(CalledProcessError): mock_sub.check_output(["hello", "world"], quiet=True) - # No ouput in quiet mode + # No output in quiet mode assert capsys.readouterr().out == "" @@ -345,15 +332,43 @@ def test_calledprocesserror_exception_logging_no_cmd_output(mock_sub, capsys): @pytest.mark.parametrize( "in_kwargs, kwargs", [ - ({}, {"text": True, "encoding": ANY}), - ({"text": True}, {"text": True, "encoding": ANY}), - ({"text": False}, {"text": False}), - ({"universal_newlines": False}, {"universal_newlines": False}), - ({"universal_newlines": True}, {"universal_newlines": True, "encoding": ANY}), + ( + {}, + { + "text": True, + "encoding": ANY, + "stderr": subprocess.STDOUT, + "errors": "backslashreplace", + }, + ), + ( + {"text": True}, + { + "text": True, + "encoding": ANY, + "stderr": subprocess.STDOUT, + "errors": "backslashreplace", + }, + ), + ({"text": False}, {"text": False, "stderr": subprocess.STDOUT}), + ( + {"universal_newlines": False}, + {"universal_newlines": False, "stderr": subprocess.STDOUT}, + ), + ( + {"universal_newlines": True}, + { + "universal_newlines": True, + "encoding": ANY, + "stderr": subprocess.STDOUT, + "errors": "backslashreplace", + }, + ), ], ) def test_text_eq_true_default_overriding(mock_sub, in_kwargs, kwargs): - """if text or universal_newlines is explicitly provided, those should override + """If text or universal_newlines is explicitly provided, those should override text=true default.""" - mock_sub.check_output(["hello", "world"], stderr=subprocess.STDOUT, **in_kwargs) + + mock_sub._subprocess.check_output.assert_called_with(["hello", "world"], **kwargs) diff --git a/tests/integrations/subprocess/test_Subprocess__final_kwargs.py b/tests/integrations/subprocess/test_Subprocess__final_kwargs.py index a802ca06d..52d140092 100644 --- a/tests/integrations/subprocess/test_Subprocess__final_kwargs.py +++ b/tests/integrations/subprocess/test_Subprocess__final_kwargs.py @@ -15,6 +15,7 @@ def test_no_overrides(mock_sub): assert mock_sub.final_kwargs() == { "text": True, "encoding": CONSOLE_ENCODING, + "errors": "backslashreplace", } @@ -28,6 +29,7 @@ def test_explicit_no_overrides(mock_sub): }, "text": True, "encoding": CONSOLE_ENCODING, + "errors": "backslashreplace", } @@ -43,6 +45,7 @@ def test_env_overrides(mock_sub): }, "text": True, "encoding": CONSOLE_ENCODING, + "errors": "backslashreplace", } @@ -52,6 +55,7 @@ def test_cwd_provided(mock_sub): assert mock_sub.final_kwargs(cwd=cwd_override) == { "text": True, "encoding": CONSOLE_ENCODING, + "errors": "backslashreplace", "cwd": cwd_override, } @@ -62,6 +66,7 @@ def test_non_str_cwd_provided(mock_sub): assert mock_sub.final_kwargs(cwd=cwd_override) == { "text": True, "encoding": CONSOLE_ENCODING, + "errors": "backslashreplace", "cwd": str(cwd_override), } @@ -69,25 +74,52 @@ def test_non_str_cwd_provided(mock_sub): @pytest.mark.parametrize( "in_kwargs, final_kwargs", [ - ({}, {"text": True, "encoding": CONSOLE_ENCODING}), - ({"text": True}, {"text": True, "encoding": CONSOLE_ENCODING}), + ( + {}, + {"text": True, "encoding": CONSOLE_ENCODING, "errors": "backslashreplace"}, + ), + ( + {"text": True}, + {"text": True, "encoding": CONSOLE_ENCODING, "errors": "backslashreplace"}, + ), ({"text": False}, {"text": False}), ({"universal_newlines": False}, {"universal_newlines": False}), ( {"universal_newlines": True}, - {"universal_newlines": True, "encoding": CONSOLE_ENCODING}, + { + "universal_newlines": True, + "encoding": CONSOLE_ENCODING, + "errors": "backslashreplace", + }, + ), + # Explicit encoding provided + ( + {"encoding": "ibm850"}, + {"text": True, "encoding": "ibm850", "errors": "backslashreplace"}, + ), + ( + {"text": True, "encoding": "ibm850"}, + {"text": True, "encoding": "ibm850", "errors": "backslashreplace"}, + ), + ( + {"text": False, "encoding": "ibm850"}, + {"text": False, "encoding": "ibm850", "errors": "backslashreplace"}, ), - # Explcit encoding provided - ({"encoding": "ibm850"}, {"text": True, "encoding": "ibm850"}), - ({"text": True, "encoding": "ibm850"}, {"text": True, "encoding": "ibm850"}), - ({"text": False, "encoding": "ibm850"}, {"text": False, "encoding": "ibm850"}), ( {"universal_newlines": False, "encoding": "ibm850"}, - {"universal_newlines": False, "encoding": "ibm850"}, + { + "universal_newlines": False, + "encoding": "ibm850", + "errors": "backslashreplace", + }, ), ( {"universal_newlines": True, "encoding": "ibm850"}, - {"universal_newlines": True, "encoding": "ibm850"}, + { + "universal_newlines": True, + "encoding": "ibm850", + "errors": "backslashreplace", + }, ), ], ) diff --git a/tests/integrations/subprocess/test_Subprocess__parse_output.py b/tests/integrations/subprocess/test_Subprocess__parse_output.py index 2c81b729c..6c0dd526d 100644 --- a/tests/integrations/subprocess/test_Subprocess__parse_output.py +++ b/tests/integrations/subprocess/test_Subprocess__parse_output.py @@ -27,23 +27,20 @@ def third_line_parser(data): raise ParseError("Input does not contain 3 lines") -def test_call(mock_sub, capsys): +def test_call(mock_sub, capsys, sub_check_output_kw): """A simple call to check_output will be invoked.""" output = mock_sub.parse_output(splitlines_parser, ["hello", "world"]) mock_sub._subprocess.check_output.assert_called_with( ["hello", "world"], - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) assert capsys.readouterr().out == "" - assert output == ["some output line 1", "more output line 2"] -def test_call_with_arg(mock_sub, capsys): +def test_call_with_arg(mock_sub, capsys, sub_check_output_kw): """Any extra keyword arguments are passed through as-is to check_output.""" output = mock_sub.parse_output( @@ -53,30 +50,26 @@ def test_call_with_arg(mock_sub, capsys): mock_sub._subprocess.check_output.assert_called_with( ["hello", "world"], extra_arg="asdf", - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) assert capsys.readouterr().out == "" - assert output == ["some output line 1", "more output line 2"] -def test_call_with_parser_success(mock_sub, capsys): +def test_call_with_parser_success(mock_sub, capsys, sub_check_output_kw): """Parser returns expected portion of check_output's output.""" output = mock_sub.parse_output(second_line_parser, ["hello", "world"]) mock_sub._subprocess.check_output.assert_called_with( ["hello", "world"], - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) + assert output == "more output line 2" -def test_call_with_parser_error(mock_sub, capsys): +def test_call_with_parser_error(mock_sub, capsys, sub_check_output_kw): """Parser errors on output from check_output.""" with pytest.raises( @@ -87,10 +80,9 @@ def test_call_with_parser_error(mock_sub, capsys): mock_sub._subprocess.check_output.assert_called_with( ["hello", "world"], - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) + expected_output = ( "\n" "Command Output Parsing Error:\n" @@ -107,11 +99,14 @@ def test_call_with_parser_error(mock_sub, capsys): @pytest.mark.parametrize( "in_kwargs, kwargs", [ - ({}, {"text": True, "encoding": ANY}), - ({"text": True}, {"text": True, "encoding": ANY}), + ({}, {"text": True, "encoding": ANY, "errors": "backslashreplace"}), + ({"text": True}, {"text": True, "encoding": ANY, "errors": "backslashreplace"}), ({"text": False}, {"text": False}), ({"universal_newlines": False}, {"universal_newlines": False}), - ({"universal_newlines": True}, {"universal_newlines": True, "encoding": ANY}), + ( + {"universal_newlines": True}, + {"universal_newlines": True, "encoding": ANY, "errors": "backslashreplace"}, + ), ], ) def test_text_eq_true_default_overriding(mock_sub, in_kwargs, kwargs): @@ -119,6 +114,9 @@ def test_text_eq_true_default_overriding(mock_sub, in_kwargs, kwargs): text=true default.""" mock_sub.parse_output(splitlines_parser, ["hello", "world"], **in_kwargs) + mock_sub._subprocess.check_output.assert_called_with( - ["hello", "world"], stderr=subprocess.STDOUT, **kwargs + ["hello", "world"], + stderr=subprocess.STDOUT, + **kwargs, ) diff --git a/tests/integrations/subprocess/test_Subprocess__run__controlled_console.py b/tests/integrations/subprocess/test_Subprocess__run__controlled_console.py index ad14647ee..ba8468756 100644 --- a/tests/integrations/subprocess/test_Subprocess__run__controlled_console.py +++ b/tests/integrations/subprocess/test_Subprocess__run__controlled_console.py @@ -6,20 +6,13 @@ import pytest -def test_call(mock_sub, capsys): +def test_call(mock_sub, capsys, sub_stream_kw): """A simple call will be invoked.""" with mock_sub.tools.input.wait_bar(): mock_sub.run(["hello", "world"]) - mock_sub._subprocess.Popen.assert_called_with( - ["hello", "world"], - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, - text=True, - encoding=ANY, - ) + mock_sub._subprocess.Popen.assert_called_with(["hello", "world"], **sub_stream_kw) # fmt: off expected_output = ( "output line 1\n" @@ -31,19 +24,17 @@ def test_call(mock_sub, capsys): assert capsys.readouterr().out == expected_output -def test_call_with_arg(mock_sub, capsys): +def test_call_with_arg(mock_sub, capsys, sub_stream_kw): """Any extra keyword arguments are passed through as-is.""" with mock_sub.tools.input.wait_bar(): mock_sub.run(["hello", "world"], universal_newlines=True) + sub_stream_kw.pop("text") mock_sub._subprocess.Popen.assert_called_with( ["hello", "world"], - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, universal_newlines=True, - encoding=ANY, + **sub_stream_kw, ) # fmt: off expected_output = ( @@ -56,21 +47,14 @@ def test_call_with_arg(mock_sub, capsys): assert capsys.readouterr().out == expected_output -def test_debug_call(mock_sub, capsys): +def test_debug_call(mock_sub, capsys, sub_stream_kw): """If verbosity is turned up, there is debug output.""" mock_sub.tools.logger.verbosity = 2 with mock_sub.tools.input.wait_bar(): mock_sub.run(["hello", "world"]) - mock_sub._subprocess.Popen.assert_called_with( - ["hello", "world"], - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, - text=True, - encoding=ANY, - ) + mock_sub._subprocess.Popen.assert_called_with(["hello", "world"], **sub_stream_kw) expected_output = ( "\n" ">>> Running Command:\n" @@ -86,7 +70,7 @@ def test_debug_call(mock_sub, capsys): assert capsys.readouterr().out == expected_output -def test_debug_call_with_env(mock_sub, capsys, tmp_path): +def test_debug_call_with_env(mock_sub, capsys, tmp_path, sub_stream_kw): """If verbosity is turned up, injected env vars are included in debug output.""" mock_sub.tools.logger.verbosity = 2 @@ -105,11 +89,7 @@ def test_debug_call_with_env(mock_sub, capsys, tmp_path): ["hello", "world"], env=merged_env, cwd=os.fsdecode(tmp_path / "cwd"), - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, - text=True, - encoding=ANY, + **sub_stream_kw, ) expected_output = ( "\n" @@ -131,14 +111,25 @@ def test_debug_call_with_env(mock_sub, capsys, tmp_path): @pytest.mark.parametrize( "in_kwargs, kwargs", [ - ({}, {"text": True, "encoding": ANY, "bufsize": 1}), - ({"text": True}, {"text": True, "encoding": ANY, "bufsize": 1}), + ( + {}, + {"text": True, "encoding": ANY, "bufsize": 1, "errors": "backslashreplace"}, + ), + ( + {"text": True}, + {"text": True, "encoding": ANY, "bufsize": 1, "errors": "backslashreplace"}, + ), ({"text": False}, {"text": False, "bufsize": 1}), ({"text": False, "bufsize": 42}, {"text": False, "bufsize": 42}), ({"universal_newlines": False}, {"universal_newlines": False, "bufsize": 1}), ( {"universal_newlines": True}, - {"universal_newlines": True, "encoding": ANY, "bufsize": 1}, + { + "universal_newlines": True, + "encoding": ANY, + "bufsize": 1, + "errors": "backslashreplace", + }, ), ], ) @@ -156,7 +147,7 @@ def test_text_eq_true_default_overriding(mock_sub, in_kwargs, kwargs): ) -def test_stderr_is_redirected(mock_sub, streaming_process, capsys): +def test_stderr_is_redirected(mock_sub, streaming_process, sub_stream_kw, capsys): """When stderr is redirected, it should be included in the result.""" stderr_output = "stderr output\nline 2" streaming_process.stderr.read.return_value = stderr_output @@ -167,13 +158,11 @@ def test_stderr_is_redirected(mock_sub, streaming_process, capsys): stderr=subprocess.PIPE, ) + sub_stream_kw.pop("stderr") mock_sub._subprocess.Popen.assert_called_with( ["hello", "world"], - stdout=subprocess.PIPE, stderr=subprocess.PIPE, - bufsize=1, - text=True, - encoding=ANY, + **sub_stream_kw, ) # fmt: off expected_output = ( @@ -187,7 +176,7 @@ def test_stderr_is_redirected(mock_sub, streaming_process, capsys): assert run_result.stderr == stderr_output -def test_stderr_dev_null(mock_sub, streaming_process, capsys): +def test_stderr_dev_null(mock_sub, streaming_process, capsys, sub_stream_kw): """When stderr is discarded, it should be None in the result.""" streaming_process.stderr = None @@ -197,13 +186,11 @@ def test_stderr_dev_null(mock_sub, streaming_process, capsys): stderr=subprocess.DEVNULL, ) + sub_stream_kw.pop("stderr") mock_sub._subprocess.Popen.assert_called_with( ["hello", "world"], - stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, - bufsize=1, - text=True, - encoding=ANY, + **sub_stream_kw, ) # fmt: off expected_output = ( diff --git a/tests/integrations/subprocess/test_Subprocess__run__stream_output__False.py b/tests/integrations/subprocess/test_Subprocess__run__stream_output__False.py index a95b855ae..a4cdcc1c7 100644 --- a/tests/integrations/subprocess/test_Subprocess__run__stream_output__False.py +++ b/tests/integrations/subprocess/test_Subprocess__run__stream_output__False.py @@ -9,34 +9,31 @@ @pytest.mark.parametrize("platform", ["Linux", "Darwin", "Windows"]) -def test_call(mock_sub, capsys, platform): +def test_call(mock_sub, capsys, platform, sub_kw): """A simple call will be invoked.""" mock_sub.tools.sys.platform = platform mock_sub.run(["hello", "world"], stream_output=False) - mock_sub._subprocess.run.assert_called_with( - ["hello", "world"], - text=True, - encoding=ANY, - ) + mock_sub._subprocess.run.assert_called_with(["hello", "world"], **sub_kw) assert capsys.readouterr().out == "" -def test_call_with_arg(mock_sub, capsys): +def test_call_with_arg(mock_sub, capsys, sub_kw): """Any extra keyword arguments are passed through as-is.""" mock_sub.run(["hello", "world"], universal_newlines=True, stream_output=False) + sub_kw.pop("text") mock_sub._subprocess.run.assert_called_with( ["hello", "world"], universal_newlines=True, - encoding=ANY, + **sub_kw, ) assert capsys.readouterr().out == "" -def test_call_with_path_arg(mock_sub, capsys, tmp_path): +def test_call_with_path_arg(mock_sub, capsys, tmp_path, sub_kw): """Path-based arguments are converted to strings and passed in as-is.""" mock_sub.run( @@ -48,8 +45,7 @@ def test_call_with_path_arg(mock_sub, capsys, tmp_path): mock_sub._subprocess.run.assert_called_with( ["hello", os.fsdecode(tmp_path / "location")], cwd=os.fsdecode(tmp_path / "cwd"), - text=True, - encoding=ANY, + **sub_kw, ) assert capsys.readouterr().out == "" @@ -78,6 +74,7 @@ def test_call_with_start_new_session( platform, start_new_session, run_kwargs, + sub_kw, ): """start_new_session is passed thru on Linux and macOS but converted for Windows.""" @@ -88,21 +85,16 @@ def test_call_with_start_new_session( stream_output=False, ) + final_kwargs = {**run_kwargs, **sub_kw} + if platform == "Windows": - mock_sub._subprocess.run.assert_called_with( - ["hello", "world"], - text=True, - encoding=ANY, - **run_kwargs, - ) + mock_sub._subprocess.run.assert_called_with(["hello", "world"], **final_kwargs) assert capsys.readouterr().out == "" else: mock_sub._subprocess.run.assert_called_with( ["hello", "world"], start_new_session=start_new_session, - text=True, - encoding=ANY, - **run_kwargs, + **final_kwargs, ) assert capsys.readouterr().out == "" @@ -138,17 +130,13 @@ def test_call_windows_with_start_new_session_and_creationflags( ) -def test_debug_call(mock_sub, capsys): +def test_debug_call(mock_sub, capsys, sub_kw): """If verbosity is turned up, there is output.""" mock_sub.tools.logger.verbosity = 2 mock_sub.run(["hello", "world"], stream_output=False) - mock_sub._subprocess.run.assert_called_with( - ["hello", "world"], - text=True, - encoding=ANY, - ) + mock_sub._subprocess.run.assert_called_with(["hello", "world"], **sub_kw) # fmt: off expected_output = ( "\n" @@ -163,7 +151,7 @@ def test_debug_call(mock_sub, capsys): assert capsys.readouterr().out == expected_output -def test_debug_call_with_env(mock_sub, capsys, tmp_path): +def test_debug_call_with_env(mock_sub, capsys, tmp_path, sub_kw): """If verbosity is turned up, injected env vars are included output.""" mock_sub.tools.logger.verbosity = 2 @@ -177,8 +165,7 @@ def test_debug_call_with_env(mock_sub, capsys, tmp_path): ["hello", "world"], env=merged_env, cwd=os.fsdecode(tmp_path / "cwd"), - text=True, - encoding=ANY, + **sub_kw, ) expected_output = ( "\n" @@ -222,11 +209,14 @@ def test_calledprocesserror_exception_logging(mock_sub, capsys): @pytest.mark.parametrize( "in_kwargs, kwargs", [ - ({}, {"text": True, "encoding": ANY}), - ({"text": True}, {"text": True, "encoding": ANY}), + ({}, {"text": True, "encoding": ANY, "errors": "backslashreplace"}), + ({"text": True}, {"text": True, "encoding": ANY, "errors": "backslashreplace"}), ({"text": False}, {"text": False}), ({"universal_newlines": False}, {"universal_newlines": False}), - ({"universal_newlines": True}, {"universal_newlines": True, "encoding": ANY}), + ( + {"universal_newlines": True}, + {"universal_newlines": True, "encoding": ANY, "errors": "backslashreplace"}, + ), ], ) def test_text_eq_true_default_overriding(mock_sub, in_kwargs, kwargs): diff --git a/tests/integrations/subprocess/test_Subprocess__run__stream_output__True.py b/tests/integrations/subprocess/test_Subprocess__run__stream_output__True.py index bb7c15c05..7d1d6de0c 100644 --- a/tests/integrations/subprocess/test_Subprocess__run__stream_output__True.py +++ b/tests/integrations/subprocess/test_Subprocess__run__stream_output__True.py @@ -10,20 +10,13 @@ @pytest.mark.parametrize("platform", ["Linux", "Darwin", "Windows"]) -def test_call(mock_sub, capsys, platform): +def test_call(mock_sub, capsys, platform, sub_stream_kw): """A simple call will be invoked.""" mock_sub.tools.sys.platform = platform mock_sub.run(["hello", "world"]) - mock_sub._subprocess.Popen.assert_called_with( - ["hello", "world"], - text=True, - encoding=ANY, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, - ) + mock_sub._subprocess.Popen.assert_called_with(["hello", "world"], **sub_stream_kw) # fmt: off expected_output = ( "output line 1\n" @@ -34,18 +27,16 @@ def test_call(mock_sub, capsys, platform): assert capsys.readouterr().out == expected_output -def test_call_with_arg(mock_sub, capsys): +def test_call_with_arg(mock_sub, capsys, sub_stream_kw): """Any extra keyword arguments are passed through as-is.""" mock_sub.run(["hello", "world"], universal_newlines=True) + sub_stream_kw.pop("text") mock_sub._subprocess.Popen.assert_called_with( ["hello", "world"], universal_newlines=True, - encoding=ANY, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, + **sub_stream_kw, ) # fmt: off expected_output = ( @@ -57,7 +48,7 @@ def test_call_with_arg(mock_sub, capsys): assert capsys.readouterr().out == expected_output -def test_call_with_path_arg(mock_sub, capsys, tmp_path): +def test_call_with_path_arg(mock_sub, capsys, tmp_path, sub_stream_kw): """Path-based arguments are converted to strings and passed in as-is.""" mock_sub.run(["hello", tmp_path / "location"], cwd=tmp_path / "cwd") @@ -65,11 +56,7 @@ def test_call_with_path_arg(mock_sub, capsys, tmp_path): mock_sub._subprocess.Popen.assert_called_with( ["hello", os.fsdecode(tmp_path / "location")], cwd=os.fsdecode(tmp_path / "cwd"), - text=True, - encoding=ANY, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, + **sub_stream_kw, ) # fmt: off expected_output = ( @@ -105,32 +92,24 @@ def test_call_with_start_new_session( platform, start_new_session, run_kwargs, + sub_stream_kw, ): """start_new_session is passed thru on Linux and macOS but converted for Windows.""" mock_sub.tools.host_os = platform mock_sub.run(["hello", "world"], start_new_session=start_new_session) + final_kwargs = {**sub_stream_kw, **run_kwargs} if platform == "Windows": mock_sub._subprocess.Popen.assert_called_with( ["hello", "world"], - text=True, - encoding=ANY, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, - **run_kwargs, + **final_kwargs, ) else: mock_sub._subprocess.Popen.assert_called_with( ["hello", "world"], start_new_session=start_new_session, - text=True, - encoding=ANY, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, - **run_kwargs, + **final_kwargs, ) # fmt: off @@ -174,20 +153,13 @@ def test_call_windows_with_start_new_session_and_creationflags( ) -def test_debug_call(mock_sub, capsys): +def test_debug_call(mock_sub, capsys, sub_stream_kw): """If verbosity is turned up, there is output.""" mock_sub.tools.logger.verbosity = 2 mock_sub.run(["hello", "world"]) - mock_sub._subprocess.Popen.assert_called_with( - ["hello", "world"], - text=True, - encoding=ANY, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, - ) + mock_sub._subprocess.Popen.assert_called_with(["hello", "world"], **sub_stream_kw) # fmt: off expected_output = ( "\n" @@ -205,7 +177,7 @@ def test_debug_call(mock_sub, capsys): assert capsys.readouterr().out == expected_output -def test_debug_call_with_env(mock_sub, capsys, tmp_path): +def test_debug_call_with_env(mock_sub, capsys, tmp_path, sub_stream_kw): """If verbosity is turned up, injected env vars are included output.""" mock_sub.tools.logger.verbosity = 2 @@ -219,11 +191,7 @@ def test_debug_call_with_env(mock_sub, capsys, tmp_path): ["hello", "world"], env=merged_env, cwd=os.fsdecode(tmp_path / "cwd"), - text=True, - encoding=ANY, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, + **sub_stream_kw, ) expected_output = ( "\n" @@ -266,11 +234,14 @@ def test_calledprocesserror_exception_logging(mock_sub, capsys): @pytest.mark.parametrize( "in_kwargs, kwargs", [ - ({}, {"text": True, "encoding": ANY}), - ({"text": True}, {"text": True, "encoding": ANY}), + ({}, {"text": True, "encoding": ANY, "errors": "backslashreplace"}), + ({"text": True}, {"text": True, "encoding": ANY, "errors": "backslashreplace"}), ({"text": False}, {"text": False}), ({"universal_newlines": False}, {"universal_newlines": False}), - ({"universal_newlines": True}, {"universal_newlines": True, "encoding": ANY}), + ( + {"universal_newlines": True}, + {"universal_newlines": True, "encoding": ANY, "errors": "backslashreplace"}, + ), ], ) def test_text_eq_true_default_overriding(mock_sub, in_kwargs, kwargs): diff --git a/tests/integrations/subprocess/test_ensure_console_is_safe.py b/tests/integrations/subprocess/test_ensure_console_is_safe.py index 68a4c4cb9..e70d97d5e 100644 --- a/tests/integrations/subprocess/test_ensure_console_is_safe.py +++ b/tests/integrations/subprocess/test_ensure_console_is_safe.py @@ -1,5 +1,5 @@ import subprocess -from unittest.mock import ANY, MagicMock, Mock +from unittest.mock import MagicMock, Mock import pytest @@ -31,7 +31,7 @@ def test_run_windows_batch_script(mock_sub, batch_script): @pytest.mark.parametrize("batch_script", ["HELLO.BAT", "hello.bat", "hElLo.BaT"]) -def test_check_output_windows_batch_script(mock_sub, batch_script): +def test_check_output_windows_batch_script(mock_sub, batch_script, sub_check_output_kw): """Console control is released for a Windows batch script in check_output.""" # Console control is only released on Windows for batch scripts mock_sub.tools.host_os = "Windows" @@ -41,9 +41,7 @@ def test_check_output_windows_batch_script(mock_sub, batch_script): mock_sub._subprocess.check_output.assert_called_with( [batch_script, "World"], - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, + **sub_check_output_kw, ) mock_sub.tools.input.release_console_control.assert_called_once() @@ -58,16 +56,12 @@ def test_run_stream_output_true(mock_sub, sub_kwargs): mock_sub.tools.input.release_console_control.assert_not_called() -def test_run_stream_output_false(mock_sub): +def test_run_stream_output_false(mock_sub, sub_kw): """Console control is released when stream_output=False.""" with mock_sub.tools.input.wait_bar("Testing..."): mock_sub.run(["Hello", "World"], stream_output=False) - mock_sub._subprocess.run.assert_called_with( - ["Hello", "World"], - text=True, - encoding=ANY, - ) + mock_sub._subprocess.run.assert_called_with(["Hello", "World"], **sub_kw) mock_sub.tools.input.release_console_control.assert_called_once() @@ -79,21 +73,21 @@ def test_run_stream_output_false(mock_sub): (["Hello", "World"], dict(val1="value1", val2="value2")), ], ) -def test_negative_condition_not_controlled(mock_sub, cmdline, kwargs): +def test_negative_condition_not_controlled( + mock_sub, + cmdline, + sub_check_output_kw, + kwargs, +): """Passthrough to Subprocess if conditions to release console control are not met while the console is not controlled.""" mock_sub.run(cmdline, **kwargs) mock_sub._run_and_stream_output.assert_called_with(cmdline, **kwargs) mock_sub.check_output(cmdline, **kwargs) - mock_sub._subprocess.check_output.assert_called_with( - cmdline, - **kwargs, - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, - ) + final_kwargs = {**sub_check_output_kw, **kwargs} + mock_sub._subprocess.check_output.assert_called_with(cmdline, **final_kwargs) mock_sub.tools.input.release_console_control.assert_not_called() @@ -105,7 +99,7 @@ def test_negative_condition_not_controlled(mock_sub, cmdline, kwargs): (["Hello", "World"], dict(val1="value1", val2="value2")), ], ) -def test_negative_condition_controlled(mock_sub, cmdline, kwargs): +def test_negative_condition_controlled(mock_sub, cmdline, kwargs, sub_check_output_kw): """Passthrough to Subprocess if conditions to release console control are not met while the console is controlled.""" with mock_sub.tools.input.wait_bar("Testing..."): @@ -113,11 +107,6 @@ def test_negative_condition_controlled(mock_sub, cmdline, kwargs): mock_sub.check_output(cmdline, **kwargs) mock_sub._run_and_stream_output.assert_called_with(cmdline, **kwargs) - mock_sub._subprocess.check_output.assert_called_with( - cmdline, - **kwargs, - text=True, - encoding=ANY, - stderr=subprocess.STDOUT, - ) + final_kwargs = {**sub_check_output_kw, **kwargs} + mock_sub._subprocess.check_output.assert_called_with(cmdline, **final_kwargs) mock_sub.tools.input.release_console_control.assert_not_called() diff --git a/tests/platforms/linux/appimage/test_build.py b/tests/platforms/linux/appimage/test_build.py index 55d959415..0b3d503a4 100644 --- a/tests/platforms/linux/appimage/test_build.py +++ b/tests/platforms/linux/appimage/test_build.py @@ -146,7 +146,7 @@ def test_verify_tools_download_failure(build_command): assert build_command.build_app.call_count == 0 -def test_build_appimage(build_command, first_app, tmp_path): +def test_build_appimage(build_command, first_app, tmp_path, sub_stream_kw): """A Linux app can be packaged as an AppImage.""" build_command.verify_app_tools(first_app) @@ -190,11 +190,7 @@ def test_build_appimage(build_command, first_app, tmp_path): cwd=os.fsdecode( tmp_path / "base_path" / "build" / "first-app" / "linux" / "appimage" ), - text=True, - encoding=mock.ANY, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, + **sub_stream_kw, ) # Binary is marked executable build_command.tools.os.chmod.assert_called_with( @@ -213,7 +209,7 @@ def test_build_appimage(build_command, first_app, tmp_path): sys.platform == "win32", reason="Windows paths can't be passed to linuxdeploy", ) -def test_build_appimage_with_plugin(build_command, first_app, tmp_path): +def test_build_appimage_with_plugin(build_command, first_app, tmp_path, sub_stream_kw): """A Linux app can be packaged as an AppImage with a plugin.""" # Mock the existence of some plugins gtk_plugin_path = ( @@ -284,11 +280,7 @@ def test_build_appimage_with_plugin(build_command, first_app, tmp_path): cwd=os.fsdecode( tmp_path / "base_path" / "build" / "first-app" / "linux" / "appimage" ), - text=True, - encoding=mock.ANY, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, + **sub_stream_kw, ) # Local plugin marked executable build_command.tools.os.chmod.assert_any_call( @@ -314,7 +306,7 @@ def test_build_appimage_with_plugin(build_command, first_app, tmp_path): ) -def test_build_failure(build_command, first_app, tmp_path): +def test_build_failure(build_command, first_app, tmp_path, sub_stream_kw): """If linuxdeploy fails, the build is stopped.""" # Mock a failure in the build build_command._subprocess.Popen.side_effect = subprocess.CalledProcessError( @@ -365,11 +357,7 @@ def test_build_failure(build_command, first_app, tmp_path): cwd=os.fsdecode( tmp_path / "base_path" / "build" / "first-app" / "linux" / "appimage" ), - text=True, - encoding=mock.ANY, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, + **sub_stream_kw, ) # chmod isn't invoked if the binary wasn't created. @@ -379,7 +367,7 @@ def test_build_failure(build_command, first_app, tmp_path): @pytest.mark.skipif( sys.platform == "win32", reason="Windows paths aren't converted in Docker context" ) -def test_build_appimage_in_docker(build_command, first_app, tmp_path, monkeypatch): +def test_build_appimage_in_docker(build_command, first_app, tmp_path, sub_stream_kw): """A Linux app can be packaged as an AppImage in a docker container.""" # Enable docker, and move to a non-Linux OS. @@ -448,11 +436,7 @@ def test_build_appimage_in_docker(build_command, first_app, tmp_path, monkeypatc "--deploy-deps-only", "/app/First App.AppDir/usr/app_packages/secondlib", ], - text=True, - encoding=mock.ANY, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, + **sub_stream_kw, ) # Binary is marked executable build_command.tools.os.chmod.assert_called_with( @@ -470,7 +454,12 @@ def test_build_appimage_in_docker(build_command, first_app, tmp_path, monkeypatc @pytest.mark.skipif( sys.platform == "win32", reason="Windows paths aren't converted in Docker context" ) -def test_build_appimage_with_plugins_in_docker(build_command, first_app, tmp_path): +def test_build_appimage_with_plugins_in_docker( + build_command, + first_app, + tmp_path, + sub_stream_kw, +): """A Linux app can be packaged as an AppImage with plugins in a Docker container.""" # Mock the existence of some plugins gtk_plugin_path = ( @@ -573,11 +562,7 @@ def test_build_appimage_with_plugins_in_docker(build_command, first_app, tmp_pat "--plugin", "something", ], - text=True, - encoding=mock.ANY, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - bufsize=1, + **sub_stream_kw, ) # Local plugin marked executable build_command.tools.os.chmod.assert_any_call(