From 835f9adec04c44d50a44f0fa6a34e5dee81ac4ee Mon Sep 17 00:00:00 2001 From: briandevans <252620095+briandevans@users.noreply.github.com> Date: Tue, 28 Apr 2026 13:10:26 -0700 Subject: [PATCH] fix(update,test): clarify wmic comment; switch tests to monkeypatch sys.platform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fix-ups for #17123: 1. Reword the inline comment in `_warn_stale_dashboard_processes` to accurately describe the failure mode (locale-dependent decoder, not a "default UTF-8 decoder") and identify `errors="ignore"` as the load-bearing protection. Per Copilot's review. 2. Switch `TestWindowsWmicEncoding` from `patch("hermes_cli.main.sys")` to `monkeypatch.setattr(sys, "platform", "win32")` — the codebase's canonical pattern (e.g. `tests/hermes_cli/test_auth_ssl_macos.py`). The MagicMock-replacement approach passed locally on Python 3.12 but the platform-equality check failed under CI's xdist+Python 3.11, leaving both new tests red despite the fix being present. Co-Authored-By: Claude Opus 4.7 (1M context) --- hermes_cli/main.py | 13 +++++++------ tests/hermes_cli/test_update_stale_dashboard.py | 14 ++++++-------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index c56fc1e3..70a30ab5 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -5274,12 +5274,13 @@ def _warn_stale_dashboard_processes() -> None: try: if sys.platform == "win32": - # wmic emits text in the system code page (e.g. cp936 on zh-CN - # locales), not UTF-8. Without an explicit encoding, Python's - # default UTF-8 decoder crashes the subprocess reader thread - # with UnicodeDecodeError, leaving result.stdout=None and the - # subsequent .split() call crashing `hermes update` with an - # AttributeError on Windows non-UTF-8 locales (#17049). + # wmic may emit text in the system code page (for example cp936 + # on zh-CN systems), not UTF-8. In text mode, subprocess output + # decoding depends on Python's configuration (locale-dependent + # by default, or UTF-8 in UTF-8 mode). The important protection + # here is errors="ignore": it prevents a reader-thread + # UnicodeDecodeError from leaving result.stdout=None and turning + # the later .split() into an AttributeError (#17049). result = subprocess.run( ["wmic", "process", "get", "ProcessId,CommandLine", "/FORMAT:LIST"], diff --git a/tests/hermes_cli/test_update_stale_dashboard.py b/tests/hermes_cli/test_update_stale_dashboard.py index b36f9e5a..20f23e58 100644 --- a/tests/hermes_cli/test_update_stale_dashboard.py +++ b/tests/hermes_cli/test_update_stale_dashboard.py @@ -182,13 +182,12 @@ class TestWindowsWmicEncoding: `hermes update` on non-UTF-8 system locales (e.g. cp936 on zh-CN). """ - def test_wmic_invoked_with_utf8_ignore_errors(self): + def test_wmic_invoked_with_utf8_ignore_errors(self, monkeypatch): """The wmic subprocess.run call must pass encoding='utf-8' and errors='ignore' so the subprocess reader thread cannot raise UnicodeDecodeError on non-UTF-8 wmic output.""" - with patch("hermes_cli.main.sys") as mock_sys, \ - patch("subprocess.run") as mock_run: - mock_sys.platform = "win32" + monkeypatch.setattr(sys, "platform", "win32") + with patch("subprocess.run") as mock_run: # Provide a minimal valid wmic /FORMAT:LIST response. mock_run.return_value = MagicMock( returncode=0, @@ -212,15 +211,14 @@ class TestWindowsWmicEncoding: "down the reader thread (#17049)." ) - def test_wmic_returns_none_stdout_does_not_crash(self, capsys): + def test_wmic_returns_none_stdout_does_not_crash(self, monkeypatch, capsys): """If subprocess.run returns successfully but stdout is None — which is what Python 3.11 leaves behind when the reader thread silently crashed on UnicodeDecodeError before this fix landed — the warning must short-circuit instead of raising AttributeError on ``None.split('\\n')`` and aborting `hermes update` (#17049).""" - with patch("hermes_cli.main.sys") as mock_sys, \ - patch("subprocess.run") as mock_run: - mock_sys.platform = "win32" + monkeypatch.setattr(sys, "platform", "win32") + with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock( returncode=0, stdout=None, stderr="" )