feat: change subprocess.run to print output while manager is running

comfy_cli/command/custom_nodes/cm_cli_util.py

@@ -55,28 +55,38 @@ def execute_cm_cli(args, channel=None, fast_deps=False, no_deps=False, mode=None
     session_path = os.path.join(_config_manager.get_config_path(), "tmp", str(uuid.uuid4()))
     new_env["__COMFY_CLI_SESSION__"] = session_path
     new_env["COMFYUI_PATH"] = workspace_path
+    new_env["PYTHONUNBUFFERED"] = "1"
 
     print(f"Execute from: {workspace_path}")
     print(f"Command: {cmd}")
     try:
-        result = subprocess.run(
-            cmd, env=new_env, check=True, capture_output=True, text=True, encoding="utf-8", errors="replace"
+        process = subprocess.Popen(
+            cmd,
+            env=new_env,
+            stdout=subprocess.PIPE,
+            text=True,
+            encoding="utf-8",
+            errors="replace",
         )
-        print(result.stdout)
+        stdout_lines = []
+        for line in process.stdout:
+            sys.stdout.write(line)
+            sys.stdout.flush()
+            stdout_lines.append(line)
+        return_code = process.wait()
+        stdout_output = "".join(stdout_lines)
+        if return_code != 0:
+            raise subprocess.CalledProcessError(return_code, cmd, output=stdout_output)
 
         if fast_deps and args[0] in _dependency_cmds:
             # we're using the fast_deps behavior and just ran a command that invalidated the dependencies
             depComp = DependencyCompiler(cwd=workspace_path, executable=python)
             depComp.compile_deps()
             depComp.install_deps()
 
-        return result.stdout
+        return stdout_output
     except subprocess.CalledProcessError as e:
         if raise_on_error:
-            if e.stdout:
-                print(e.stdout)
-            if e.stderr:
-                print(e.stderr, file=sys.stderr)
             raise e
 
         if e.returncode == 1:

tests/comfy_cli/test_cm_cli_python_resolution.py

@@ -1,16 +1,44 @@
+import subprocess
+import sys
+import textwrap
+import time
 from unittest.mock import MagicMock, patch
 
+import pytest
+
 from comfy_cli.command.custom_nodes import cm_cli_util
 
 
+def _setup_cm_cli(tmp_path, script_body):
+    """Create a stub cm-cli.py with the given body and patch workspace to tmp_path."""
+    cm_cli_path = tmp_path / "custom_nodes" / "ComfyUI-Manager" / "cm-cli.py"
+    cm_cli_path.parent.mkdir(parents=True)
+    cm_cli_path.write_text(textwrap.dedent(script_body))
+    (tmp_path / "config").mkdir(exist_ok=True)
+    return tmp_path
+
+
+def _run(tmp_path, args, *, fast_deps=False, raise_on_error=False):
+    """Call execute_cm_cli with standard patches for workspace/config."""
+    with (
+        patch(
+            "comfy_cli.command.custom_nodes.cm_cli_util.resolve_workspace_python",
+            return_value=sys.executable,
+        ),
+        patch.object(cm_cli_util.workspace_manager, "workspace_path", str(tmp_path)),
+        patch.object(cm_cli_util.workspace_manager, "set_recent_workspace"),
+        patch("comfy_cli.command.custom_nodes.cm_cli_util.ConfigManager") as MockConfig,
+    ):
+        MockConfig.return_value.get_config_path.return_value = str(tmp_path / "config")
+        return cm_cli_util.execute_cm_cli(args, fast_deps=fast_deps, raise_on_error=raise_on_error)
+
+
 class TestExecuteCmCli:
     def test_uses_resolved_python(self, tmp_path):
-        cm_cli_path = tmp_path / "custom_nodes" / "ComfyUI-Manager" / "cm-cli.py"
-        cm_cli_path.parent.mkdir(parents=True)
-        cm_cli_path.touch()
-
-        mock_result = MagicMock()
-        mock_result.stdout = "output"
+        _setup_cm_cli(tmp_path, 'print("ok")')
+        mock_proc = MagicMock()
+        mock_proc.stdout = iter(["ok\n"])
+        mock_proc.wait.return_value = 0
 
         with (
             patch(
@@ -20,22 +48,20 @@ def test_uses_resolved_python(self, tmp_path):
             patch.object(cm_cli_util.workspace_manager, "workspace_path", str(tmp_path)),
             patch.object(cm_cli_util.workspace_manager, "set_recent_workspace"),
             patch("comfy_cli.command.custom_nodes.cm_cli_util.ConfigManager") as MockConfig,
-            patch("comfy_cli.command.custom_nodes.cm_cli_util.subprocess.run", return_value=mock_result) as mock_run,
+            patch("comfy_cli.command.custom_nodes.cm_cli_util.subprocess.Popen", return_value=mock_proc) as mock_popen,
         ):
             MockConfig.return_value.get_config_path.return_value = str(tmp_path / "config")
             cm_cli_util.execute_cm_cli(["show", "installed"])
 
         mock_resolve.assert_called_once_with(str(tmp_path))
-        cmd = mock_run.call_args[0][0]
+        cmd = mock_popen.call_args[0][0]
         assert cmd[0] == "/resolved/python"
 
     def test_fast_deps_passes_python_to_compiler(self, tmp_path):
-        cm_cli_path = tmp_path / "custom_nodes" / "ComfyUI-Manager" / "cm-cli.py"
-        cm_cli_path.parent.mkdir(parents=True)
-        cm_cli_path.touch()
-
-        mock_result = MagicMock()
-        mock_result.stdout = "output"
+        _setup_cm_cli(tmp_path, 'print("ok")')
+        mock_proc = MagicMock()
+        mock_proc.stdout = iter(["ok\n"])
+        mock_proc.wait.return_value = 0
 
         with (
             patch(
@@ -45,7 +71,7 @@ def test_fast_deps_passes_python_to_compiler(self, tmp_path):
             patch.object(cm_cli_util.workspace_manager, "workspace_path", str(tmp_path)),
             patch.object(cm_cli_util.workspace_manager, "set_recent_workspace"),
             patch("comfy_cli.command.custom_nodes.cm_cli_util.ConfigManager") as MockConfig,
-            patch("comfy_cli.command.custom_nodes.cm_cli_util.subprocess.run", return_value=mock_result),
+            patch("comfy_cli.command.custom_nodes.cm_cli_util.subprocess.Popen", return_value=mock_proc),
             patch("comfy_cli.command.custom_nodes.cm_cli_util.DependencyCompiler") as MockCompiler,
         ):
             MockConfig.return_value.get_config_path.return_value = str(tmp_path / "config")
@@ -56,3 +82,104 @@ def test_fast_deps_passes_python_to_compiler(self, tmp_path):
 
         MockCompiler.assert_called_once()
         assert MockCompiler.call_args[1]["executable"] == "/resolved/python"
+
+    def test_stdout_returned_and_streamed(self, tmp_path, capsys):
+        _setup_cm_cli(
+            tmp_path,
+            """\
+            print("line 1")
+            print("line 2")
+            print("line 3")
+        """,
+        )
+        result = _run(tmp_path, ["test"])
+
+        assert result == "line 1\nline 2\nline 3\n"
+        captured = capsys.readouterr()
+        assert "line 1\nline 2\nline 3\n" in captured.out
+
+    @pytest.mark.parametrize("returncode", [1, 2])
+    def test_expected_error_codes_return_none(self, tmp_path, returncode):
+        _setup_cm_cli(
+            tmp_path,
+            f"""\
+            import sys
+            sys.exit({returncode})
+        """,
+        )
+        result = _run(tmp_path, ["test"])
+        assert result is None
+
+    def test_unexpected_error_code_raises(self, tmp_path):
+        _setup_cm_cli(
+            tmp_path,
+            """\
+            import sys
+            sys.exit(42)
+        """,
+        )
+        with pytest.raises(subprocess.CalledProcessError) as exc_info:
+            _run(tmp_path, ["test"])
+        assert exc_info.value.returncode == 42
+
+    def test_raise_on_error_overrides_silent_return(self, tmp_path):
+        _setup_cm_cli(
+            tmp_path,
+            """\
+            import sys
+            print("output before fail")
+            sys.exit(1)
+        """,
+        )
+        with pytest.raises(subprocess.CalledProcessError) as exc_info:
+            _run(tmp_path, ["test"], raise_on_error=True)
+        assert exc_info.value.returncode == 1
+        assert "output before fail" in exc_info.value.output
+
+    def test_output_streams_incrementally(self, tmp_path):
+        _setup_cm_cli(
+            tmp_path,
+            """\
+            import time
+            for i in range(3):
+                print(f"line {i}")
+                time.sleep(0.3)
+        """,
+        )
+        timestamps = []
+        original_write = sys.stdout.write
+
+        def recording_write(s):
+            if s.startswith("line "):
+                timestamps.append(time.monotonic())
+            return original_write(s)
+
+        with patch("sys.stdout") as mock_stdout:
+            mock_stdout.write = recording_write
+            mock_stdout.flush = lambda: None
+            _run(tmp_path, ["test"])
+
+        assert len(timestamps) == 3
+        assert timestamps[2] - timestamps[0] >= 0.4
+
+    def test_pythonunbuffered_set_in_env(self, tmp_path):
+        _setup_cm_cli(tmp_path, 'print("ok")')
+        mock_proc = MagicMock()
+        mock_proc.stdout = iter(["ok\n"])
+        mock_proc.wait.return_value = 0
+
+        with (
+            patch(
+                "comfy_cli.command.custom_nodes.cm_cli_util.resolve_workspace_python",
+                return_value=sys.executable,
+            ),
+            patch.object(cm_cli_util.workspace_manager, "workspace_path", str(tmp_path)),
+            patch.object(cm_cli_util.workspace_manager, "set_recent_workspace"),
+            patch("comfy_cli.command.custom_nodes.cm_cli_util.ConfigManager") as MockConfig,
+            patch("comfy_cli.command.custom_nodes.cm_cli_util.subprocess.Popen", return_value=mock_proc) as mock_popen,
+        ):
+            MockConfig.return_value.get_config_path.return_value = str(tmp_path / "config")
+            cm_cli_util.execute_cm_cli(["show", "installed"])
+
+        env = mock_popen.call_args[1]["env"]
+        assert env["PYTHONUNBUFFERED"] == "1"