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]>
nodejs · Nov 23, 2023 · 0bd694a · 0bd694a
1 parent 982b576
commit 0bd694a
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 13 deletions.
diff --git 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
@@ -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);
+        });
       });
     });
   });