test_runner: add input validation tests and refactor planning with wait

nodejs · nodejs-github-bot · Feb 23, 2025 · Feb 14, 2025 · Feb 14, 2025 · Feb 14, 2025
commit c942eb9cc1bcb7384dd0de5938cb55ff9ee16d76
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -176,47 +176,104 @@ function testMatchesPattern(test, patterns) {
 }
 
 class TestPlan {
-  #timeoutPromise;
-  #testSignal;
+  #waitIndefinitely = false;
 
   constructor(count, options = kEmptyObject) {
     validateUint32(count, 'count');
+    validateObject(options, 'options');
+
     this.expected = count;
     this.actual = 0;
-    this.timeout = options.timeout;
 
-    if (options.signal) {
-      this.#testSignal = options.signal;
+    switch (typeof options.wait) {
+      case 'boolean':
+        this.wait = options.wait;
+        this.#waitIndefinitely = true;
+        break;
+
+      case 'number':
+        validateNumber(options.wait, 'options.wait', 0, TIMEOUT_MAX);
+        this.wait = options.wait;
+        break;
+
+      default:
+        if (options.wait !== undefined) {
+          throw new ERR_INVALID_ARG_TYPE('options.wait', ['boolean', 'number'], options.wait);
+        }
     }
-    if (this.timeout !== undefined) {
-      validateNumber(this.timeout, 'options.timeout', 0, TIMEOUT_MAX);
-      this.#timeoutPromise = stopTest(
-        this.timeout,
-        this.#testSignal,
-        `plan timed out after ${this.timeout}ms with ${this.actual} assertions when expecting ${this.expected}`,
-      );
+  }
+
+  #planMet() {
+    return this.actual === this.expected;
+  }
+
+  #startPoller() {
+    const { promise, resolve, reject } = PromiseWithResolvers();
+    const noError = Symbol();
+    let pollerId;
+    let timeoutId;
+    const interval = 50;
+
+    const done = (err, result) => {
+      clearTimeout(pollerId);
+      timeoutId ?? clearTimeout(timeoutId);
+
+      if (err === noError) {
+        resolve(result);
+      } else {
+        reject(err);
+      }
+    };
+
+    // If the plan has a maximum wait time, then we need to set a timeout
+    // Otherwise, the plan will wait indefinitely
+    if (!this.#waitIndefinitely) {
+      timeoutId = setTimeout(() => {
+        const err = new ERR_TEST_FAILURE(
+          `plan timed out after ${this.wait}ms with ${this.actual} assertions when expecting ${this.expected}`,
+          kTestTimeoutFailure,
+        );
+          // The pooler has timed out, the test should fail
+        done(err);
+      }, this.wait);
     }
+
+    const poller = async () => {
+      if (this.#planMet()) {
+        done(noError);
+      } else {
+        pollerId = setTimeout(poller, interval);
+      }
+    };
+
+    poller();
+    return promise;
   }
 
   check() {
-    if (this.actual !== this.expected) {
+    if (this.#planMet()) {
+      // If the plan has been met, then there is no need to check for a timeout.
+      return;
+    }
+    // The plan has not been met
+    if (!this.hasTimeout()) {
+      // If the plan doesn't have a timeout, then the test should fail immediately.
       throw new ERR_TEST_FAILURE(
         `plan expected ${this.expected} assertions but received ${this.actual}`,
         kTestCodeFailure,
       );
     }
+    // If the plan has a timeout, then the test is still running
+    // we need to pool until the timeout is reached or the plan is met
+    return this.#startPoller();
   }
 
-  increaseActualCount() {
+  count() {
     this.actual++;
   }
 
-  get timeoutPromise() {
-    return this.#timeoutPromise;
-  }
-
   hasTimeout() {
-    return this.#timeoutPromise !== undefined;
+    return this.wait !== undefined;
   }
 }
 
@@ -256,19 +313,15 @@ class TestContext {
     this.#test.diagnostic(message);
   }
 
-  plan(count, options) {
+  plan(count, options = kEmptyObject) {
     if (this.#test.plan !== null) {
       throw new ERR_TEST_FAILURE(
         'cannot set plan more than once',
         kTestCodeFailure,
       );
     }
 
-    this.#test.plan = new TestPlan(count, {
-      __proto__: null,
-      ...options,
-      signal: this.#test.signal,
-    });
+    this.#test.plan = new TestPlan(count, options);
   }
 
   get assert() {
@@ -281,7 +334,7 @@ class TestContext {
       map.forEach((method, name) => {
         assert[name] = (...args) => {
           if (plan !== null) {
-            plan.increaseActualCount();
+            plan.count();
           }
           return ReflectApply(method, this, args);
         };
@@ -292,7 +345,7 @@ class TestContext {
         // stacktrace from the correct starting point.
         function ok(...args) {
           if (plan !== null) {
-            plan.increaseActualCount();
+            plan.count();
           }
           innerOk(ok, args.length, ...args);
         }
@@ -328,7 +381,7 @@ class TestContext {
 
     const { plan } = this.#test;
     if (plan !== null) {
-      plan.increaseActualCount();
+      plan.count();
     }
 
     const subtest = this.#test.createSubtest(
@@ -1024,9 +1077,6 @@ class Test extends AsyncResource {
       }
 
       ArrayPrototypePush(promises, stopPromise);
-      if (this.plan?.hasTimeout()) {
-        ArrayPrototypePush(promises, this.plan.timeoutPromise);
-      }
 
       // Wait for the race to finish
       await SafePromiseRace(promises);
@@ -1037,7 +1087,15 @@ class Test extends AsyncResource {
         await SafePromiseRace([this.subtestsPromise.promise, stopPromise]);
       }
 
-      this.plan?.check();
+      if (this.plan !== null) {
+        const checkPollPromise = this.plan?.check();
+        // If the plan returns a promise, then the plan is polling and we need to wait for it to finish
+        // or the test to stop
+        if (checkPollPromise) {
+          await SafePromiseRace([checkPollPromise, stopPromise]);
+        }
+      }
+
       this.pass();
       await afterEach();
       await after();
@@ -1055,7 +1113,6 @@ class Test extends AsyncResource {
       try { await after(); } catch { /* Ignore error. */ }
     } finally {
       stopPromise?.[SymbolDispose]();
-      this.plan?.timeoutPromise?.[SymbolDispose]();
 
       // Do not abort hooks and the root test as hooks instance are shared between tests suite so aborting them will
       // cause them to not run for further tests.

diff --git a/test/fixtures/test-runner/output/test-runner-plan-timeout.js b/test/fixtures/test-runner/output/test-runner-plan-timeout.js
@@ -1,34 +1,65 @@
 'use strict';
 const { describe, it } = require('node:test');
-const { setTimeout } = require('node:timers/promises');
+const { platformTimeout } = require('../../../common');
 
-describe('planning with timeout', () => {
-  it(`planning should pass if plan it's correct`, async (t) => {
-    t.plan(1, { timeout: 500_000_000 });
-    t.assert.ok(true);
+describe('planning with wait', () => {
+  it('planning with wait and passing', async (t) => {
+    t.plan(1, { wait: platformTimeout(5000) });
+
+    const asyncActivity = () => {
+      setTimeout(() => {
+        t.assert.ok(true);
+      }, platformTimeout(250));
+    };
+
+    asyncActivity();
   });
-
-  it(`planning should fail if plan it's incorrect`, async (t) => {
-    t.plan(1, { timeout: 500_000_000 });
-    t.assert.ok(true);
-    t.assert.ok(true);
+
+  it('planning with wait and failing', async (t) => {
+    t.plan(1, { wait: platformTimeout(5000) });
+
+    const asyncActivity = () => {
+      setTimeout(() => {
+        t.assert.ok(false);
+      }, platformTimeout(250));
+    };
+
+    asyncActivity();
+  });
+
+  it('planning wait time expires before plan is met', async (t) => {
+    t.plan(2, { wait: platformTimeout(500) });
+
+    const asyncActivity = () => {
+      setTimeout(() => {
+        t.assert.ok(true);
+      }, platformTimeout(50_000_000));
+    };
+
+    asyncActivity();
   });
-
-  it('planning with timeout', async (t) => {
-    t.plan(1, { timeout: 2000 });
-
-    while (true) {
-      await setTimeout(5000);
-    }
+
+  it(`planning with wait "options.wait : true" and passing`, async (t) => {
+    t.plan(1, { wait: true });
+
+    const asyncActivity = () => {
+      setTimeout(() => {
+        t.assert.ok(true);
+      }, platformTimeout(250));
+    };
+
+    asyncActivity();
   });
 
-  it('nested planning with timeout', async (t) => {
-    t.plan(1, { timeout: 2000 });
-
-    t.test('nested', async (t) => {
-      while (true) {
-        await setTimeout(5000);
-      }
-    });
+  it(`planning with wait "options.wait : true" and failing`, async (t) => {
+    t.plan(1, { wait: true });
+
+    const asyncActivity = () => {
+      setTimeout(() => {
+        t.assert.ok(false);
+      }, platformTimeout(250));
+    };
+
+    asyncActivity();
   });
 });