Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 40 additions & 0 deletions commitizen/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 2 additions & 7 deletions commitizen/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")


Expand Down
8 changes: 4 additions & 4 deletions tests/test_bump_hooks.py
Comment thread
PhilipNelson5 marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand Down
192 changes: 157 additions & 35 deletions tests/test_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading