From e5c591c0a7ea9a75f5ac30bd958a67df9200fbf4 Mon Sep 17 00:00:00 2001 From: Alishahryar1 Date: Mon, 29 Jun 2026 20:43:12 -0700 Subject: [PATCH] Fix smoke subprocess text decoding --- smoke/lib/child_process.py | 25 ++++++++ smoke/lib/claude_cli_matrix.py | 13 ++-- smoke/lib/e2e.py | 5 +- smoke/prereq/test_cli_prereq_live.py | 15 +++-- .../product/test_cli_package_product_live.py | 7 +-- .../test_config_extensibility_product_live.py | 23 +++---- tests/contracts/test_nvidia_nim_cli_matrix.py | 57 ++++++++++++++++++ tests/contracts/test_smoke_child_process.py | 60 +++++++++++++++++++ 8 files changed, 169 insertions(+), 36 deletions(-) create mode 100644 tests/contracts/test_smoke_child_process.py diff --git a/smoke/lib/child_process.py b/smoke/lib/child_process.py index a61f7057..9fcd26d3 100644 --- a/smoke/lib/child_process.py +++ b/smoke/lib/child_process.py @@ -8,7 +8,10 @@ should use the same interpreter. from __future__ import annotations +import subprocess import sys +from collections.abc import Mapping, Sequence +from pathlib import Path def python_exe() -> str: @@ -42,3 +45,25 @@ def cmd_fcc_init() -> list[str]: def cmd_free_claude_code_serve() -> list[str]: return [python_exe(), "-c", "from cli.entrypoints import serve; serve()"] + + +def run_captured_text( + command: Sequence[str], + *, + cwd: str | Path | None = None, + env: Mapping[str, str] | None = None, + timeout: float | None = None, + check: bool = False, +) -> subprocess.CompletedProcess[str]: + """Run a smoke child process with deterministic captured text decoding.""" + return subprocess.run( + list(command), + cwd=cwd, + env=env, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=timeout, + check=check, + ) diff --git a/smoke/lib/claude_cli_matrix.py b/smoke/lib/claude_cli_matrix.py index 39352436..6b58df10 100644 --- a/smoke/lib/claude_cli_matrix.py +++ b/smoke/lib/claude_cli_matrix.py @@ -12,6 +12,7 @@ from dataclasses import asdict, dataclass from pathlib import Path from typing import Any +from smoke.lib.child_process import run_captured_text from smoke.lib.config import ProviderModel, SmokeConfig, redacted from smoke.lib.server import RunningServer @@ -132,12 +133,10 @@ def run_claude_cli( started = time.monotonic() try: - result = subprocess.run( + result = run_captured_text( cmd, cwd=cwd, env=env, - capture_output=True, - text=True, timeout=config.timeout_s, check=False, ) @@ -154,8 +153,8 @@ def run_claude_cli( return ClaudeCliRun( command=tuple(cmd), returncode=result.returncode, - stdout=result.stdout, - stderr=result.stderr, + stdout=_coerce_timeout_text(result.stdout), + stderr=_coerce_timeout_text(result.stderr), duration_s=time.monotonic() - started, ) @@ -773,7 +772,9 @@ def _marker(scope: str, prefix: str) -> str: return f"FCC_{scope}_{prefix}_{uuid.uuid4().hex[:8].upper()}" -def _excerpt(value: str, *, max_chars: int = 2400) -> str: +def _excerpt(value: str | None, *, max_chars: int = 2400) -> str: + if value is None: + value = "" if len(value) <= max_chars: return redacted(value) return redacted(value[-max_chars:]) diff --git a/smoke/lib/e2e.py b/smoke/lib/e2e.py index d12bde4a..b937e359 100644 --- a/smoke/lib/e2e.py +++ b/smoke/lib/e2e.py @@ -30,6 +30,7 @@ from core.anthropic.stream_contracts import ( from messaging.models import IncomingMessage from messaging.session import SessionStore from messaging.workflow import MessagingWorkflow +from smoke.lib.child_process import run_captured_text from smoke.lib.config import ProviderModel, SmokeConfig, auth_headers from smoke.lib.server import RunningServer, start_server from smoke.lib.skips import fail_missing_env @@ -285,7 +286,7 @@ class ClientProtocolDriver: env["ANTHROPIC_AUTH_TOKEN"] = config.settings.anthropic_auth_token else: env.pop("ANTHROPIC_AUTH_TOKEN", None) - return subprocess.run( + return run_captured_text( [ claude_bin, "--bare", @@ -298,8 +299,6 @@ class ClientProtocolDriver: ], cwd=cwd, env=env, - capture_output=True, - text=True, timeout=config.timeout_s, check=False, ) diff --git a/smoke/prereq/test_cli_prereq_live.py b/smoke/prereq/test_cli_prereq_live.py index f20079c6..751c88f9 100644 --- a/smoke/prereq/test_cli_prereq_live.py +++ b/smoke/prereq/test_cli_prereq_live.py @@ -2,12 +2,15 @@ from __future__ import annotations import os import shutil -import subprocess from pathlib import Path import pytest -from smoke.lib.child_process import cmd_fcc_init, cmd_free_claude_code_serve +from smoke.lib.child_process import ( + cmd_fcc_init, + cmd_free_claude_code_serve, + run_captured_text, +) from smoke.lib.config import SmokeConfig from smoke.lib.server import start_server from smoke.lib.skips import skip_upstream_unavailable @@ -21,12 +24,10 @@ def test_fcc_init_scaffolds_user_config( env = os.environ.copy() env["HOME"] = str(tmp_path) env["USERPROFILE"] = str(tmp_path) - result = subprocess.run( + result = run_captured_text( cmd_fcc_init(), cwd=smoke_config.root, env=env, - capture_output=True, - text=True, timeout=smoke_config.timeout_s, check=False, ) @@ -63,12 +64,10 @@ def test_claude_cli_prompt_when_available( env["ANTHROPIC_BASE_URL"] = server.base_url if smoke_config.settings.anthropic_auth_token: env["ANTHROPIC_AUTH_TOKEN"] = smoke_config.settings.anthropic_auth_token - result = subprocess.run( + result = run_captured_text( [claude_bin, "-p", "Reply with exactly FCC_SMOKE_PONG"], cwd=tmp_path, env=env, - capture_output=True, - text=True, timeout=smoke_config.timeout_s, check=False, ) diff --git a/smoke/product/test_cli_package_product_live.py b/smoke/product/test_cli_package_product_live.py index 8a6cd238..4ebc996f 100644 --- a/smoke/product/test_cli_package_product_live.py +++ b/smoke/product/test_cli_package_product_live.py @@ -2,7 +2,6 @@ from __future__ import annotations import asyncio import os -import subprocess from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch @@ -10,7 +9,7 @@ import pytest from cli.managed.manager import ManagedClaudeSessionManager from cli.managed.session import ManagedClaudeSession -from smoke.lib.child_process import cmd_fcc_init +from smoke.lib.child_process import cmd_fcc_init, run_captured_text from smoke.lib.config import SmokeConfig pytestmark = [pytest.mark.live, pytest.mark.smoke_target("cli")] @@ -20,12 +19,10 @@ def test_entrypoint_init_e2e(smoke_config: SmokeConfig, tmp_path: Path) -> None: env = os.environ.copy() env["HOME"] = str(tmp_path) env["USERPROFILE"] = str(tmp_path) - result = subprocess.run( + result = run_captured_text( cmd_fcc_init(), cwd=smoke_config.root, env=env, - capture_output=True, - text=True, timeout=smoke_config.timeout_s, check=False, ) diff --git a/smoke/product/test_config_extensibility_product_live.py b/smoke/product/test_config_extensibility_product_live.py index 046c478f..4178eefd 100644 --- a/smoke/product/test_config_extensibility_product_live.py +++ b/smoke/product/test_config_extensibility_product_live.py @@ -1,7 +1,6 @@ from __future__ import annotations import os -import subprocess import pytest @@ -9,7 +8,11 @@ from config.provider_catalog import PROVIDER_CATALOG from config.settings import Settings from messaging.platforms.factory import create_messaging_components from providers.runtime import build_provider_config -from smoke.lib.child_process import cmd_free_claude_code_serve, cmd_python_c +from smoke.lib.child_process import ( + cmd_free_claude_code_serve, + cmd_python_c, + run_captured_text, +) from smoke.lib.config import SmokeConfig from smoke.lib.e2e import SmokeServerDriver @@ -32,12 +35,10 @@ def test_env_precedence_e2e(smoke_config: SmokeConfig, tmp_path) -> None: "s=get_settings(); " "print(s.model); print(s.anthropic_auth_token)" ) - result = subprocess.run( + result = run_captured_text( cmd_python_c(script), cwd=smoke_config.root, env=env, - capture_output=True, - text=True, timeout=smoke_config.timeout_s, check=False, ) @@ -52,12 +53,10 @@ def test_removed_env_migration_e2e(smoke_config: SmokeConfig, tmp_path) -> None: env_file.write_text('NIM_ENABLE_THINKING="true"\n', encoding="utf-8") env = os.environ.copy() env["FCC_ENV_FILE"] = str(env_file) - result = subprocess.run( + result = run_captured_text( cmd_python_c("from config.settings import Settings; Settings()"), cwd=smoke_config.root, env=env, - capture_output=True, - text=True, timeout=smoke_config.timeout_s, check=False, ) @@ -86,12 +85,10 @@ def test_per_model_thinking_config_e2e(smoke_config: SmokeConfig, tmp_path) -> N "print(r.resolve('claude-haiku-4-20250514').thinking_enabled); " "print(r.resolve('unknown-model').thinking_enabled)" ) - result = subprocess.run( + result = run_captured_text( cmd_python_c(script), cwd=smoke_config.root, env=env, - capture_output=True, - text=True, timeout=smoke_config.timeout_s, check=False, ) @@ -121,12 +118,10 @@ def test_proxy_timeout_config_e2e(smoke_config: SmokeConfig, tmp_path) -> None: "print(c.proxy); print(c.http_read_timeout); " "print(c.http_connect_timeout); print(c.http_write_timeout)" ) - result = subprocess.run( + result = run_captured_text( cmd_python_c(script), cwd=smoke_config.root, env=env, - capture_output=True, - text=True, timeout=smoke_config.timeout_s, check=False, ) diff --git a/tests/contracts/test_nvidia_nim_cli_matrix.py b/tests/contracts/test_nvidia_nim_cli_matrix.py index ae45b4d6..dcf052ed 100644 --- a/tests/contracts/test_nvidia_nim_cli_matrix.py +++ b/tests/contracts/test_nvidia_nim_cli_matrix.py @@ -1,7 +1,9 @@ from __future__ import annotations import json +import subprocess from pathlib import Path +from typing import cast from config.settings import Settings from smoke.lib.claude_cli_matrix import ( @@ -10,9 +12,11 @@ from smoke.lib.claude_cli_matrix import ( _subagent_probe_options, make_outcome, regression_failures, + run_claude_cli, write_matrix_report, ) from smoke.lib.config import DEFAULT_TARGETS, SmokeConfig +from smoke.lib.server import RunningServer def _smoke_config(tmp_path: Path) -> SmokeConfig: @@ -75,6 +79,59 @@ def test_nvidia_nim_cli_matrix_report_shape_and_redaction( assert "secret-nim-key" not in path.read_text(encoding="utf-8") +def test_cli_matrix_normalizes_missing_captured_output( + tmp_path: Path, monkeypatch +) -> None: + def fake_run_captured_text( + command: list[str], + **_kwargs: object, + ) -> subprocess.CompletedProcess[str]: + return cast( + subprocess.CompletedProcess[str], + subprocess.CompletedProcess( + args=command, + returncode=0, + stdout=None, + stderr=None, + ), + ) + + monkeypatch.setattr( + "smoke.lib.claude_cli_matrix.run_captured_text", + fake_run_captured_text, + ) + server = RunningServer( + base_url="http://127.0.0.1:9999", + port=9999, + log_path=tmp_path / "server.log", + process=cast(subprocess.Popen[bytes], object()), + ) + + run = run_claude_cli( + claude_bin="claude", + server=server, + config=_smoke_config(tmp_path), + cwd=tmp_path / "workspace", + prompt="hello", + tools="", + ) + outcome = make_outcome( + model="z-ai/glm-5.1", + full_model="nvidia_nim/z-ai/glm-5.1", + source="nvidia_nim_cli_default", + feature="basic_text", + marker="FCC_NIM_BASIC", + run=run, + log_delta='POST /v1/messages HTTP/1.1" 200 OK', + log_path=tmp_path / "server.log", + ) + + assert run.stdout == "" + assert run.stderr == "" + assert outcome.stdout_excerpt == "" + assert outcome.stderr_excerpt == "" + + def test_openrouter_free_cli_matrix_report_shape_and_redaction( tmp_path: Path, monkeypatch ) -> None: diff --git a/tests/contracts/test_smoke_child_process.py b/tests/contracts/test_smoke_child_process.py new file mode 100644 index 00000000..26a4a06e --- /dev/null +++ b/tests/contracts/test_smoke_child_process.py @@ -0,0 +1,60 @@ +from __future__ import annotations + +import subprocess +from pathlib import Path + +from smoke.lib import child_process +from smoke.lib.child_process import cmd_python_c, run_captured_text + + +def test_run_captured_text_uses_utf8_replacement(monkeypatch, tmp_path: Path) -> None: + calls: dict[str, object] = {} + + def fake_run( + command: list[str], + **kwargs: object, + ) -> subprocess.CompletedProcess[str]: + calls["command"] = command + calls.update(kwargs) + return subprocess.CompletedProcess( + args=command, + returncode=0, + stdout="ok", + stderr="", + ) + + monkeypatch.setattr(child_process.subprocess, "run", fake_run) + + result = run_captured_text( + ("cmd", "arg"), + cwd=tmp_path, + env={"FCC_TEST": "1"}, + timeout=1.0, + ) + + assert result.stdout == "ok" + assert calls["command"] == ["cmd", "arg"] + assert calls["cwd"] == tmp_path + assert calls["env"] == {"FCC_TEST": "1"} + assert calls["capture_output"] is True + assert calls["text"] is True + assert calls["encoding"] == "utf-8" + assert calls["errors"] == "replace" + assert calls["timeout"] == 1.0 + assert calls["check"] is False + + +def test_run_captured_text_replaces_invalid_utf8_bytes(tmp_path: Path) -> None: + result = run_captured_text( + cmd_python_c( + "import sys; " + "sys.stdout.buffer.write(bytes([0x8f])); " + "sys.stderr.buffer.write(bytes([0x8f]))" + ), + cwd=tmp_path, + timeout=10.0, + ) + + assert result.returncode == 0 + assert result.stdout == "\ufffd" + assert result.stderr == "\ufffd"