FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: make crashOnUnhandleRejection opt-out · nodejs/node@bea1ee8 · GitHub

/ node Public

Commit bea1ee8

Browse files
targos
committed
test: make crashOnUnhandleRejection opt-out
This commit removes `common.crashOnUnhandledRejection()` and adds `common.disableCrashOnUnhandledRejection()`. To reduce the risk of mistakes and make writing tests that involve promises simpler, always install the unhandledRejection hook in tests and provide a way to disable it for the rare cases where it's needed. PR-URL: #21849 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
1 parent 9817e40 commit bea1ee8

117 files changed

Lines changed: 58 additions & 241 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎doc/guides/writing-tests.md‎

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -225,34 +225,24 @@ countdown.dec(); // The countdown callback will be invoked now.
225225

226226
#### Testing promises
227227

228-
When writing tests involving promises, either make sure that the
229-
`onFulfilled` or the `onRejected` handler is wrapped in
230-
`common.mustCall()` or `common.mustNotCall()` accordingly, or
231-
call `common.crashOnUnhandledRejection()` in the top level of the
232-
test to make sure that unhandled rejections would result in a test
233-
failure. For example:
228+
When writing tests involving promises, it is generally good to wrap the
229+
`onFulfilled` handler, otherwise the test could successfully finish if the
230+
promise never resolves (pending promises do not keep the event loop alive). The
231+
`common` module automatically adds a handler that makes the process crash - and
232+
hence, the test fail - in the case of an `unhandledRejection` event. It is
233+
possible to disable it with `common.disableCrashOnUnhandledRejection()` if
234+
needed.
234235

235236
```javascript
236237
const common = require('../common');
237238
const assert = require('assert');
238239
const fs = require('fs').promises;
239240

240-
// Use `common.crashOnUnhandledRejection()` to make sure unhandled rejections
241-
// will fail the test.
242-
common.crashOnUnhandledRejection();
243-
244-
// Or, wrap the `onRejected` handler in `common.mustNotCall()`.
245-
fs.writeFile('test-file', 'test').catch(common.mustNotCall());
246-
247-
// Or, wrap the `onFulfilled` handler in `common.mustCall()`.
248-
// If there are assertions in the `onFulfilled` handler, wrap
249-
// the next `onRejected` handler in `common.mustNotCall()`
250-
// to handle potential failures.
241+
// Wrap the `onFulfilled` handler in `common.mustCall()`.
251242
fs.readFile('test-file').then(
252243
common.mustCall(
253244
(content) => assert.strictEqual(content.toString(), 'test2')
254-
))
255-
.catch(common.mustNotCall());
245+
));
256246
```
257247

258248
### Flags

‎test/addons-napi/test_promise/test.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ const common = require('../../common');
77
const assert = require('assert');
88
const test_promise = require(`./build/${common.buildType}/test_promise`);
99

10-
common.crashOnUnhandledRejection();
11-
1210
// A resolution
1311
{
1412
const expected_result = 42;

‎test/addons-napi/test_threadsafe_function/test.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ const expectedArray = (function(arrayLength) {
1212
return result;
1313
})(binding.ARRAY_LENGTH);
1414

15-
common.crashOnUnhandledRejection();
16-
1715
// Handle the rapid teardown test case as the child process. We unref the
1816
// thread-safe function after we have received two values. This causes the
1917
// process to exit and the environment cleanup handler to be invoked.

‎test/addons/callback-scope/test-resolve-async.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ const common = require('../../common');
44
const assert = require('assert');
55
const { testResolveAsync } = require(`./build/${common.buildType}/binding`);
66

7-
common.crashOnUnhandledRejection();
8-
97
let called = false;
108
testResolveAsync().then(() => { called = true; });
119

‎test/addons/make-callback-recurse/test.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ const makeCallback = binding.makeCallback;
99
// Make sure this is run in the future.
1010
const mustCallCheckDomains = common.mustCall(checkDomains);
1111

12-
common.crashOnUnhandledRejection();
13-
1412
// Make sure that using MakeCallback allows the error to propagate.
1513
assert.throws(function() {
1614
makeCallback({}, function() {

‎test/async-hooks/test-promise.chain-promise-before-init-hooks.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ const { checkInvocations } = require('./hook-checks');
88
if (!common.isMainThread)
99
common.skip('Worker bootstrapping works differently -> different async IDs');
1010

11-
common.crashOnUnhandledRejection();
12-
1311
const p = new Promise(common.mustCall(function executor(resolve, reject) {
1412
resolve(5);
1513
}));

‎test/async-hooks/test-promise.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ const { checkInvocations } = require('./hook-checks');
99
if (!common.isMainThread)
1010
common.skip('Worker bootstrapping works differently -> different async IDs');
1111

12-
common.crashOnUnhandledRejection();
13-
1412
const hooks = initHooks();
1513

1614
hooks.enable();

‎test/common/README.md‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,19 @@ symlinks
5555
([SeCreateSymbolicLinkPrivilege](https://msdn.microsoft.com/en-us/library/windows/desktop/bb530716(v=vs.85).aspx)).
5656
On non-Windows platforms, this always returns `true`.
5757

58-
### crashOnUnhandledRejection()
59-
60-
Installs a `process.on('unhandledRejection')` handler that crashes the process
61-
after a tick. This is useful for tests that use Promises and need to make sure
62-
no unexpected rejections occur, because currently they result in silent
63-
failures.
64-
6558
### ddCommand(filename, kilobytes)
6659
* return [&lt;Object>]
6760

6861
Platform normalizes the `dd` command
6962

63+
### disableCrashOnUnhandledRejection()
64+
65+
Removes the `process.on('unhandledRejection')` handler that crashes the process
66+
after a tick. The handler is useful for tests that use Promises and need to make
67+
sure no unexpected rejections occur, because currently they result in silent
68+
failures. However, it is useful in some rare cases to disable it, for example if
69+
the `unhandledRejection` hook is directly used by the test.
70+
7071
### enoughTestMem
7172
* [&lt;boolean>]
7273

‎test/common/index.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -815,9 +815,10 @@ exports.getBufferSources = function getBufferSources(buf) {
815815
};
816816

817817
// Crash the process on unhandled rejections.
818-
exports.crashOnUnhandledRejection = function() {
819-
process.on('unhandledRejection',
820-
(err) => process.nextTick(() => { throw err; }));
818+
const crashOnUnhandledRejection = (err) => { throw err; };
819+
process.on('unhandledRejection', crashOnUnhandledRejection);
820+
exports.disableCrashOnUnhandledRejection = function() {
821+
process.removeListener('unhandledRejection', crashOnUnhandledRejection);
821822
};
822823

823824
exports.getTTYfd = function getTTYfd() {

‎test/common/index.mjs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ const {
5252
skipIf32Bits,
5353
getArrayBufferViews,
5454
getBufferSources,
55-
crashOnUnhandledRejection,
55+
disableCrashOnUnhandledRejection,
5656
getTTYfd,
5757
runWithInvalidFD,
5858
hijackStdout,
@@ -112,7 +112,7 @@ export {
112112
skipIf32Bits,
113113
getArrayBufferViews,
114114
getBufferSources,
115-
crashOnUnhandledRejection,
115+
disableCrashOnUnhandledRejection,
116116
getTTYfd,
117117
runWithInvalidFD,
118118
hijackStdout,

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL