exitCode always 0 when encountering stale approvals
When errorOnStaleApprovedFiles is set to true, and the code finds stale approvals, it exits in a way that leaves the process exit code at 0.
| library | version |
|---|---|
node |
14.5 |
mocha |
6.2.2 |
approvals |
3.0.5 |
It took me the longest time to figure out why my mocha unit tests were always returning 0 even when tests failed. Eventually I considered interaction between mocha and other libraries and it occurred to me to investigate how the approvals library may interact with exitCode.
This led me quickly to issue #7 and commit 6e8e859fd91334ee9db47dc93b1bf91f9f8d0112 which introduced this https://github.com/approvals/Approvals.NodeJS/blob/6e8e859fd91334ee9db47dc93b1bf91f9f8d0112/lib/Approvals.js#L70-L72
It turns out that mocha also uses the process.on('exit', function(..) { ... }) pattern to set process.exitCode, so when this approvals postRunCleanup handler squats on the same exit event and throws, the process seems to exit without mocha's handler properly setting the exit code.
This is significant because we expect builds to fail when tests fail. I also expect builds to fail when stale approvals are found if the flag is set such that we are running this check.
hacktoberfest-ready 🎃 😁
@davidalpert I think I'd like to understand more about why you are not getting the proper exit code. I ran a test locally and cannot seem to reproduce.
| library | version |
|---|---|
node |
14.6.0 |
mocha |
6.0.2 |
approvals |
3.0.5 && master/latest code |
===> ./node_modules/.bin/mocha test/approvalsTests.js
approvals
verify
✓ can verify some manual text
✓ should verify an image
(node:81860) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
✓ should verify an image 2
verifyAsJSON
✓ can verify some json object
✓ can be run with same JSON but keys ordered differently
verifyAsJSONAndScrub
✓ can verify and scrub some json object
6 passing (37ms)
/Users/jjarrett/code/personal/Approvals.NodeJS/lib/postRunCleanup.js:55
throw new Error('ERROR: Found stale approvals files: \n - ' + staleApprovals.join('\n - ') + '\n');
^
Error: ERROR: Found stale approvals files:
- /Users/jjarrett/code/personal/Approvals.NodeJS/test/commandlineTest.approved.txt
- /Users/jjarrett/code/personal/Approvals.NodeJS/test/FileApprover.should_verify_two_files_match.approved.txt
- /Users/jjarrett/code/personal/Approvals.NodeJS/test/FileApproverTests.ByteOrderMark.approved.txt
- /Users/jjarrett/code/personal/Approvals.NodeJS/test/postRunCleanup_reporting_bad_file.approved.txt
at module.exports (/Users/jjarrett/code/personal/Approvals.NodeJS/lib/postRunCleanup.js:55:13)
at process.<anonymous> (/Users/jjarrett/code/personal/Approvals.NodeJS/lib/Approvals.js:42:3)
at process.emit (events.js:326:22)
===> echo $?
1