fix(painter): keep the cursor position query off the tty in tests - #1156
Open
kronberger-droid wants to merge 1 commit into
Open
fix(painter): keep the cursor position query off the tty in tests#1156kronberger-droid wants to merge 1 commit into
kronberger-droid wants to merge 1 commit into
Conversation
crossterm queries the file descriptor directly, bypassing libtest's capture, so every test that painted with a stale anchor sent `ESC[6n` to the real terminal and blocked on crossterm's timeout, serialized behind its global reader lock. A full run could stall for a minute until a keypress completed the cooked-mode line; on CI without a tty the read loop never returns. `cursor::position()` now goes through the painter's writer. The test writers answer with an error, which is the branch those paints already took after the timeout. The one-off `cfg(not(test))` around the resize query goes away with it.
Contributor
|
Is this the cause to the ESC[6n we see sometimes? We have people report these not knowing what it is because the terminal/nushell is reporting the cursor position. |
Collaborator
Author
|
Well I don't know, my starting point was a blocking test which was using the sink, waiting for a cursor position. Do we have an issue on the ESC[6n then i check it out. |
Contributor
|
We probably do in nushell but I doubt it contains the string ESC[6n. I think this is one of them https://github.com/nushell/nushell/issues/15312 |
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
2 participants
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
Painteralready routes all writes throughWso tests don't hit the terminal, since crossterm bypasses libtest's capture.The read side slipped through: four
cursor::position()calls went straight to crossterm.Any test that painted with a stale anchor (every deferred-completion test does, via
invalidate_anchor_if_host_completer_runs) sentESC[6nto the real tty and waited on crossterm's timeout, serialized behind its global reader lock.Locally that shows as
a_lone_late_suggestion_is_accepted_rather_than_spliced has been running for over 60 secondsuntil a keypress completes the cooked-mode line; on CI without a tty the read loop never returns.Wgets acursor_position()that delegates forTerminaland returns an error for the test writers, which is the branch those paints already took after the timeout, thus no test semantics moved.The one-off
#[cfg(not(test))]around the resize query is gone with it.No public API change, no behavior change on the terminal.
Additional notes
Full
cargo test --libwith stdin closed: 1487 passed in 0.02s, no keypress needed.Follow-up:
(column, row)and the other painter tuples could use a smallCursorPos-style type.