Skip to content

Commit 2a51ae4

Browse files
szabolcsitBridgeAR
authored andcommitted
test: cover thenables when we check for promises
Added tests that cover the issue when assert.rejects() and assert.doesNotReject() should not accept Thenables without a `catch` method or any Thenable function with `catch` and `then` methods attached. PR-URL: #24219 Reviewed-By: Ruben Bridgewater <[email protected]>
1 parent d6317d0 commit 2a51ae4

File tree

2 files changed

+80
-5
lines changed

2 files changed

+80
-5
lines changed

‎lib/assert.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,11 @@ function checkIsPromise(obj){
600600
// Accept native ES6 promises and promises that are implemented in a similar
601601
// way. Do not accept thenables that use a function as `obj` and that have no
602602
// `catch` handler.
603+
604+
// TODO: thenables are checked up until they have the correct methods,
605+
// but according to documentation, the `then` method should receive
606+
// the `fulfill` and `reject` arguments as well or it may be never resolved.
607+
603608
returnisPromise(obj)||
604609
obj!==null&&typeofobj==='object'&&
605610
typeofobj.then==='function'&&

‎test/parallel/test-assert-async.js‎

Lines changed: 75 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,34 @@
22
constcommon=require('../common');
33
constassert=require('assert');
44

5-
// Test assert.rejects() and assert.doesNotReject() by checking their
6-
// expected output and by verifying that they do not work sync
7-
85
// Run all tests in parallel and check their outcome at the end.
96
constpromises=[];
107

8+
// Thenable object without `catch` method,
9+
// shouldn't be considered as a valid Thenable
10+
constinvalidThenable={
11+
then: (fulfill,reject)=>{
12+
fulfill();
13+
},
14+
};
15+
16+
// Function that returns a Thenable function,
17+
// a function with `catch` and `then` methods attached,
18+
// shouldn't be considered as a valid Thenable.
19+
constinvalidThenableFunc=()=>{
20+
functionf(){}
21+
22+
f.then=(fulfill,reject)=>{
23+
fulfill();
24+
};
25+
f.catch=()=>{};
26+
27+
returnf;
28+
};
29+
30+
// Test assert.rejects() and assert.doesNotReject() by checking their
31+
// expected output and by verifying that they do not work sync
32+
1133
// Check `assert.rejects`.
1234
{
1335
constrejectingFn=async()=>assert.fail();
@@ -16,9 +38,34 @@ const promises = [];
1638
name: 'AssertionError',
1739
message: 'Failed'
1840
};
19-
// `assert.rejects` accepts a function or a promise as first argument.
41+
42+
// `assert.rejects` accepts a function or a promise
43+
// or a thenable as first argument.
2044
promises.push(assert.rejects(rejectingFn,errObj));
2145
promises.push(assert.rejects(rejectingFn(),errObj));
46+
47+
constvalidRejectingThenable={
48+
then: (fulfill,reject)=>{
49+
reject({code: 'FAIL'});
50+
},
51+
catch: ()=>{}
52+
};
53+
promises.push(assert.rejects(validRejectingThenable,{code: 'FAIL'}));
54+
55+
// `assert.rejects` should not accept thenables that
56+
// use a function as `obj` and that have no `catch` handler.
57+
promises.push(assert.rejects(
58+
assert.rejects(invalidThenable,{}),
59+
{
60+
code: 'ERR_INVALID_ARG_TYPE'
61+
})
62+
);
63+
promises.push(assert.rejects(
64+
assert.rejects(invalidThenableFunc,{}),
65+
{
66+
code: 'ERR_INVALID_RETURN_VALUE'
67+
})
68+
);
2269
}
2370

2471
{
@@ -69,7 +116,8 @@ promises.push(assert.rejects(
69116

70117
// Check `assert.doesNotReject`.
71118
{
72-
// `assert.doesNotReject` accepts a function or a promise as first argument.
119+
// `assert.doesNotReject` accepts a function or a promise
120+
// or a thenable as first argument.
73121
constpromise=assert.doesNotReject(()=>newMap(),common.mustNotCall());
74122
promises.push(assert.rejects(promise,{
75123
message: 'Expected instance of Promise to be returned '+
@@ -79,6 +127,28 @@ promises.push(assert.rejects(
79127
}));
80128
promises.push(assert.doesNotReject(async()=>{}));
81129
promises.push(assert.doesNotReject(Promise.resolve()));
130+
131+
// `assert.doesNotReject` should not accept thenables that
132+
// use a function as `obj` and that have no `catch` handler.
133+
constvalidFulfillingThenable={
134+
then: (fulfill,reject)=>{
135+
fulfill();
136+
},
137+
catch: ()=>{}
138+
};
139+
promises.push(assert.doesNotReject(validFulfillingThenable));
140+
promises.push(assert.rejects(
141+
assert.doesNotReject(invalidThenable),
142+
{
143+
code: 'ERR_INVALID_ARG_TYPE'
144+
})
145+
);
146+
promises.push(assert.rejects(
147+
assert.doesNotReject(invalidThenableFunc),
148+
{
149+
code: 'ERR_INVALID_RETURN_VALUE'
150+
})
151+
);
82152
}
83153

84154
{

0 commit comments

Comments
(0)