fix(jest task): Return error when tests fail #1053
Conversation
|
Result after change in azure devops pipeline forcefully making a test fail:
|
Callback always resolved successfully even when test failed Incorrect data was accessed in result data to determine failure Closes aurelia#1052
michaelw85
force-pushed
the
1052_fix_jest_no_error
branch
from
b42c7ce to
bfbe072
Compare
lib/resources/tasks/jest.js
Outdated
| cb(new PluginError('gulp-jest', { message: 'Tests Failed' })); | ||
| jest.runCLI(options, [path.resolve(__dirname, '../../')]).then(({ results }) => { | ||
| if (results.numFailedTests || results.numFailedTestSuites) { | ||
| cb(new PluginError('jest-cli', { message: 'Tests Failed' })); |
There was a problem hiding this comment.
It looks like (existing code) new PluginError generated excessive output in the console. To my limited understanding of gulp, it seems this is not a place for PluginError because it's not within context of a plugin.
Following gulpjs's example, cb(new Error('Tests Failed')); is enough and probably the correct way. @JeroenVinke can you have a look?
There was a problem hiding this comment.
I have hardly any experience with Gulp so left it in place, I agree the output seems excessive but it resolved the issue which was more important in my case. I will replace it with an error in my local setup and test to see if it also forces a build failure (I assume it does). If it needs to be replaced please let me know and I will update the PR.
There was a problem hiding this comment.
BTW, it looks jest is only feature in cli that uses plugin-error as a dep. If we decide to get rid of PluginError here, we can cleanup the dependencies.json and unit-test-runners/jest.js too.
There was a problem hiding this comment.
When using new Error('...') the build fails as expected:
@huochunpeng I think we can replace the PluginError.
There was a problem hiding this comment.
Today I also noticed just returning a string in the callback is enough to make it show an error.
There was a problem hiding this comment.
Today I also noticed just returning a string in the callback is enough to make it show an error.
That's even better! Let's just clean up everything.
There was a problem hiding this comment.
@huochunpeng Updated and removed plugin-error package.
Remove plugin error, returning a message is enough to make the process emit an error.
|
@EisenbergEffect this can be merged. |
Callback always resolved successfully even when test failed
Incorrect data was accessed in result data to determine failure
Closes #1052