Skip to content

test(update-autostash): include stdout in fake_run defaults so stale-dashboard scan doesn't crash#17149

Closed
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/test-update-autostash-stale-dashboard-fixture
Closed

test(update-autostash): include stdout in fake_run defaults so stale-dashboard scan doesn't crash#17149
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/test-update-autostash-stale-dashboard-fixture

Conversation

@briandevans

@briandevans briandevans commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Lifts a CI baseline introduced by 66b1142 — two cmd_update integration tests in test_update_autostash.py have been crashing on origin/main since the _warn_stale_dashboard_processes() call landed in _cmd_update_impl.

The bug

Commit 66b1142 ("fix(cli): warn about stale dashboard processes after hermes update") added a new

subprocess.run([\"ps\", \"-A\", \"-o\", \"pid=,command=\"], capture_output=True, text=True, timeout=10)

call inside _cmd_update_impl and reads result.stdout.split(\"\\n\"). The two cmd_update integration tests in tests/hermes_cli/test_update_autostash.py mock subprocess.run with a catch-all SimpleNamespace(returncode=0) that omits stdout. After 66b1142, both tests crash inside _warn_stale_dashboard_processes with:

hermes_cli/main.py:5309: AttributeError
'types.SimpleNamespace' object has no attribute 'stdout'

Affected tests:

  • tests/hermes_cli/test_update_autostash.py::test_cmd_update_succeeds_with_extras
  • tests/hermes_cli/test_update_autostash.py::test_cmd_update_retries_optional_extras_individually_when_all_fails

Reproduces deterministically on a clean clone of origin/main (verified at a830f25f7).

The fix

Add stdout=\"\", stderr=\"\" to the catch-all SimpleNamespace(returncode=0) returns in both tests' fake_run so the new ps-scan finds zero dashboard processes and short-circuits cleanly. The two explicit pip-install branches that returned bare SimpleNamespace(returncode=0) are extended the same way for symmetry — no production behavior changes, only test fixture hygiene.

This is intentionally test-only. The production path is already covered by the _warn_stale_dashboard_processes regression tests added in #17123 (and the wmic-encoding fix in #17074).

Test plan

  • uv run --with pytest --with pytest-xdist python3 -m pytest tests/hermes_cli/test_update_autostash.py -v — 23 passed, was 21 passed + 2 failed on origin/main.
  • uv run --with pytest --with pytest-xdist python3 -m pytest tests/hermes_cli/test_update_stale_dashboard.py -v — 10 passed (no regression).
  • Regression guard: removed the stdout=\"\" additions on a clean origin/main clone, reproduced the original AttributeError: 'types.SimpleNamespace' object has no attribute 'stdout' failure with identical traceback at hermes_cli/main.py:5309. Restored the fix, both tests pass.

Related

@briandevans @claude
test(update-autostash): include stdout in fake_run defaults so stale-…
a7884da 
…dashboard scan doesn't crash

`_cmd_update_impl` gained a new `subprocess.run(["ps", "-A", ...])` call
in commit 66b1142 ("fix(cli): warn about stale dashboard processes
after hermes update") that reads `result.stdout.split("\n")`. The two
`cmd_update` integration tests in `test_update_autostash.py` mock
`subprocess.run` with a catch-all `SimpleNamespace(returncode=0)` that
omits `stdout`, so those tests now crash inside the new
`_warn_stale_dashboard_processes()` with:

    AttributeError: 'types.SimpleNamespace' object has no attribute 'stdout'

This has been a baseline `Tests` job failure on `origin/main` since
66b1142 landed — see for example the CI run on
NousResearch#17123.

Add `stdout=""`, `stderr=""` to the catch-all `SimpleNamespace` returns
in `test_cmd_update_retries_optional_extras_individually_when_all_fails`
and `test_cmd_update_succeeds_with_extras` so the new ps-scan call
short-circuits as a no-match instead of raising. The other
explicitly-matched returns in the same fixture are also extended for
symmetry. No production code changes; the new behavior tests added in
PR NousResearch#17074 / NousResearch#17123 already cover the production path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 28, 2026 21:10
Copilot AI reviewed Apr 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes two cmd_update integration tests that started crashing after _warn_stale_dashboard_processes() began reading subprocess.run(...).stdout during hermes update.

Changes:

  • Update the fake_run subprocess mock in test_update_autostash.py to return stdout/stderr on default and pip-install branches.
  • Prevent AttributeError when _warn_stale_dashboard_processes() runs during these tests by ensuring mocked results match CompletedProcess shape more closely.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels Apr 28, 2026
@briandevans

briandevans commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

CI audit — all 40 test job failures on this branch are pre-existing baselines on clean origin/main (87d3fa6f1). Zero failures intersect with touched code (tests/hermes_cli/test_update_autostash.py).

Reproduction: cloned origin/main at 87d3fa6f1 into a worktree and ran the failing tests directly — the failures reproduce without this PR's diff applied. Sample:

Test Symptom Root cause on main
tests/agent/test_anthropic_adapter.py::test_custom_base_url beta header set mismatch (computer-use-2025-08-07 vs computer-use-2025-05-14) Already-stale assertion vs current production beta string
tests/tools/test_web_tools_config.py::TestFirecrawlClientConfig::* (8 tests) AttributeError: module tools.web_tools has no attribute 'Firecrawl' Test references symbol removed/renamed in tools/web_tools.py on main
tests/hermes_cli/test_gemini_provider.py::test_gemini_resolve_provider_client_keeps_openai_for_non_native_base_url isinstance() arg 2 must be a type, a tuple of types, or a union Pre-existing on main
tests/tools/test_clipboard.py::TestIsWsl::* (3 tests) Linux WSL detection assertions Pass locally on macOS; fail on Linux CI on main
tests/hermes_cli/test_web_server.py::TestReloadEnv::* and TestBuildSchemaFromConfig::test_no_single_field_categories env reload + schema category drift Pre-existing on main
tests/gateway/test_gateway_shutdown.py::test_cancel_background_tasks_cancels_inflight_message_processing and tests/gateway/test_session_split_brain_11016.py::* TimeoutError 30s Pre-existing flake/hang on main
tests/tools/test_mcp_structured_content.py::* and test_mcp_dynamic_discovery.py::* KeyError: 'result' / _rpc_lock MCP harness drift on main
tests/tui_gateway/test_protocol.py::test_session_resume_returns_hydrated_messages unexpected keyword argument 'include_ancestors' Already addressed by sibling PR #16683

The nix-lockfile-check failure is a workflow infrastructure error (fix-lockfiles exited without reporting stale status), not related to the diff. nix (ubuntu-latest) failed transiently; nix (macos-latest) passed.

Touched test (tests/hermes_cli/test_update_autostash.py) — all 23 cases pass locally under pytest-xdist.

This was referenced Apr 29, 2026
Closed
Closed
@briandevans

briandevans commented May 13, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #17642 ("fix(ci): recover 38 failing tests on main") which already updated test_update_autostash to handle the stale-dashboard scan. Closing — happy to reopen if anything's still missing here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have type/test Test coverage or test infrastructure

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

hermes update doesn't warn about stale dashboard processes (frontend/backend version mismatch)

3 participants

@briandevans @alt-glitch