Skip to content

test(cli): fix order-dependent test_resume_quiet_stderr flake at the source - #68872

Closed
mehmetkr-31 wants to merge 1 commit into
NousResearch:mainfrom
mehmetkr-31:tests-cli-resume-flake-isolation
Closed

test(cli): fix order-dependent test_resume_quiet_stderr flake at the source#68872
mehmetkr-31 wants to merge 1 commit into
NousResearch:mainfrom
mehmetkr-31:tests-cli-resume-flake-isolation

Conversation

@mehmetkr-31

Copy link
Copy Markdown
Contributor

Summary

tests/cli/test_resume_quiet_stderr.py::TestResumeQuietStderr::test_session_not_found_goes_to_stdout_in_full_mode passes in isolation and in its own file but fails in a full tests/cli run — a long-standing order-dependent flake. This fixes the two independent leaks at their source rather than papering over the assertion.

Root cause (two independent leaks, both from test_cli_init.py::_make_cli)

  1. cli left with mock prompt_toolkit globals. _make_cli() reloads cli.py inside patch.dict(sys.modules, prompt_toolkit_stubs) and never reloads it back. patch.dict restores sys.modules on exit, but not the names the reloaded module already bound — so sys.modules["cli"] keeps a MagicMock _pt_print/_PT_ANSI, and cli._cprint silently no-ops for every later test.

  2. prompt_toolkit output cache pinned to a stale capture buffer. print_formatted_text (used by _cprint) caches its Output on the process-global default AppSession the first time it renders with no explicit output=, and never re-reads sys.stdout. Under capsys (which swaps sys.stdout per test), the first CLI test to emit through _cprint — e.g. any TestBusyInputMode test that runs /queue and prints a "Queued: …" line — locks that cache onto its captured stdout. Every later capsys test asserting on _cprint output then reads an empty buffer. Confirmed by tracing the cached output to a PlainTextOutput bound to the polluter's capture fd while the victim's sys.stdout was a different object.

Each leak alone is enough to blank the victim's captured stdout; with leak 1 active the polluter's _cprint is a mock and never populates the PT cache, so fixing 1 exposes 2 — both must be addressed.

Fix (test-only, no production change)

  • tests/cli/test_cli_init.py: wrap _make_cli's stubbed reload in try/finally and reload cli once more with the real modules visible, so sys.modules["cli"] rebinds clean PT globals.
  • tests/cli/conftest.py (new): an autouse fixture that resets the default AppSession's cached _output around each test, so every CLI test re-creates a fresh prompt_toolkit output bound to its own sys.stdout. This fixes the whole class of capsys + prompt_toolkit flakes, not just this one test.

Neither change touches production code or the flaky test's own assertion.

Verification

# the exact failing pair, previously red:
pytest tests/cli/test_cli_init.py::TestBusyInputMode::test_queue_command_works_while_idle \
       tests/cli/test_resume_quiet_stderr.py::TestResumeQuietStderr::test_session_not_found_goes_to_stdout_in_full_mode
# 2 passed

pytest tests/cli   # 1101 passed (previously 1 failed in full runs)

Relation to #59358

#59358 targets the same flaky test and identifies the same prompt_toolkit/capsys interaction, but fixes it by patch("cli._cprint") and asserting on the mock's call args — a test-local assertion workaround that leaves both underlying leaks in place (other capsys tests remain exposed, and the _pt_print mock leak is untouched). This PR instead removes the two leaks at the source and does not modify test_resume_quiet_stderr.py, so the two changes don't conflict; if #59358 lands first, its assertion change simply becomes redundant. Flagging for the maintainer to pick the preferred approach.

🤖 Generated with Claude Code

…source

test_session_not_found_goes_to_stdout_in_full_mode passes in isolation but
fails in a full tests/cli run. Two independent leaks from the same neighbor
test conspire:

1. test_cli_init.py's _make_cli() reloads cli.py while prompt_toolkit is
   stubbed with MagicMocks and never reloads it back, so sys.modules['cli']
   is left with a mock _pt_print/_PT_ANSI and cli._cprint silently no-ops
   for every later test. Fixed by reloading cli once more with the real
   modules visible (try/finally).

2. prompt_toolkit's print_formatted_text caches its Output on the
   process-global default AppSession the first time it renders without an
   explicit output=. Under capsys (which swaps sys.stdout per test), the
   first CLI test to emit through _cprint locks that cache onto its own
   captured stdout, so later capsys tests read an empty buffer. Fixed with
   an autouse fixture in a new tests/cli/conftest.py that resets the cached
   output around each test.

Neither change touches production code or the flaky test's own assertion.

Related to NousResearch#59358 (which addresses the same flaky test by mocking _cprint in
the assertion instead; this fixes the two underlying leaks at the source and
does not modify test_resume_quiet_stderr.py).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@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 Jul 21, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #74553 (commit c15aab9, your authorship preserved, contributor mapping added via contributors/emails/). Excellent root-cause work — both legs verified (stale mock globals after cli reload + prompt_toolkit AppSession output cache pinned to a dead capsys buffer), reproduced both orders pre-fix, 723/723 tests/cli green post-fix. The release.py AUTHOR_MAP hunk was rerouted to contributors/emails/ (the dict is frozen). Thanks @mehmetkr-31!

@teknium1 teknium1 closed this Jul 30, 2026
teknium1 added a commit that referenced this pull request Jul 30, 2026
…ontributors/emails/

The #68873 salvage re-added a line to the frozen dict; the canonical
mapping (contributors/emails/mehmet.kar@std.yildiz.edu.tr) already
exists from the #68872 salvage.
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…ontributors/emails/

The NousResearch#68873 salvage re-added a line to the frozen dict; the canonical
mapping (contributors/emails/mehmet.kar@std.yildiz.edu.tr) already
exists from the NousResearch#68872 salvage.
33hodl pushed a commit to 33hodl/hermes-agent that referenced this pull request Aug 12, 2026
…ontributors/emails/

The NousResearch#68873 salvage re-added a line to the frozen dict; the canonical
mapping (contributors/emails/mehmet.kar@std.yildiz.edu.tr) already
exists from the NousResearch#68872 salvage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

3 participants