From d116a336a08642fb4b33fdba18e683256f697beb Mon Sep 17 00:00:00 2001
From: Lan Gong <lgong@modular.com>
Date: Wed, 22 Oct 2025 14:55:54 +0000
Subject: [PATCH] Capture stdout and stderr when quiet is on

Example usage: `stack-pr submit` returned with exit 1 due to an outdated version of `gh`.

```
$ stack-pr submit

Adding cross-links to PRs
Exitcode: 1
Stdout: None
Stderr: GraphQL: Projects (classic) is being deprecated in favor of the new Projects experience, see: https://github.blog/changelog/2024-05-23-sunset-notice-projects-classic/. (repository.pullRequest.projectCards)

subprocess.CalledProcessError: Command '['gh', 'pr', 'edit', 'https://github.com/modularml/modular/pull/70936', '-t', 'Test GH', '-F', '-', '-B', 'main']' returned non-zero exit status 1.
```

Test Plan:

```
python -m pytest tests/test_shell_commands.py -v
```
---
 src/stack_pr/shell_commands.py |  7 ++-
 tests/test_shell_commands.py   | 83 ++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100644 tests/test_shell_commands.py

diff --git a/src/stack_pr/shell_commands.py b/src/stack_pr/shell_commands.py
index ac12e30..d47c589 100644
--- a/src/stack_pr/shell_commands.py
+++ b/src/stack_pr/shell_commands.py
@@ -45,7 +45,12 @@ def run_shell_command(
         raise ValueError("shell support has been removed")
     _ = subprocess.list2cmdline(cmd)
     if quiet:
-        kwargs.update({"stdout": subprocess.DEVNULL, "stderr": subprocess.DEVNULL})
+        # If quiet, capture stdout and stderr so they are not printed to the console
+        # But respects explicit stderr/stdout settings at the call sites
+        if "stderr" not in kwargs:
+            kwargs["stderr"] = subprocess.PIPE
+        if "stdout" not in kwargs:
+            kwargs["stdout"] = subprocess.PIPE
     logger.debug("Running: %s", cmd)
     return subprocess.run(list(map(str, cmd)), **kwargs, check=check)
 
diff --git a/tests/test_shell_commands.py b/tests/test_shell_commands.py
new file mode 100644
index 0000000..64fc80d
--- /dev/null
+++ b/tests/test_shell_commands.py
@@ -0,0 +1,83 @@
+import subprocess
+import sys
+from pathlib import Path
+
+sys.path.append(str(Path(__file__).parent.parent / "src"))
+
+import pytest
+
+from stack_pr.shell_commands import run_shell_command
+
+
+def test_cmd_success_quiet_false_print(capfd: pytest.CaptureFixture) -> None:
+    """Test that stdout and stderr are printed when quiet=False on success."""
+    # Use a command that produces both stdout and stderr
+    # sh -c 'echo "out" && echo "err" >&2' produces both
+    result = run_shell_command(
+        ["sh", "-c", 'echo "stdout_msg" && echo "stderr_msg" >&2'],
+        quiet=False,
+    )
+
+    # stdout and stderr are not captured in memory
+    assert result.returncode == 0
+    assert result.stdout is None
+    assert result.stderr is None
+
+    # stdout and stderr are printed to console
+    captured = capfd.readouterr()
+    assert "stdout_msg" in captured.out
+    assert "stderr_msg" in captured.err
+
+
+def test_cmd_success_quiet_true_captured(capfd: pytest.CaptureFixture) -> None:
+    """Test that stdout and stderr are captured when quiet=True on success."""
+    result = run_shell_command(
+        ["sh", "-c", 'echo "stdout_msg" && echo "stderr_msg" >&2'],
+        quiet=True,
+    )
+
+    # stdout and stderr are captured in memory
+    assert result.returncode == 0
+    assert "stdout_msg" in result.stdout.decode("utf-8")
+    assert "stderr_msg" in result.stderr.decode("utf-8")
+
+    # stdout and stderr are not printed to console
+    captured = capfd.readouterr()
+    assert "stdout_msg" not in captured.out
+    assert "stderr_msg" not in captured.err
+
+
+def test_cmd_fail_quiet_true_captured(capfd: pytest.CaptureFixture) -> None:
+    """Test that stdout and stderr are caught by exception handling.
+
+    Tests behavior when quiet=True on failure.
+    """
+    with pytest.raises(subprocess.CalledProcessError) as exc:
+        run_shell_command(
+            ["sh", "-c", 'echo "stdout_msg" && echo "stderr_msg" >&2 && exit 1'],
+            quiet=True,
+        )
+
+    # stdout and stderr are captured in exception info
+    exception = exc.value
+    assert "stdout_msg" in exception.stdout.decode("utf-8")
+    assert "stderr_msg" in exception.stderr.decode("utf-8")
+
+    # stdout and stderr are not printed to console
+    captured = capfd.readouterr()
+    assert captured.out == ""
+    assert captured.err == ""
+
+
+def test_cmd_fail_quiet_false_print(capfd: pytest.CaptureFixture) -> None:
+    """Test that stdout and stderr are printed when quiet=False on failure."""
+    with pytest.raises(subprocess.CalledProcessError):
+        run_shell_command(
+            ["sh", "-c", 'echo "stdout_msg" && echo "stderr_msg" >&2 && exit 1'],
+            quiet=False,
+        )
+
+    # stdout and stderr are printed to console
+    captured = capfd.readouterr()
+    assert "stdout_msg" in captured.out
+    assert "stderr_msg" in captured.err