Repository navigation
Fix capture asymmetry in suspend_global_capture(in_=True) - #14356
FazeelUsmani wants to merge 5 commits into
Conversation
for more information, see https://pre-commit.ci
RonnyPfannschmidt
left a comment
There was a problem hiding this comment.
at first glacne this looks good, but also makes the age and the state-managment issues of the apis apparent
i'll need to do a deeper review
| EMPTY_BUFFER: AnyStr | ||
| # Set by stateful subclasses in ``__init__``; left as ``None`` on | ||
| # stateless implementations such as ``NoCapture``. | ||
| _state: str | None = None |
There was a problem hiding this comment.
we should use a literal or pivot to a enum
There was a problem hiding this comment.
Went with a Literal, matching _CaptureMethod right above it. _assert_state takes it too, so a typo'd state now fails mypy.
|
@RonnyPfannschmidt Thanks for taking a look. Agreed, the fix is local. The underlying issue is that suspend/resume infer state from whichever streams happen to be active at call time rather than tracking it explicitly. No rush on the deeper review. If a broader rework feels like the right path, happy to scope that into a separate issue. |
Fixes #13322.
If a plugin hook suspends out/err and then calls
suspend_global_capture(in_=True)(e.g. to read input), the matchingresume_global_capture()was incorrectly re-enabling out/err even though the caller had them suspended. Fix is to remember what was started before the nested suspend and only resume those.Includes a
pytesterregression that runs the flow from apytest_terminal_summaryhook and checks that the print after the resume reaches stdout before the1 passedline. Before the fix it only shows up at teardown, after the summary.