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

File tree

7 files changed

+377
-30
lines changed

7 files changed

+377
-30
lines changed

doc/api/test.md

+35-1
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+
const asyncActivity = () => {
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

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

178178
class TestPlan {
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 (typeof wait === 'boolean') {
191+
this.wait = wait;
192+
this.#waitIndefinitely = wait;
193+
} else if (typeof wait === 'number') {
194+
validateNumber(wait, 'options.wait', 0, TIMEOUT_MAX);
195+
this.wait = wait;
196+
} else if (wait !== undefined) {
197+
throw new ERR_INVALID_ARG_TYPE('options.wait', ['boolean', 'number'], wait);
198+
}
199+
}
200+
201+
#planMet() {
202+
return this.actual === this.expected;
203+
}
204+
205+
#createTimeout(reject) {
206+
return setTimeout(() => {
207+
const err = new ERR_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
throw new ERR_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+
return this.#planPromise.promise;
246+
}
247+
248+
count() {
249+
this.actual++;
250+
if (this.#planPromise) {
251+
this.check();
252+
}
253+
}
254+
255+
#shouldWait() {
256+
return this.wait !== undefined && this.wait !== false;
192257
}
193258
}
194259

260+
195261
class TestContext {
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
throw new ERR_TEST_FAILURE(
234300
'cannot set plan more than once',
235301
kTestCodeFailure,
236302
);
237303
}
238304

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

242308
get assert() {
@@ -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
return ReflectApply(method, this, args);
255321
};
@@ -260,7 +326,7 @@ class TestContext {
260326
// stacktrace from the correct starting point.
261327
function ok(...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
const subtest = this.#test.createSubtest(
@@ -959,30 +1025,46 @@ class Test extends AsyncResource {
9591025
const runArgs = ArrayPrototypeSlice(args);
9601026
ArrayPrototypeUnshift(runArgs, this.fn, ctx);
9611027

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

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

1051+
ArrayPrototypePush(promises, stopPromise);
1052+
1053+
// Wait for the race to finish
1054+
await SafePromiseRace(promises);
1055+
9841056
this[kShouldAbort]();
9851057
this.plan?.check();
1058+
1059+
if (this.plan !== null) {
1060+
const planPromise = 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+
await SafePromiseRace([planPromise, stopPromise]);
1065+
}
1066+
}
1067+
9861068
this.pass();
9871069
await afterEach();
9881070
await after();
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+
const asyncActivity = () => {
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+
const asyncActivity = () => {
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+
const asyncActivity = () => {
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+
const asyncActivity = () => {
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+
const asyncActivity = () => {
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+
const asyncActivity = () => {
69+
setTimeout(() => {
70+
t.assert.ok(true);
71+
}, 500_000);
72+
};
73+
74+
asyncActivity();
75+
})
76+
});

0 commit comments

Comments
 (0)