Skip to content

tests(painter): wrapper enum to allow tests to write to sink#1098

Merged
kronberger-droid merged 2 commits into
nushell:mainfrom
kronberger-droid:painter-test-sink
Jun 9, 2026
Merged

tests(painter): wrapper enum to allow tests to write to sink#1098
kronberger-droid merged 2 commits into
nushell:mainfrom
kronberger-droid:painter-test-sink

Conversation

@kronberger-droid

Copy link
Copy Markdown
Collaborator

This is in rensponse to #1078, which had to use a workaround to not leak test output into the terminal.

I solved this using a wrapper enum:

pub enum W {
    #[cfg_attr(test, allow(dead_code))]
    Terminal(std::io::BufWriter<std::io::Stderr>),
    #[cfg(test)]
    Sink(std::io::Sink),
}

Allowing to either paint to the Terminal or to a Sink, which will be used in tests.

@fdncred @ymcx
What do you think about using this instead of the workaround in #1078?

kronberger-droid and others added 2 commits May 28, 2026 13:40
Restored from stash for safekeeping on the branch.
Not ready for review.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@fdncred

fdncred commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

I think it's probably ok for now but in the future I'd like to have a way of running tests and capturing ansi escapes to ensure that they're correct. It seems like this would prevent that right now.

@kronberger-droid

Copy link
Copy Markdown
Collaborator Author

Hmm, my idea for that would be to add another field with Buffer (Vec<u8>), which allows you to capture the Bytes without writing it to anything.

@fdncred
What do you think about this?
It would still not fully solve the problem since checking the raw bytes is still unreliable, but there apparently are crates that can transform it into the escape codes.

@fdncred

fdncred commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

I'm ok with this and also adding a Buffer later if we get to it.

@kronberger-droid kronberger-droid merged commit 46a0e0c into nushell:main Jun 9, 2026
7 checks passed
kronberger-droid added a commit to kronberger-droid/reedline that referenced this pull request Jun 13, 2026
…ll#1098)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

2 participants