Skip to content

retry_until forwards args, so a wait in a loop needs no closure - #729

Open
tony wants to merge 1 commit into
masterfrom
fix/726-retry-until-args
Open

retry_until forwards args, so a wait in a loop needs no closure#729
tony wants to merge 1 commit into
masterfrom
fix/726-retry-until-args

Conversation

@tony

@tony tony commented Jul 29, 2026

Copy link
Copy Markdown
Member

Summary

  • Add a keyword-only args tuple to retry_until(), unpacked into every call of the predicate, so the object being waited on is passed rather than closed over.
  • Add two typing overloads: the zero-argument predicate keeps strict checking when args is absent, and args is required on the forwarding overload so it cannot be selected by accident.
  • Document the parameter, with a doctest that waits on each pane of a loop.

The problem

A zero-argument predicate has nowhere to put the subject, so a wait written inside a loop has to close over the loop variable. This repo enables flake8-bugbear, which rejects that:

B023 Function definition does not bind loop variable `pane`
    retry_until(lambda: any(pane.capture_pane()))
                             ^^^^

B023's documented fix is a default-argument binding — which mypy then refuses to infer:

error: Cannot infer type of lambda  [misc]

So the lint-clean form does not type-check and the type-clean form does not lint, leaving # type: ignore[misc] as the only way out at every such call site.

No existing call site on master suffers from this — they all use named nested functions. This is a preventative ergonomic change, not a repair of live breakage.

Before / After

for pane in panes:
    retry_until(lambda pane=pane: any(pane.capture_pane()))  # type: ignore[misc]
for pane in panes:
    retry_until(lambda p: any(p.capture_pane()), args=(pane,))

Design decisions

Overloads rather than one widened signature. Widening fun to Callable[..., bool] outright would have stopped mypy catching a predicate that wrongly takes arguments in the common zero-argument case. Two overloads keep that: the first takes Callable[[], bool] and has no args parameter at all, the second takes Callable[..., bool] with args required — no default — so the strict overload is selected whenever args is absent.

args is keyword-only. seconds is already positional-second, so a positional args would have been ambiguous at the call site and a source of silent misreads.

Not shipping ready-made waits. The issue also proposes exposing the two common cases ("this pane produced anything", "this pane printed this whole line") as methods. That is a larger, separate change with its own API surface, and would deserve its own issue; this PR only removes the reason a caller has to write a closure.

A trap the examples deliberately avoid

capture_pane() returns the command tmux echoed onto the pane, so a substring test for the expected output matches that echo and is already true before the shell has run anything. Both the new doctest and the new test therefore compare whole lines, and say why in a comment — otherwise they would pass in a world where no shell ever ran, which is exactly the failure mode being tracked in #715.

Test plan

  • test_retry_until_forwards_args_from_loop — waits on each pane of a loop through the new path, with no # type: ignore and no # noqa in the file
  • test_retry_until_args_reach_predicate — the tuple arrives at the predicate on every attempt
  • uv run ruff check . clean, which is the assertion that B023 no longer fires
  • uv run mypy clean, which is the assertion that the overloads resolve
  • Doctests on retry_until collected and run
  • uv run py.test --reruns 0 clean
  • just build-docs clean

Note for the merger

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

why: A zero-argument predicate has nowhere to put the object being
waited on, so a wait written inside a loop must close over the loop
variable. Ruff's B023 rejects that closure, and B023's documented fix
(a default-argument binding) makes mypy report "Cannot infer type of
lambda", leaving `# type: ignore[misc]` as the only way out. Refs #726.

what:
- Add a keyword-only `args` tuple to retry_until(), unpacked into every
  call of the predicate
- Add two overloads: the zero-argument predicate keeps strict checking
  when `args` is absent, and `args` is required on the forwarding
  overload so it cannot be selected by accident
- Document `args` and add a doctest that waits on each pane of a loop
- Cover the loop case and argument forwarding in tests/test/test_retry.py
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.31%. Comparing base (be7c8c1) to head (e3e88a1).

Files with missing lines Patch % Lines
src/libtmux/test/retry.py 20.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #729      +/-   ##
==========================================
- Coverage   52.45%   52.31%   -0.14%     
==========================================
  Files          26       26              
  Lines        3729     3733       +4     
  Branches      747      749       +2     
==========================================
- Hits         1956     1953       -3     
- Misses       1469     1476       +7     
  Partials      304      304              

☔ 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.

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