Skip to content

Commit 116c1fe

Browse files
pmarchinitargos
authored andcommitted
test_runner: refactor testPlan counter increse
PR-URL: #56765 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent 51dff8b commit 116c1fe

7 files changed

Lines changed: 377 additions & 30 deletions

File tree

‎doc/api/test.md‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3398,19 +3398,33 @@ added:
33983398

33993399
The name of the test.
34003400

3401-
### `context.plan(count)`
3401+
### `context.plan(count[,options])`
34023402

34033403
<!-- YAML
34043404
added:
34053405
- v22.2.0
34063406
- v20.15.0
34073407
changes:
3408+
- version:
3409+
- REPLACEME
3410+
pr-url: https://github.com/nodejs/node/pull/56765
3411+
description: Add the `options` parameter.
34083412
- version: v23.4.0
34093413
pr-url: https://github.com/nodejs/node/pull/55895
34103414
description: This function is no longer experimental.
34113415
-->
34123416

34133417
*`count` {number} The number of assertions and subtests that are expected to run.
3418+
*`options` {Object} Additional options for the plan.
3419+
*`wait` {boolean|number} The wait time for the plan:
3420+
* If `true`, the plan waits indefinitely for all assertions and subtests to run.
3421+
* If `false`, the plan performs an immediate check after the test function completes,
3422+
without waiting for any pending assertions or subtests.
3423+
Any assertions or subtests that complete after this check will not be counted towards the plan.
3424+
* If a number, it specifies the maximum wait time in milliseconds
3425+
before timing out while waiting for expected assertions and subtests to be matched.
3426+
If the timeout is reached, the test will fail.
3427+
**Default:**`false`.
34143428

