Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow only showing stdout or stderr when using --success-output and --failure-output #1688

Open
Kriskras99 opened this issue Sep 5, 2024 · 6 comments · May be fixed by #1707
Open

Allow only showing stdout or stderr when using --success-output and --failure-output #1688

Kriskras99 opened this issue Sep 5, 2024 · 6 comments · May be fixed by #1707

Comments

@Kriskras99
Copy link

Kriskras99 commented Sep 5, 2024 •
edited
Loading

I'm writing a file parser and when I get a new version of the format I like to add the samples to my tests.
However, this means that the tests will start hitting todo!() and panic!() and this pollutes the output of the tests:

        FAIL [   0.004s] ubiart_toolkit::tape tape_parse_wiiu2015::base/tape.ckd/f054654bfd32299c39542c768be25c6e.tape.ckd

--- STDOUT:              ubiart_toolkit::tape tape_parse_wiiu2015::base/tape.ckd/f054654bfd32299c39542c768be25c6e.tape.ckd ---

running 1 test
test tape_parse_wiiu2015::base/tape.ckd/f054654bfd32299c39542c768be25c6e.tape.ckd ... FAILED

failures:

---- tape_parse_wiiu2015::base/tape.ckd/f054654bfd32299c39542c768be25c6e.tape.ckd ----
test panicked: not yet implemented


failures:
    tape_parse_wiiu2015::base/tape.ckd/f054654bfd32299c39542c768be25c6e.tape.ckd

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s


--- STDERR:              ubiart_toolkit::tape tape_parse_wiiu2015::base/tape.ckd/f054654bfd32299c39542c768be25c6e.tape.ckd ---
thread '<unnamed>' panicked at ubiart_toolkit/src/cooked/tape/mod.rs:2087:9:
not yet implemented
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

In these cases stdout has no useful additional information compared to stderr, so I would like to disable it.
I was thinking of the following:

  • --failure-output immediate -> outputs both stdout and stderr (current case)
  • --failure-output stdout=never -> does not output to stdout, uses default for stderr
  • --failure-output stdout=never,stderr=immediate -> does not output to stdout, immediate output for stderr

Another option is adding more flags.
@sunshowers
Copy link
Member

Thanks! I think the general suggestion makes sense, and I like the stdout=never,stderr=immediate approach.

In the specific case of the output generated by libtest, I actually want to just start filtering that out -- it's been somewhat annoying for sure.

A few things that come to mind:

  1. We should add an equivalent to the config, via success-output and failure-output also taking a table in addition to a string.
  2. Nextest internally supports combining stdout and stderr. This is only used for the unstable libtest feature, but there are other use cases where that's useful as well. It would be nice to have this feature make sense for combined outputs as well. Thoughts? (Maybe we just use the stdout setting in that case.)

@Kriskras99
Copy link
Author

For combined outputs I would expect that it would apply the filter before combining them.
My expectations of how these flags affect libtest output:

  • --failure-output never would not include stdout and stderr in the json
  • --failure-output stdout=never would only include stderr
  • using immediate or final would return an error as those keys don't make sense when using the libtest output.

@sunshowers
Copy link
Member

For combined outputs I would expect that it would apply the filter before combining them.

Ah sadly that's not possible on Unix.

@Kriskras99
Copy link
Author

Then I would suggest only supporting never when using libtest output, and returning an error when a user inputs stdout=never or stderr=never or a combination of the two.
Otherwise you're going to get bug reports that setting stdout=never also disables stderr (when outputting libtest json), or that stderr=never doesn't work

@sunshowers
Copy link
Member

Then I would suggest only supporting never when using libtest output, and returning an error when a user inputs stdout=never or stderr=never or a combination of the two.

I definitely see where this is coming from! I think that might end up denying too much, though -- especially if specified in configuration this can be quite annoying in practice. Also, we'll likely support combined output not just via libtest (as we do today) but also via per-test settings.

I think if at least one test is run with combined output, I would:

  • use the stdout configuration
  • if the stdout and stderr configurations are different, warn about it

Thanks for your help designing this! Would you also like to implement it? Otherwise I can get to this in the next few weeks.

@Kriskras99
Copy link
Author

I'll have a try this weekend 👍

@Kriskras99 Kriskras99 linked a pull request Sep 12, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add support for silencing only one of the outputs of a test Kriskras99/nextest
2 participants
@sunshowers @Kriskras99