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

WIP feat: add switch to suppress reporting of test failure causes #5676

Closed
wants to merge 3 commits into from

Conversation

jeysal
Copy link
Contributor

@jeysal jeysal commented Feb 27, 2018

Summary

As requested in #5358, this PR adds a --noErrorDetails switch that suppresses information on why tests failed, instead showing only what and how many tests failed.
@SimenB

Test plan

Integration test added.

When intentionally breaking one test of Jest and running with --noErrorDetails, the failing test is now displayed in a single line:
image
as opposed to displaying the full details without the switch:
image

TODO

  • is --noErrorDetails a reasonable name for the switch?
  • update doc
  • update changelog
  • Summary of all failing tests

Summary of all failing tests

image
This is not pretty.
Should we just omit the "Summary of all failing tests" section if this switch is enabled?
Might also be annoying to test, because we would need to run summary_reporter.TEST_SUMMARY_THRESHOLD + 1 (21) tests to verify this behavior.

@SimenB
Copy link
Member

SimenB commented Feb 27, 2018

I think we should either do this, or somehow allow users to customize the output as it goes.

Might make sense to put in --silent, but I think that's quite a big change.

@SimenB SimenB requested review from cpojer and thymikee February 27, 2018 07:16
@SimenB
Copy link
Member

SimenB commented Feb 27, 2018

@jeysal mind fixing the test on windows?

@cpojer
Copy link
Member

cpojer commented Feb 27, 2018

I'm a bit confused about this – what exactly is the value that this feature provides? --silent is meant to silence console output from users. When you run a test, even on CI, and you remove the test failure details, you lose all the information about what happened. I do not think this would be a good addition to Jest and would like to see this in a custom reporter rather than a part of Jest.

@jeysal
Copy link
Contributor Author

jeysal commented Feb 27, 2018

I think the purpose is similar to that of --silent — avoiding pollution of the test results so that it is easier to get an overview of what succeeded and what failed - not on CI, but during development in watch mode. When I am in the middle of refactoring something or have a full test suite for a new feature to implement, I know that a certain group of tests cannot currently work, but I want to make sure that I'm not breaking anything else. Without the switch, the expected test failures can take up more than half the terminal height each, so this becomes really difficult. With the switch, the results will even be on one page if the project has like 30 test suites.

@jeysal
Copy link
Contributor Author

jeysal commented Feb 27, 2018

@SimenB will do.

@cpojer
Copy link
Member

cpojer commented Feb 27, 2018

Yeah, as said, I don't think the output of this is useful and it requires to re-run tests to understand what is going on. If you have a flaky test, this may be really confusing to people who are not experts with Jest. I think it's best to implement this as a custom reporter so you can use Jest to your liking without possibly impacting every other user of Jest. Thank you for submitting a PR, though.

@cpojer cpojer closed this Feb 27, 2018
@theoutlander
Copy link

Was this PR not accepted?

There's clearly a need for this feature. I don't mind re-running tests if I choose to suppress it via a CLI option. It's not a big deal. If I want details, I shouldn't suppress the output details. It's really that simple.

@jeysal
Copy link
Contributor Author

jeysal commented May 24, 2018

@theoutlander I imagine what @cpojer is worried about is a bad experience for users who are not knowingly applying the flag (e.g. because someone mindlessly put it into a npm script) and wonder why they do not see the reason for test failures.

@sunfit
Copy link

sunfit commented Jan 11, 2019

@jeysal A lot of different things can be put in to an npm script and then mindlessly applied. I don't think it is a valid reason.

There are at least 4 npm packages for custom reporters that provide this behaviour, and probably many more private implementations. Also at least 3 git issues.

So I agree with @theoutlander that this is clearly a needed feature.
Just put the diffs at the top, and then the summary below them, no need to hide them completely if there is fear for the user experience. Example:
jestsummaryreporter

@meerkat-citronella
Copy link

bump. this is one of the most frustrating aspects of jest for me. I am running like 300 tests, 90% of which fail, and I just want to see which ones actually succeeded so I can build on those. I have to scroll up through hundreds of terminal lines to see the summary. And then when I add more tests, inevitably I run out of terminal lines and i have to adjust the line limit. I am not smart enough to make my own custom reporter. And the packages I've found from similar people are not useful. This should be a common enough issue to make it a feature, no?

@github-actions
Copy link

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 May 11, 2021
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.

7 participants