-
Notifications
You must be signed in to change notification settings - Fork 72
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
Feat: Add errorMessageFormatter #468
Conversation
@yannbf I couldn't push to your branch so had to create a sep PR, I did not update docs (is that in the main SB repo?) and I did not remove the changes that should not be merged which you added, I can remove them once this PR is approved. |
Hey @Foxhoundn thanks for providing the code! I think we need to think a bit more about this since it should be a common function for errors, and right now it seems to be applied only if an exception is coming from the play function. @kasperpeulen @shilman do you want to chime in regarding:
I would say this function should be applicable to other error events too, and receive as second argument some metadata (whenever possible) that contains at least the storyId, so users have more flexibility to know how to deal with formatting their messages. |
That would be splendid - I would love to have a list of stories that failed at the end of the test run too, right now it just says 5 test suites, 25 tests. Then you have to scroll through the whole history to find those 25 tests, but it would be wonderful to have the actual story list there, for easy orientation. Something to think about? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be even better if we could fix the problem at its root and provide better error messages out of the box. But in the meantime, this change looks totally reasonable to me as a workaround. 💯
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #468 +/- ##
==========================================
- Coverage 93.86% 93.19% -0.67%
==========================================
Files 15 15
Lines 277 294 +17
Branches 71 78 +7
==========================================
+ Hits 260 274 +14
- Misses 17 20 +3 ☔ View full report in Codecov by Sentry. |
🚀 PR was released in |
Closes storybookjs/storybook#25131
What I did
The
errorMessageFormatter
property defines a function that will pre-format the error messages before they get reported in the CLI:Here's a small example of the formatting
If the formatter throws an error, the test-runner will notify the user
Checklist for Contributors
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
Checklist for Maintainers
Make sure this PR contains one of the labels below:
Available labels
skip-release
: Skip any releases, e.g., documentation only changes, CI config etc.patch
: Upgrade patch version (e.g. 0.0.x)minor
: Upgrade patch version (e.g. 0.x.0)major
: Upgrade patch version (e.g. x.0.0)