diff --git a/commitizen/cmd.py b/commitizen/cmd.py index effe1ff37..f2b98502d 100644 --- a/commitizen/cmd.py +++ b/commitizen/cmd.py @@ -103,3 +103,43 @@ def run_shell(cmd: str, env: Mapping[str, str] | None = None) -> Command: https://github.com/commitizen-tools/commitizen/issues/1918 """ return _popen(cmd, shell=True, env=env) + + +def run_interactive( + cmd: str | Sequence[str], env: Mapping[str, str] | None = None +) -> int: + """Run a command safely without shell interpretation and without redirecting stdin, stdout, or stderr + + Args: + cmd: The command to run + env: Extra environment variables to define in the subprocess. Defaults to None. + + Returns: + subprocess returncode + """ + if env is not None: + env = {**os.environ, **env} + if isinstance(cmd, str): + warnings.warn( + "Passing a string to cmd.run_interactive() is deprecated and will be removed in v5. " + "Use a list of arguments instead, or use cmd.run_interactive_shell() explicitly.", + DeprecationWarning, + stacklevel=2, + ) + return subprocess.run(cmd, shell=True, env=env).returncode + return subprocess.run(cmd, shell=False, env=env).returncode + + +def run_interactive_shell(cmd: str, env: Mapping[str, str] | None = None) -> int: + """Run a command without redirecting stdin, stdout, or stderr + + Args: + cmd: The command to run + env: Extra environment variables to define in the subprocess. Defaults to None. + + Returns: + subprocess returncode + """ + if env is not None: + env = {**os.environ, **env} + return subprocess.run(cmd, shell=True, env=env).returncode diff --git a/commitizen/hooks.py b/commitizen/hooks.py index ccb966672..cf9fe01d2 100644 --- a/commitizen/hooks.py +++ b/commitizen/hooks.py @@ -17,14 +17,9 @@ def run(hooks: str | list[str], _env_prefix: str = "CZ_", **env: object) -> None for hook in hooks: out.info(f"Running hook '{hook}'") - c = cmd.run_shell(hook, env=_format_env(_env_prefix, env)) + return_code = cmd.run_interactive(hook, env=_format_env(_env_prefix, env)) - if c.out: - out.write(c.out) - if c.err: - out.error(c.err) - - if c.return_code != 0: + if return_code != 0: raise RunHookError(f"Running hook '{hook}' failed") diff --git a/tests/test_bump_hooks.py b/tests/test_bump_hooks.py index 28f0db944..e73cb49ec 100644 --- a/tests/test_bump_hooks.py +++ b/tests/test_bump_hooks.py @@ -12,8 +12,8 @@ def test_run(mocker: MockFixture): bump_hooks = ["pre_bump_hook", "pre_bump_hook_1"] cmd_run_mock = mocker.Mock() - cmd_run_mock.return_value.return_code = 0 - mocker.patch.object(cmd, "run_shell", cmd_run_mock) + cmd_run_mock.return_value = 0 + mocker.patch.object(cmd, "run_interactive", cmd_run_mock) hooks.run(bump_hooks) @@ -29,8 +29,8 @@ def test_run_error(mocker: MockFixture): bump_hooks = ["pre_bump_hook", "pre_bump_hook_1"] cmd_run_mock = mocker.Mock() - cmd_run_mock.return_value.return_code = 1 - mocker.patch.object(cmd, "run_shell", cmd_run_mock) + cmd_run_mock.return_value = 1 + mocker.patch.object(cmd, "run_interactive", cmd_run_mock) with pytest.raises(RunHookError): hooks.run(bump_hooks) diff --git a/tests/test_cmd.py b/tests/test_cmd.py index 734c90091..c462067a5 100644 --- a/tests/test_cmd.py +++ b/tests/test_cmd.py @@ -57,39 +57,161 @@ def decode(self, encoding="utf-8", errors="strict"): cmd._try_decode(_bytes()) -def test_run_returns_command_with_shell_false(): - """Test that cmd.run executes a list-based command without shell.""" - c = cmd.run(["python", "-c", "print('hello')"]) - assert c.return_code == 0 - assert "hello" in c.out - - -def test_run_shell_returns_command_with_shell_true(): - """Test that cmd.run_shell executes a string command via the shell.""" - c = cmd.run_shell("python -c \"print('hello')\"") - assert c.return_code == 0 - assert "hello" in c.out - - -def test_run_with_env(): - """Test that cmd.run passes extra environment variables.""" - c = cmd.run( - ["python", "-c", "import os; print(os.environ['CZ_TEST_VAR'])"], - env={"CZ_TEST_VAR": "test_value"}, - ) - assert c.return_code == 0 - assert "test_value" in c.out - - -def test_run_with_string_emits_deprecation_warning(): - """Test that passing a string to cmd.run() emits a DeprecationWarning.""" - import warnings - - with warnings.catch_warnings(record=True) as w: - warnings.simplefilter("always") - c = cmd.run("python -c \"print('deprecated')\"") +class TestRun: + def test_returns_command_with_shell_false(self): + """Test that cmd.run executes a list-based command without shell.""" + c = cmd.run(["python", "-c", "print('hello')"]) + assert c.return_code == 0 + assert "hello" in c.out + + def test_with_env(self): + """Test that cmd.run passes extra environment variables.""" + c = cmd.run( + ["python", "-c", "import os; print(os.environ['CZ_TEST_VAR'])"], + env={"CZ_TEST_VAR": "test_value"}, + ) + assert c.return_code == 0 + assert "test_value" in c.out + + def test_with_string_emits_deprecation_warning(self): + """Test that passing a string to cmd.run() emits a DeprecationWarning.""" + import warnings + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + c = cmd.run("python -c \"print('deprecated')\"") + assert c.return_code == 0 + assert "deprecated" in c.out + assert len(w) == 1 + assert issubclass(w[0].category, DeprecationWarning) + assert "cmd.run()" in str(w[0].message) + + def test_stdout_captured(self): + result = cmd.run(["python", "-c", "print('hello')"]) + assert "hello" in result.out + assert isinstance(result.stdout, bytes) + assert b"hello" in result.stdout + + def test_stderr_captured(self): + result = cmd.run( + ["python", "-c", "import sys; print('err msg', file=sys.stderr)"] + ) + assert "err msg" in result.err + assert isinstance(result.stderr, bytes) + assert b"err msg" in result.stderr + + def test_zero_return_code_on_success(self): + result = cmd.run(["python", "-c", "import sys; sys.exit(0)"]) + assert result.return_code == 0 + + def test_nonzero_return_code_on_failure(self): + result = cmd.run(["python", "-c", "import sys; sys.exit(42)"]) + assert result.return_code == 42 + + def test_env_passed_to_subprocess(self): + result = cmd.run( + ["python", "-c", "import os; print(os.environ['CZ_TEST_VAR'])"], + env={"CZ_TEST_VAR": "sentinelvalue"}, + ) + assert "sentinelvalue" in result.out + assert result.return_code == 0 + + def test_env_merged_with_os_environ(self, monkeypatch): + monkeypatch.setenv("CZ_EXISTING_VAR", "fromenv") + result = cmd.run( + ["python", "-c", "import os; print(os.environ['CZ_EXISTING_VAR'])"], + env={"CZ_EXTRA_VAR": "extra"}, + ) + assert "fromenv" in result.out + + def test_empty_stdout_and_stderr(self): + result = cmd.run(["python", "-c", '"pass"']) + assert result.out == "" + assert result.err == "" + assert result.stdout == b"" + assert result.stderr == b"" + + def test_no_env_uses_os_environ(self, monkeypatch): + monkeypatch.setenv("CZ_NO_ENV_TEST", "inherited") + result = cmd.run( + ["python", "-c", "import os; print(os.environ['CZ_NO_ENV_TEST'])"] + ) + assert "inherited" in result.out + + +class TestRunShell: + def test_returns_command_with_shell_true(self): + """Test that cmd.run_shell executes a string command via the shell.""" + c = cmd.run_shell("python -c \"print('hello')\"") assert c.return_code == 0 - assert "deprecated" in c.out - assert len(w) == 1 - assert issubclass(w[0].category, DeprecationWarning) - assert "cmd.run()" in str(w[0].message) + assert "hello" in c.out + + +class TestRunInteractive: + def test_zero_return_code_on_success(self): + return_code = cmd.run_interactive(["python", "-c", "import sys; sys.exit(0)"]) + assert return_code == 0 + + def test_nonzero_return_code_on_failure(self): + return_code = cmd.run_interactive(["python", "-c", "import sys; sys.exit(3)"]) + assert return_code == 3 + + def test_env_passed_to_subprocess(self): + return_code = cmd.run_interactive( + [ + "python", + "-c", + "import os, sys; sys.exit(0 if os.environ['CZ_ITEST_VAR'] == 'val' else 1)", + ], + env={"CZ_ITEST_VAR": "val"}, + ) + assert return_code == 0 + + def test_env_merged_with_os_environ(self, monkeypatch): + monkeypatch.setenv("CZ_ITEST_EXISTING", "yes") + return_code = cmd.run_interactive( + [ + "python", + "-c", + "import os, sys; sys.exit(0 if os.environ['CZ_ITEST_EXISTING'] == 'yes' else 1)", + ], + env={"CZ_ITEST_EXTRA": "extra"}, + ) + assert return_code == 0 + + def test_runs_with_string(self): + """Test that passing a string to cmd.run_interactive emits a DeprecationWarning.""" + import warnings + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + return_code = cmd.run_interactive("python -c \"print('hello')\"") + assert return_code == 0 + assert len(w) == 1 + assert issubclass(w[0].category, DeprecationWarning) + assert "cmd.run_interactive()" in str(w[0].message) + + +class TestRunInteractiveShell: + def test_zero_return_code_on_success(self): + return_code = cmd.run_interactive_shell('python -c "import sys; sys.exit(0)"') + assert return_code == 0 + + def test_nonzero_return_code_on_failure(self): + return_code = cmd.run_interactive_shell('python -c "import sys; sys.exit(3)"') + assert return_code == 3 + + def test_env_passed_to_subprocess(self): + return_code = cmd.run_interactive_shell( + "python -c \"import os, sys; sys.exit(0 if os.environ['CZ_ITEST_VAR'] == 'val' else 1)\"", + env={"CZ_ITEST_VAR": "val"}, + ) + assert return_code == 0 + + def test_env_merged_with_os_environ(self, monkeypatch): + monkeypatch.setenv("CZ_ITEST_EXISTING", "yes") + return_code = cmd.run_interactive_shell( + "python -c \"import os, sys; sys.exit(0 if os.environ['CZ_ITEST_EXISTING'] == 'yes' else 1)\"", + env={"CZ_ITEST_EXTRA": "extra"}, + ) + assert return_code == 0