34153429
This function is used to set the number of assertions and subtests that are expected to run
34163430
within the test. If the number of assertions and subtests that run does not match the
@@ -3449,6 +3463,26 @@ test('planning with streams', (t, done) => {
34493463
});
34503464
```
34513465

3466+
When using the `wait` option, you can control how long the test will wait for the expected assertions.
3467+
For example, setting a maximum wait time ensures that the test will wait for asynchronous assertions
3468+
to complete within the specified timeframe:
3469+
3470+
```js
3471+
test('plan with wait: 2000 waits for async assertions', (t) => {
3472+
t.plan(1, { wait:2000 }); // Waits for up to 2 seconds for the assertion to complete.
3473+
3474+
constasyncActivity= () => {
3475+
setTimeout(() => {
3476+
t.assert.ok(true, 'Async assertion completed within the wait time');
3477+
}, 1000); // Completes after 1 second, within the 2-second wait time.
3478+
};
3479+
3480+
asyncActivity(); // The test will pass because the assertion is completed in time.
3481+
});
3482+
```
3483+
3484+
Note: If a `wait` timeout is specified, it begins counting down only after the test function finishes executing.
3485+
34523486
### `context.runOnly(shouldRunOnlyTests)`
34533487

34543488
<!-- YAML

‎lib/internal/test_runner/test.js‎

Lines changed: 94 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -176,22 +176,88 @@ function testMatchesPattern(test, patterns) {
176176
}
177177

178178
classTestPlan{
179-
constructor(count){
179+
#waitIndefinitely =false;
180+
#planPromise =null;
181+
#timeoutId =null;
182+
183+
constructor(count,options=kEmptyObject){
180184
validateUint32(count,'count');
185+
validateObject(options,'options');
181186
this.expected=count;
182187
this.actual=0;
188+
189+
const{ wait }=options;
190+
if(typeofwait==='boolean'){
191+
this.wait=wait;
192+
this.#waitIndefinitely =wait;
193+
}elseif(typeofwait==='number'){
194+
validateNumber(wait,'options.wait',0,TIMEOUT_MAX);
195+
this.wait=wait;
196+
}elseif(wait!==undefined){
197+
thrownewERR_INVALID_ARG_TYPE('options.wait',['boolean','number'],wait);
198+
}
199+
}
200+
201+
#planMet(){
202+
returnthis.actual===this.expected;
203+
}
204+
205+
#createTimeout(reject){
206+
returnsetTimeout(()=>{
207+
consterr=newERR_TEST_FAILURE(
208+
`plan timed out after ${this.wait}ms with ${this.actual} assertions when expecting ${this.expected}`,
209+
kTestTimeoutFailure,
210+
);
211+
reject(err);
212+
},this.wait);
183213
}
184214

185215
check(){
186-
if(this.actual!==this.expected){
216+
if(this.#planMet()){
217+
if(this.#timeoutId){
218+
clearTimeout(this.#timeoutId);
219+
this.#timeoutId =null;
220+
}
221+
if(this.#planPromise){
222+
const{ resolve }=this.#planPromise;
223+
resolve();
224+
this.#planPromise =null;
225+
}
226+
return;
227+
}
228+
229+
if(!this.#shouldWait()){
187230
thrownewERR_TEST_FAILURE(
188231
`plan expected ${this.expected} assertions but received ${this.actual}`,
189232
kTestCodeFailure,
190233
);
191234
}
235+
236+
if(!this.#planPromise){
237+
const{ promise, resolve, reject }=PromiseWithResolvers();
238+
this.#planPromise ={__proto__: null, promise, resolve, reject };
239+
240+
if(!this.#waitIndefinitely){
241+
this.#timeoutId =this.#createTimeout(reject);
242+
}
243+
}
244+
245+
returnthis.#planPromise.promise;
246+
}
247+
248+
count(){
249+
this.actual++;
250+
if(this.#planPromise){
251+
this.check();
252+
}
253+
}
254+
255+
#shouldWait(){
256+
returnthis.wait!==undefined&&this.wait!==false;
192257
}
193258
}
194259

260+
195261
classTestContext{
196262
#assert;
197263
#test;
@@ -228,15 +294,15 @@ class TestContext {
228294
this.#test.diagnostic(message);
229295
}
230296

231-
plan(count){
297+
plan(count,options=kEmptyObject){
232298
if(this.#test.plan!==null){
233299
thrownewERR_TEST_FAILURE(
234300
'cannot set plan more than once',
235301
kTestCodeFailure,
236302
);
237303
}
238304

239-
this.#test.plan=newTestPlan(count);
305+
this.#test.plan=newTestPlan(count,options);
240306
}
241307

242308
getassert(){
@@ -249,7 +315,7 @@ class TestContext {
249315
map.forEach((method,name)=>{
250316
assert[name]=(...args)=>{
251317
if(plan!==null){
252-
plan.actual++;
318+
plan.count();
253319
}
254320
returnReflectApply(method,this,args);
255321
};
@@ -260,7 +326,7 @@ class TestContext {
260326
// stacktrace from the correct starting point.
261327
functionok(...args){
262328
if(plan!==null){
263-
plan.actual++;
329+
plan.count();
264330
}
265331
innerOk(ok,args.length, ...args);
266332
}
@@ -296,7 +362,7 @@ class TestContext {
296362

297363
const{ plan }=this.#test;
298364
if(plan!==null){
299-
plan.actual++;
365+
plan.count();
300366
}
301367

302368
constsubtest=this.#test.createSubtest(
@@ -959,30 +1025,46 @@ class Test extends AsyncResource {
9591025
construnArgs=ArrayPrototypeSlice(args);
9601026
ArrayPrototypeUnshift(runArgs,this.fn,ctx);
9611027

1028+
constpromises=[];
9621029
if(this.fn.length===runArgs.length-1){
963-
// This test is using legacy Node.js errorfirst callbacks.
1030+
// This test is using legacy Node.js error-first callbacks.
9641031
const{ promise, cb }=createDeferredCallback();
965-
9661032
ArrayPrototypePush(runArgs,cb);
1033+
9671034
constret=ReflectApply(this.runInAsyncScope,this,runArgs);
9681035

9691036
if(isPromise(ret)){
9701037
this.fail(newERR_TEST_FAILURE(
9711038
'passed a callback but also returned a Promise',
9721039
kCallbackAndPromisePresent,
9731040
));
974-
awaitSafePromiseRace([ret,stopPromise]);
1041+
ArrayPrototypePush(promises,ret);
9751042
}else{
976-
awaitSafePromiseRace([PromiseResolve(promise),stopPromise]);
1043+
ArrayPrototypePush(promises,PromiseResolve(promise));
9771044
}
9781045
}else{
9791046
// This test is synchronous or using Promises.
9801047
constpromise=ReflectApply(this.runInAsyncScope,this,runArgs);
981-
awaitSafePromiseRace([PromiseResolve(promise),stopPromise]);
1048+
ArrayPrototypePush(promises,PromiseResolve(promise));
9821049
}
9831050

1051+
ArrayPrototypePush(promises,stopPromise);
1052+
1053+
// Wait for the race to finish
1054+
awaitSafePromiseRace(promises);
1055+
9841056
this[kShouldAbort]();
9851057
this.plan?.check();
1058+
1059+
if(this.plan!==null){
1060+
constplanPromise=this.plan?.check();
1061+
// If the plan returns a promise, it means that it is waiting for more assertions to be made before
1062+
// continuing.
1063+
if(planPromise){
1064+
awaitSafePromiseRace([planPromise,stopPromise]);
1065+
}
1066+
}
1067+
9861068
this.pass();
9871069
awaitafterEach();
9881070
awaitafter();
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
'use strict';
2+
const{ describe, it }=require('node:test');
3+
4+
describe('planning with wait',()=>{
5+
it('planning with wait and passing',async(t)=>{
6+
t.plan(1,{wait: 5000});
7+
8+
constasyncActivity=()=>{
9+
setTimeout(()=>{
10+
t.assert.ok(true);
11+
},250);
12+
};
13+
14+
asyncActivity();
15+
});
16+
17+
it('planning with wait and failing',async(t)=>{
18+
t.plan(1,{wait: 5000});
19+
20+
constasyncActivity=()=>{
21+
setTimeout(()=>{
22+
t.assert.ok(false);
23+
},250);
24+
};
25+
26+
asyncActivity();
27+
});
28+
29+
it('planning wait time expires before plan is met',async(t)=>{
30+
t.plan(2,{wait: 500});
31+
32+
constasyncActivity=()=>{
33+
setTimeout(()=>{
34+
t.assert.ok(true);
35+
},50_000_000);
36+
};
37+
38+
asyncActivity();
39+
});
40+
41+
it(`planning with wait "options.wait : true" and passing`,async(t)=>{
42+
t.plan(1,{wait: true});
43+
44+
constasyncActivity=()=>{
45+
setTimeout(()=>{
46+
t.assert.ok(true);
47+
},250);
48+
};
49+
50+
asyncActivity();
51+
});
52+
53+
it(`planning with wait "options.wait : true" and failing`,async(t)=>{
54+
t.plan(1,{wait: true});
55+
56+
constasyncActivity=()=>{
57+
setTimeout(()=>{
58+
t.assert.ok(false);
59+
},250);
60+
};
61+
62+
asyncActivity();
63+
});
64+
65+
it(`planning with wait "options.wait : false" should not wait`,async(t)=>{
66+
t.plan(1,{wait: false});
67+
68+
constasyncActivity=()=>{
69+
setTimeout(()=>{
70+
t.assert.ok(true);
71+
},500_000);
72+
};
73+
74+
asyncActivity();
75+
})
76+
});

0 commit comments

Comments
 (0)