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.

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

fix: make reporters respect useStderr option #6583

Closed
wants to merge 2 commits into from

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Jun 30, 2018

Summary

useStderr is currently respected for console output, but not our reporter. That's perhaps on purpose, but it was en easy enough fix to make, so I thought why not 🙂

If this is not accepted, the issue should be closed.

Fixes #5064

Test plan

Ran script in #5064 and it now printed stdout instead.

@SimenB
Copy link
Member Author

SimenB commented Jun 30, 2018

Ugh, this broke horribly :(

@aaronabramov
Copy link
Contributor

i also remember that we were not buffering stderr in reporters, because if something goes terribly wrong, writes to stderr and blows up we wouldn't get any error printed

@connorjclark
Copy link
Contributor

Would benefit from this fix. @SimenB , any insight on what broke? Would be happy to jump in if you don't have the time.

@SimenB
Copy link
Member Author

SimenB commented Jul 26, 2018

I don't think any functionality broke. A lot is how the integration tests are written - they fetch errors from stderr and have expectations on what's there and what's not. Feel free to jump in and take a look :)

@connorjclark
Copy link
Contributor

Ah, understood. Will try tonight.

@thymikee
Copy link
Collaborator

thymikee commented Aug 7, 2018

@SimenB you think it makes sense to pursue fixing all the tests?

@MitchLillie
Copy link

Is this still in progress? Would love to see a resolution to this issue, happy to delve into helping with tests as well

@SimenB
Copy link
Member Author

SimenB commented Oct 24, 2018

Help would be greatly appreciated!

An aside is, should we print the summary to stderr if we exit with code 1 regardless if useStderr is set or not?

@ByronBecker
Copy link

ByronBecker commented Jul 12, 2019

@SimenB @thymikee Is this feature still in progress? It seems someone has made a package to address this issue -> https://github.com/chrisgalvan/jest-standard-reporter

@lucasfcosta
Copy link
Contributor

lucasfcosta commented Jul 14, 2019

Hi @SimenB I'm happy to help with this if needed 😊

I was taking a look at what's currently in master and it seems like your PR is pretty much what's currently needed, we just gotta rebase it.

I was also searching for any other issues or PRs blocking this one but couldn't find any. Is this what's currently blocking the PR?

An aside is, should we print the summary to stderr if we exit with code 1 regardless if useStderr is set or not?

My 2 cents: I think that the summary should be printed to stdout since it's not an error message itself. Since the summary is a piece of information which would be printed regardless of whether there's an error, I think it should go into stdout.


Just let me know what can I do to help you and I'll do my best to answer timely. I'm happy to rebase/solve conflicts and open a PR or push to your branch (given write access to it) if you prefer.

@SimenB SimenB force-pushed the reporters-stdout branch from 20c4f8a to 90f04c6 Compare July 15, 2019 07:58
@SimenB
Copy link
Member Author

SimenB commented Jul 15, 2019

Thanks @lucasfcosta! I just rebased this - feel free to open up a new PR based on it which closes this one if you make it work 🙂

@marcoebbinghaus
Copy link

any news on this topic?

@kenrick95
Copy link
Contributor

kenrick95 commented May 18, 2020

Hi, wanna let you know that in the past few days, I attempted to fix all these errors and now have a PR in which all tests are passing (#10054), but I apologize for touching so many files (113 files changed) 😨

I hope the maintainer can slowly take their time to review the changes and I will help to keep rebasing/resolving conflicts on the branch if needed

@PabloSzx
Copy link

any news?

@TheBit
Copy link

TheBit commented Jan 2, 2022

Any news?

@github-actions
Copy link

github-actions bot commented Jan 2, 2023

This PR is stale because it has been open 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Jan 2, 2023
@github-actions
Copy link

github-actions bot commented Feb 1, 2023

This PR was closed because it has been stalled for 30 days with no activity. Please open a new PR if the issue is still relevant, linking to this one.

@github-actions github-actions bot closed this Feb 1, 2023
@github-actions
Copy link

github-actions bot commented Mar 4, 2023

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jest is using stderr as output