retry_until forwards args, so a wait in a loop needs no closure - #729
Open
tony wants to merge 1 commit into
Open
retry_until forwards args, so a wait in a loop needs no closure#729tony wants to merge 1 commit into
tony wants to merge 1 commit into
Conversation
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 Report❌ Patch coverage is
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. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
argstuple toretry_until(), unpacked into every call of the predicate, so the object being waited on is passed rather than closed over.argsis absent, andargsis required on the forwarding overload so it cannot be selected by accident.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's documented fix is a default-argument binding — which mypy then refuses to infer:
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
mastersuffers from this — they all use named nested functions. This is a preventative ergonomic change, not a repair of live breakage.Before / After
Design decisions
Overloads rather than one widened signature. Widening
funtoCallable[..., 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 takesCallable[[], bool]and has noargsparameter at all, the second takesCallable[..., bool]withargsrequired — no default — so the strict overload is selected wheneverargsis absent.argsis keyword-only.secondsis already positional-second, so a positionalargswould 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: ignoreand no# noqain the filetest_retry_until_args_reach_predicate— the tuple arrives at the predicate on every attemptuv run ruff check .clean, which is the assertion that B023 no longer firesuv run mypyclean, which is the assertion that the overloads resolveretry_untilcollected and runuv run py.test --reruns 0cleanjust build-docscleanNote for the merger
The
CHANGESentry 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.