-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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 option to not fail on failing test suite #4771
feat: add option to not fail on failing test suite #4771
Conversation
This PR hasn't had any recent activity, and I'm labeling it |
Ping |
This PR hasn't had any recent activity, and I'm labeling it |
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.
Looking good as a start! The logical flow makes sense and seems reasonable to me. 🔥
Requesting changes mostly on docs/naming and testing. A bit of code shuffling too, but I'm open to getting told I'm being overly nitpicky. 😄
👋 ping @ilgonmic, is this still something you have time for? |
Hi @JoshuaKGoldberg |
5c1be0d
to
964fa2c
Compare
@JoshuaKGoldberg Hi, I tried to fix all concers, could you please take a look? |
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.
🙌 This looks great, thanks so much @ilgonmic!
I'll merge and release this in a new minor version soon. 🚀
return code => { | ||
const clampedCode = passOnFailingTestSuite | ||
? 0 | ||
: Math.min(code, 255); |
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.
[Praise] I like the moving of the Math.min(code, 255)
here, nice spot!
Published in |
Requirements
Description of the Change
Fixed #4216
Add possibility to distinguish 2 types of fails:
When there are problems with test run, it is definitely problem.
When tests were run, but there are some test fails, it can be considered as ok.
It is useful to use Mocha in integrations with different build tools (for example Gradle), when we extract information from Mocha about tests and pass it to build tool and check exit code of Mocha.
Karma has similar feature
Alternate Designs
It can be done with 2 runs of Mocha:
dry-run
and check if JavaScript code can be run without problemsBut it requires 2 runs, which is not very convenient.
Why should this be in core?
When using Mocha and look at exit code, it is impossible to distinguish 2 cases
But these 2 cases can be completely different
Benefits
New useful feature for build pipelines integrations
Possible Drawbacks
Applicable issues
#4216