From 9830ae4bf729e0a5113dda1de0d29ff7305b1abc Mon Sep 17 00:00:00 2001 From: Mika Fischer Date: Sun, 12 Nov 2023 09:26:28 +0100 Subject: [PATCH] test_runner: add tests for various mock timer issues PR-URL: https://github.com/nodejs/node/pull/50384 Fixes: https://github.com/nodejs/node/issues/50365 Fixes: https://github.com/nodejs/node/issues/50381 Fixes: https://github.com/nodejs/node/issues/50382 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Chemi Atlow Reviewed-By: James M Snell --- lib/internal/test_runner/mock/mock_timers.js | 29 ++++--- test/parallel/test-runner-mock-timers.js | 85 ++++++++++++++++++++ 2 files changed, 101 insertions(+), 13 deletions(-) diff --git a/lib/internal/test_runner/mock/mock_timers.js b/lib/internal/test_runner/mock/mock_timers.js index 8fac645e9a3617..85512cc4b66a51 100644 --- a/lib/internal/test_runner/mock/mock_timers.js +++ b/lib/internal/test_runner/mock/mock_timers.js @@ -260,20 +260,23 @@ class MockTimers { #createTimer(isInterval, callback, delay, ...args) { const timerId = this.#currentTimer++; - this.#executionQueue.insert({ + const timer = { __proto__: null, id: timerId, callback, runAt: this.#now + delay, - interval: isInterval, + interval: isInterval ? delay : undefined, args, - }); - - return timerId; + }; + this.#executionQueue.insert(timer); + return timer; } - #clearTimer(position) { - this.#executionQueue.removeAt(position); + #clearTimer(timer) { + if (timer.priorityQueuePosition !== undefined) { + this.#executionQueue.removeAt(timer.priorityQueuePosition); + timer.priorityQueuePosition = undefined; + } } #createDate() { @@ -397,10 +400,10 @@ class MockTimers { emitter.emit('data', result); }; - const timerId = this.#createTimer(true, callback, interval, options); + const timer = this.#createTimer(true, callback, interval, options); const clearListeners = () => { emitter.removeAllListeners(); - context.#clearTimer(timerId); + context.#clearTimer(timer); }; const iterator = { __proto__: null, @@ -453,11 +456,11 @@ class MockTimers { } const onabort = () => { - clearFn(id); + clearFn(timer); return reject(abortIt(options.signal)); }; - const id = timerFn(() => { + const timer = timerFn(() => { return resolve(result); }, ms); @@ -620,11 +623,11 @@ class MockTimers { FunctionPrototypeApply(timer.callback, undefined, timer.args); this.#executionQueue.shift(); + timer.priorityQueuePosition = undefined; - if (timer.interval) { + if (timer.interval !== undefined) { timer.runAt += timer.interval; this.#executionQueue.insert(timer); - return; } timer = this.#executionQueue.peek(); diff --git a/test/parallel/test-runner-mock-timers.js b/test/parallel/test-runner-mock-timers.js index fae520b94cdf67..eb5cec84e122f2 100644 --- a/test/parallel/test-runner-mock-timers.js +++ b/test/parallel/test-runner-mock-timers.js @@ -479,6 +479,59 @@ describe('Mock Timers Test Suite', () => { code: 'ERR_INVALID_ARG_TYPE', }); }); + + // Test for https://github.com/nodejs/node/issues/50365 + it('should not affect other timers when aborting', async (t) => { + const f1 = t.mock.fn(); + const f2 = t.mock.fn(); + t.mock.timers.enable({ apis: ['setTimeout'] }); + const ac = new AbortController(); + + // id 1 & pos 1 in priority queue + nodeTimersPromises.setTimeout(100, undefined, { signal: ac.signal }).then(f1, f1); + // id 2 & pos 1 in priority queue (id 1 is moved to pos 2) + nodeTimersPromises.setTimeout(50).then(f2, f2); + + ac.abort(); // BUG: will remove timer at pos 1 not timer with id 1! + + t.mock.timers.runAll(); + await nodeTimersPromises.setImmediate(); // let promises settle + + // First setTimeout is aborted + assert.strictEqual(f1.mock.callCount(), 1); + assert.strictEqual(f1.mock.calls[0].arguments[0].code, 'ABORT_ERR'); + + // Second setTimeout should resolve, but never settles, because it was eronously removed by ac.abort() + assert.strictEqual(f2.mock.callCount(), 1); + }); + + // Test for https://github.com/nodejs/node/issues/50365 + it('should not affect other timers when aborted after triggering', async (t) => { + const f1 = t.mock.fn(); + const f2 = t.mock.fn(); + t.mock.timers.enable({ apis: ['setTimeout'] }); + const ac = new AbortController(); + + // id 1 & pos 1 in priority queue + nodeTimersPromises.setTimeout(50, true, { signal: ac.signal }).then(f1, f1); + // id 2 & pos 2 in priority queue + nodeTimersPromises.setTimeout(100).then(f2, f2); + + // First setTimeout resolves + t.mock.timers.tick(50); + await nodeTimersPromises.setImmediate(); // let promises settle + assert.strictEqual(f1.mock.callCount(), 1); + assert.strictEqual(f1.mock.calls[0].arguments.length, 1); + assert.strictEqual(f1.mock.calls[0].arguments[0], true); + + // Now timer with id 2 will be at pos 1 in priority queue + ac.abort(); // BUG: will remove timer at pos 1 not timer with id 1! + + // Second setTimeout should resolve, but never settles, because it was eronously removed by ac.abort() + t.mock.timers.runAll(); + await nodeTimersPromises.setImmediate(); // let promises settle + assert.strictEqual(f2.mock.callCount(), 1); + }); }); describe('setInterval Suite', () => { @@ -626,6 +679,38 @@ describe('Mock Timers Test Suite', () => { }); assert.strictEqual(numIterations, expectedIterations); }); + + // Test for https://github.com/nodejs/node/issues/50381 + it('should use the mocked interval', (t) => { + t.mock.timers.enable({ apis: ['setInterval'] }); + const fn = t.mock.fn(); + setInterval(fn, 1000); + assert.strictEqual(fn.mock.callCount(), 0); + t.mock.timers.tick(1000); + assert.strictEqual(fn.mock.callCount(), 1); + t.mock.timers.tick(1); + t.mock.timers.tick(1); + t.mock.timers.tick(1); + assert.strictEqual(fn.mock.callCount(), 1); + }); + + // Test for https://github.com/nodejs/node/issues/50382 + it('should not prevent due timers to be processed', async (t) => { + t.mock.timers.enable({ apis: ['setInterval', 'setTimeout'] }); + const f1 = t.mock.fn(); + const f2 = t.mock.fn(); + + setInterval(f1, 1000); + setTimeout(f2, 1001); + + assert.strictEqual(f1.mock.callCount(), 0); + assert.strictEqual(f2.mock.callCount(), 0); + + t.mock.timers.tick(1001); + + assert.strictEqual(f1.mock.callCount(), 1); + assert.strictEqual(f2.mock.callCount(), 1); + }); }); }); });