Skip to content

test: convert pty_session wall-clock waits to deterministic predicate witnesses - #116117

Open
Kyzcreig wants to merge 1 commit into
NousResearch:mainfrom
ANG-Ventures:tests-deflake-pty-session
Open

Kyzcreig wants to merge 1 commit into
NousResearch:mainfrom
ANG-Ventures:tests-deflake-pty-session

Conversation

@Kyzcreig

Copy link
Copy Markdown

Defect

tests/hermes_cli/test_pty_session.py::test_reaper_loop_invokes_reap is a wall-clock flake. It starts the reaper at interval=0.01, sleeps 0.05, then asserts the reap counter >= 2:

task = asyncio.create_task(run_reaper(reg, interval=0.01))
await asyncio.sleep(0.05)
task.cancel()
...
assert calls["n"] >= 2

On a loaded CI runner that stopwatch measures the machine, not the reaper. We hit assert 1 >= 2 on a PR that touches no pty code at all (agent/credential_pool.py only) — the test reds out other people's work.

Conversion

Tests only — hermes_cli/pty_session.py is untouched; no production seam was needed.

  • test_reaper_loop_invokes_reap now awaits an asyncio.Event the fake reap_idle sets on its second call, under a 10s wait_for hang guard. The witness is the iteration count reached, so load costs time, never correctness.
  • The four sibling drain tests in the same file replace fixed asyncio.sleep(0.05) budgets with a wait_until(predicate) helper that polls the exact fact each test asserts (buffer contents / socket close code). Converting the whole family in one pass keeps the file from taxing unrelated PRs later.

The one remaining sleep(0.05) in the file is deliberately left alone: it precedes an assertion that something did not happen (s.attached is True, last_detached_at is None), so load only makes it more permissive.

RED proofs

Each mutation's red names the new witness, not an incidental error. Production restored and re-verified green after each.

mutation red
run_reaper while True → for _ in range(1) AssertionError: reaper loop only invoked reap_idle 1 time(s)
_drain self.buffer.append(chunk) → pass 3 failures, all timed out after 10.0s waiting for the drain task to buffer ...

Verification

$ pytest tests/hermes_cli/test_pty_session.py -q
16 passed, 2 warnings in 1.25s

Also verified under load on the equivalent file in our downstream tree: 20/20 green for the reaper test and 20/20 for the whole file with 64 CPU burners on 32 cores (load average 14 → 75).

tests/hermes_cli/test_pty_session.py::test_reaper_loop_invokes_reap starts the
reaper at interval=0.01, sleeps 0.05, then asserts the reap counter >= 2. On a
loaded CI runner that stopwatch measures the machine rather than the reaper: we
observed "assert 1 >= 2" on a PR that touches no pty code at all.

Tests only — hermes_cli/pty_session.py is untouched; no production seam needed.

- test_reaper_loop_invokes_reap now awaits an asyncio.Event the fake reap_idle
  sets on its second call, under a 10s wait_for hang guard. The witness is the
  iteration COUNT reached, so load costs time, never correctness.
- The four sibling drain tests in the same file replace fixed
  asyncio.sleep(0.05) budgets with a wait_until(predicate) helper that polls
  the exact fact each test asserts (buffer contents / socket close code).
  Converting the whole family in one pass keeps the file from taxing unrelated
  PRs later.

The one remaining sleep(0.05) in the file is deliberately left alone: it
precedes an assertion that something did NOT happen, so load only makes it more
permissive.

RED proofs, each red naming the new witness rather than an incidental error:
- run_reaper "while True" -> "for _ in range(1)":
  AssertionError: reaper loop only invoked reap_idle 1 time(s)
- _drain "self.buffer.append(chunk)" -> pass: 3 failures, all
  "timed out after 10.0s waiting for the drain task to buffer ..."

GREEN before and after each mutation: 16 passed.
@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 tool/terminal Terminal execution and process management labels Sep 19, 2026

This branch has not been deployed

No deployments
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 tool/terminal Terminal execution and process management type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants