Skip to content

Automation patterns: wait for tmux instead of guessing - #734

Open
tony wants to merge 2 commits into
masterfrom
docs/lift-automation-patterns
Open

Automation patterns: wait for tmux instead of guessing#734
tony wants to merge 2 commits into
masterfrom
docs/lift-automation-patterns

Conversation

@tony

@tony tony commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

  • Fix a doctest on {ref}automation-patterns that fails deterministically today at --reruns 0, hidden by the repo's --reruns=2 default.
  • Replace fixed sleeps with a tmux channel the command signals when it finishes, blocked on with Server.wait_for() — which returns the moment the command is done rather than after a guess.
  • Replace substring searches of the joined capture with whole-line comparisons, because capture_pane() returns the command tmux echoed onto the pane.
  • No new API. Uses send_keys, capture_pane and Server.wait_for, all of which ship today.

Why the sleeps could not work

Measured on this branch: send_keys to visible in capture_pane() is p50 100.7 ms under pytest -n auto, p90 130 ms. The page used time.sleep(0.1). That is roughly -1 ms of headroom at the median.

The bug this fixes is live

docs/topics/automation_patterns.md on master, four consecutive runs at --reruns 0:

1 failed, 15 passed
1 failed, 15 passed
1 failed, 15 passed
1 failed, 15 passed

Same command on this branch:

16 passed
16 passed
16 passed
16 passed

The failing example is "Capturing output between markers", whose helper searches '\n'.join(pane.capture_pane()) for a marker the command itself contains — so it matches the echoed command line and returns before the command has produced anything. Its TimeoutError branch is unreachable. The page's own assertion then passes off the echo.

The two changes

Signal a channel, block on it. tmux remembers a signal sent before the waiter starts, so there is no lost-wakeup race regardless of ordering. Measured: a pane running sleep 1.5; tmux wait-for -S <ch> released the waiter after 1.506 s, with no polling; signalling first and then waiting returned in 3 ms.

Compare whole lines. capture_pane() already returns list[str]; joining it and searching for a substring is what lets the echo satisfy the check. Demonstrated with enter=False, so no shell ever runs:

capture: ['$ echo READY']
'READY' in '\n'.join(capture)  -> True     <- the echo
'READY' in capture             -> False

Sleeps remain only where an example is about elapsed time itself.

Test plan

  • docs/topics/automation_patterns.md green 4/4 at --reruns 0, against 0/4 on master
  • uv run ruff check . / uv run ruff format . / uv run mypy clean
  • uv run py.test --reruns 0 clean
  • just build-docs clean, no new warnings

Note for the merger

The CHANGES heading cites (#NN) — swap it for this PR's number on merge. It will conflict with the sibling PRs for #723, #724, #725, #726 and #731, which all add to the same block; keep both sides.

Related but not required: Server.wait_for() still has no timeout (#732), so a command that dies before signalling blocks forever. That does not affect the examples here, all of which signal on every exit path, but it is worth closing before this pattern is recommended more widely.

@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.40%. Comparing base (be7c8c1) to head (8d35933).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #734      +/-   ##
==========================================
- Coverage   52.45%   52.40%   -0.06%     
==========================================
  Files          26       26              
  Lines        3729     3729              
  Branches      747      747              
==========================================
- Hits         1956     1954       -2     
- Misses       1469     1472       +3     
+ Partials      304      303       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tony
tony force-pushed the docs/lift-automation-patterns branch 2 times, most recently from a09ab07 to d4c8f7f Compare August 1, 2026 00:33
@tony

tony commented Aug 1, 2026

Copy link
Copy Markdown
Member Author

Code review

Found 1 issue, now fixed in d4c8f7f:

  1. The new deliverable was inserted with its own ### Documentation header instead of being appended under the existing one, so the unreleased entry carried two ### Documentation sections and the from_env/ruff/tmux-options entries ended up under a detached second block. CLAUDE.md says "Fixed subheadings, in this order when present: ### Breaking changes, ### Dependencies, ### What's new, ### Fixes, ### Documentation, ### Development" — one of each.

libtmux/CHANGES

Lines 62 to 67 in a09ab07

that echo and is already true before the shell has run.
### Documentation
#### Cleaner `from_env` examples (#719)

The same hunk shipped the literal placeholder (#NN) in the deliverable heading; that is now (#734).

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@tony
tony force-pushed the docs/lift-automation-patterns branch 2 times, most recently from 80c8797 to 3f81cf8 Compare August 1, 2026 03:15
@tony

tony commented Aug 1, 2026

Copy link
Copy Markdown
Member Author

Code review

Found 1 issue, fixed in 3f81cf8:

  1. The "Detecting errors in output" section claimed the tagged scan tells the two commands apart. It does not — command_output() reads the whole pane, so the check after the second command also sees the first command's out:ok. The example passed by accumulation rather than by discrimination, and a later success would still have reported an earlier failure.

https://github.com/tmux-python/libtmux/blob/80c8797c1b0f8b4e2f6a0c9d3e5a7b1c4d8e9f0a/docs/topics/automation_patterns.md#L166-L170

The prose now says what the tag actually does — it separates the command's output from the shell's echo, not one command's output from the last one's — and the failing command clears the pane first, so command_output() returns ['Error: no disk'] on its own.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@tony
tony force-pushed the docs/lift-automation-patterns branch from 3f81cf8 to 5d304cb Compare August 1, 2026 04:05
@tony

tony commented Aug 1, 2026

Copy link
Copy Markdown
Member Author

Code review

Found 1 issue, fixed in 5d304cb:

  1. Four polling examples used a 2.0 s budget, which does not cover cold shell startup when the suite runs in parallel. CI invokes py.test ... -n auto, so these were being carried by the --reruns=2 in addopts rather than passing outright. Raised to 10.0 s; the page then passes 3/3 under -n auto, against 3 failures before.

https://github.com/tmux-python/libtmux/blob/3f81cf82a9d4e5b6c7f8a0b1c2d3e4f5a6b7c8d9/docs/topics/automation_patterns.md#L337-L339

Worth noting for the record: under a full-suite -n auto run on a many-core machine, this page also fails on master with identical counts, so the residual flakiness there is pre-existing and not introduced here. The budget change is an improvement rather than a regression fix — a timeout is a ceiling, not a cost.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@tony

tony commented Aug 1, 2026

Copy link
Copy Markdown
Member Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@tony
tony force-pushed the docs/lift-automation-patterns branch 2 times, most recently from c5aa46c to 21de528 Compare August 1, 2026 11:44
tony added 2 commits August 1, 2026 07:19
why: The examples slept a fixed amount and then read the pane. Measured,
send_keys to visible in capture_pane is 100ms at the median under a
parallel test run, so the 100ms sleeps had no headroom at all -- the
"Capturing output between markers" example fails deterministically at
--reruns 0 today, and the repo default of --reruns=2 hides it. Several
checks were also searching the joined capture, which matches the command
tmux echoed onto the pane and is true before the shell has run.

what:
- Have commands signal a tmux channel and block on Server.wait_for,
  which returns when the command is actually done
- Compare whole captured lines instead of searching the joined capture
- Keep sleeps only where the example is about elapsed time itself
why: The page's examples were failing at `--reruns 0` before the
rewrite, so a reader copying them got the flake too.

what:
- Record the automation-patterns rewrite under `Documentation`.
@tony
tony force-pushed the docs/lift-automation-patterns branch from 21de528 to 8d35933 Compare August 1, 2026 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant