Skip to content

Commit 9830ae4

Browse files
mika-fischerUlisesGascon
authored andcommitted
test_runner: add tests for various mock timer issues
PR-URL: #50384 Fixes: #50365 Fixes: #50381 Fixes: #50382 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 92a153e commit 9830ae4

File tree

2 files changed

+101
-13
lines changed

2 files changed

+101
-13
lines changed

lib/internal/test_runner/mock/mock_timers.js

+16-13
Original file line numberDiff line numberDiff line change
@@ -260,20 +260,23 @@ class MockTimers {
260260

261261
#createTimer(isInterval, callback, delay, ...args) {
262262
const timerId = this.#currentTimer++;
263-
this.#executionQueue.insert({
263+
const timer = {
264264
__proto__: null,
265265
id: timerId,
266266
callback,
267267
runAt: this.#now + delay,
268-
interval: isInterval,
268+
interval: isInterval ? delay : undefined,
269269
args,
270-
});
271-
272-
return timerId;
270+
};
271+
this.#executionQueue.insert(timer);
272+
return timer;
273273
}
274274

275-
#clearTimer(position) {
276-
this.#executionQueue.removeAt(position);
275+
#clearTimer(timer) {
276+
if (timer.priorityQueuePosition !== undefined) {
277+
this.#executionQueue.removeAt(timer.priorityQueuePosition);
278+
timer.priorityQueuePosition = undefined;
279+
}
277280
}
278281

279282
#createDate() {
@@ -397,10 +400,10 @@ class MockTimers {
397400
emitter.emit('data', result);
398401
};
399402

400-
const timerId = this.#createTimer(true, callback, interval, options);
403+
const timer = this.#createTimer(true, callback, interval, options);
401404
const clearListeners = () => {
402405
emitter.removeAllListeners();
403-
context.#clearTimer(timerId);
406+
context.#clearTimer(timer);
404407
};
405408
const iterator = {
406409
__proto__: null,
@@ -453,11 +456,11 @@ class MockTimers {
453456
}
454457

455458
const onabort = () => {
456-
clearFn(id);
459+
clearFn(timer);
457460
return reject(abortIt(options.signal));
458461
};
459462

460-
const id = timerFn(() => {
463+
const timer = timerFn(() => {
461464
return resolve(result);
462465
}, ms);
463466

@@ -620,11 +623,11 @@ class MockTimers {
620623
FunctionPrototypeApply(timer.callback, undefined, timer.args);
621624

622625
this.#executionQueue.shift();
626+
timer.priorityQueuePosition = undefined;
623627

624-
if (timer.interval) {
628+
if (timer.interval !== undefined) {
625629
timer.runAt += timer.interval;
626630
this.#executionQueue.insert(timer);
627-
return;
628631
}
629632

630633
timer = this.#executionQueue.peek();

test/parallel/test-runner-mock-timers.js

+85
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,59 @@ describe('Mock Timers Test Suite', () => {
479479
code: 'ERR_INVALID_ARG_TYPE',
480480
});
481481
});
482+
483+
// Test for https://github.com/nodejs/node/issues/50365
484+
it('should not affect other timers when aborting', async (t) => {
485+
const f1 = t.mock.fn();
486+
const f2 = t.mock.fn();
487+
t.mock.timers.enable({ apis: ['setTimeout'] });
488+
const ac = new AbortController();
489+
490+
// id 1 & pos 1 in priority queue
491+
nodeTimersPromises.setTimeout(100, undefined, { signal: ac.signal }).then(f1, f1);
492+
// id 2 & pos 1 in priority queue (id 1 is moved to pos 2)
493+
nodeTimersPromises.setTimeout(50).then(f2, f2);
494+
495+
ac.abort(); // BUG: will remove timer at pos 1 not timer with id 1!
496+
497+
t.mock.timers.runAll();
498+
await nodeTimersPromises.setImmediate(); // let promises settle
499+
500+
// First setTimeout is aborted
501+
assert.strictEqual(f1.mock.callCount(), 1);
502+
assert.strictEqual(f1.mock.calls[0].arguments[0].code, 'ABORT_ERR');
503+
504+
// Second setTimeout should resolve, but never settles, because it was eronously removed by ac.abort()
505+
assert.strictEqual(f2.mock.callCount(), 1);
506+
});
507+
508+
// Test for https://github.com/nodejs/node/issues/50365
509+
it('should not affect other timers when aborted after triggering', async (t) => {
510+
const f1 = t.mock.fn();
511+
const f2 = t.mock.fn();
512+
t.mock.timers.enable({ apis: ['setTimeout'] });
513+
const ac = new AbortController();
514+
515+
// id 1 & pos 1 in priority queue
516+
nodeTimersPromises.setTimeout(50, true, { signal: ac.signal }).then(f1, f1);
517+
// id 2 & pos 2 in priority queue
518+
nodeTimersPromises.setTimeout(100).then(f2, f2);
519+
520+
// First setTimeout resolves
521+
t.mock.timers.tick(50);
522+
await nodeTimersPromises.setImmediate(); // let promises settle
523+
assert.strictEqual(f1.mock.callCount(), 1);
524+
assert.strictEqual(f1.mock.calls[0].arguments.length, 1);
525+
assert.strictEqual(f1.mock.calls[0].arguments[0], true);
526+
527+
// Now timer with id 2 will be at pos 1 in priority queue
528+
ac.abort(); // BUG: will remove timer at pos 1 not timer with id 1!
529+
530+
// Second setTimeout should resolve, but never settles, because it was eronously removed by ac.abort()
531+
t.mock.timers.runAll();
532+
await nodeTimersPromises.setImmediate(); // let promises settle
533+
assert.strictEqual(f2.mock.callCount(), 1);
534+
});
482535
});
483536

484537
describe('setInterval Suite', () => {
@@ -626,6 +679,38 @@ describe('Mock Timers Test Suite', () => {
626679
});
627680
assert.strictEqual(numIterations, expectedIterations);
628681
});
682+
683+
// Test for https://github.com/nodejs/node/issues/50381
684+
it('should use the mocked interval', (t) => {
685+
t.mock.timers.enable({ apis: ['setInterval'] });
686+
const fn = t.mock.fn();
687+
setInterval(fn, 1000);
688+
assert.strictEqual(fn.mock.callCount(), 0);
689+
t.mock.timers.tick(1000);
690+
assert.strictEqual(fn.mock.callCount(), 1);
691+
t.mock.timers.tick(1);
692+
t.mock.timers.tick(1);
693+
t.mock.timers.tick(1);
694+
assert.strictEqual(fn.mock.callCount(), 1);
695+
});
696+
697+
// Test for https://github.com/nodejs/node/issues/50382
698+
it('should not prevent due timers to be processed', async (t) => {
699+
t.mock.timers.enable({ apis: ['setInterval', 'setTimeout'] });
700+
const f1 = t.mock.fn();
701+
const f2 = t.mock.fn();
702+
703+
setInterval(f1, 1000);
704+
setTimeout(f2, 1001);
705+
706+
assert.strictEqual(f1.mock.callCount(), 0);
707+
assert.strictEqual(f2.mock.callCount(), 0);
708+
709+
t.mock.timers.tick(1001);
710+
711+
assert.strictEqual(f1.mock.callCount(), 1);
712+
assert.strictEqual(f2.mock.callCount(), 1);
713+
});
629714
});
630715
});
631716
});

0 commit comments

Comments
 (0)