From e97eefa53877ea469f1795060a212d6ccfd57171 Mon Sep 17 00:00:00 2001
From: Chemi Atlow <chemi@testim.io>
Date: Tue, 18 Apr 2023 23:59:49 +0300
Subject: [PATCH] test_runner: execute before hook on test

Execute the before hook for Test as well, and execute it on root before
executing any tests.

Fixes: https://github.com/nodejs/node/issues/47518
PR-URL: https://github.com/nodejs/node/pull/47586
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
---
 doc/api/test.md                               | 24 +++++++
 lib/internal/test_runner/test.js              | 14 +++-
 test/fixtures/test-runner/output/hooks.js     | 14 +++-
 .../test-runner/output/hooks.snapshot         | 71 +++++++++++++++----
 4 files changed, 107 insertions(+), 16 deletions(-)

diff --git a/doc/api/test.md b/doc/api/test.md
index 367b9d9dfa841c..0cdf37e42c4cec 100644
--- a/doc/api/test.md
+++ b/doc/api/test.md
@@ -1429,12 +1429,36 @@ Emitted when a test starts.
 
 <!-- YAML
 added: v18.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/47586
+    description: The `before` function was added to TestContext.
 -->
 
 An instance of `TestContext` is passed to each test function in order to
 interact with the test runner. However, the `TestContext` constructor is not
 exposed as part of the API.
 
+### `context.before([fn][, options])`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* `fn` {Function|AsyncFunction} The hook function. The first argument
+  to this function is a [`TestContext`][] object. If the hook uses callbacks,
+  the callback function is passed as the second argument. **Default:** A no-op
+  function.
+* `options` {Object} Configuration options for the hook. The following
+  properties are supported:
+  * `signal` {AbortSignal} Allows aborting an in-progress hook.
+  * `timeout` {number} A number of milliseconds the hook will fail after.
+    If unspecified, subtests inherit this value from their parent.
+    **Default:** `Infinity`.
+
+This function is used to create a hook running before
+subtest of the current test.
+
 ### `context.beforeEach([fn][, options])`
 
 <!-- YAML
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
index 4aeb2ba0ffc25d..0fd74d1b257653 100644
--- a/lib/internal/test_runner/test.js
+++ b/lib/internal/test_runner/test.js
@@ -127,6 +127,10 @@ class TestContext {
     return subtest.start();
   }
 
+  before(fn, options) {
+    this.#test.createHook('before', fn, options);
+  }
+
   after(fn, options) {
     this.#test.createHook('after', fn, options);
   }
@@ -414,6 +418,9 @@ class Test extends AsyncResource {
     validateOneOf(name, 'hook name', kHookNames);
     // eslint-disable-next-line no-use-before-define
     const hook = new TestHook(fn, options);
+    if (name === 'before') {
+      hook.run = runOnce(hook.run);
+    }
     ArrayPrototypePush(this.hooks[name], hook);
     return hook;
   }
@@ -525,6 +532,9 @@ class Test extends AsyncResource {
       if (this.parent?.hooks.beforeEach.length > 0) {
         await this.parent.runHook('beforeEach', { args, ctx });
       }
+      if (this.parent?.hooks.before.length > 0) {
+        await this.parent.runHook('before', this.parent.getRunArgs());
+      }
       const stopPromise = stopTest(this.timeout, this.signal);
       const runArgs = ArrayPrototypeSlice(args);
       ArrayPrototypeUnshift(runArgs, this.fn, ctx);
@@ -561,7 +571,7 @@ class Test extends AsyncResource {
       this.pass();
     } catch (err) {
       try { await after(); } catch { /* Ignore error. */ }
-      try { await afterEach(); } catch { /* test is already failing, let's the error */ }
+      try { await afterEach(); } catch { /* test is already failing, let's ignore the error */ }
       if (isTestFailureError(err)) {
         if (err.failureType === kTestTimeoutFailure) {
           this.#cancel(err);
@@ -793,7 +803,7 @@ class Suite extends Test {
 
       this.pass();
     } catch (err) {
-      try { await afterEach(); } catch { /* test is already failing, let's the error */ }
+      try { await afterEach(); } catch { /* test is already failing, let's ignore the error */ }
       if (isTestFailureError(err)) {
         this.fail(err);
       } else {
diff --git a/test/fixtures/test-runner/output/hooks.js b/test/fixtures/test-runner/output/hooks.js
index 16a58c5a17af57..815d209f15abcd 100644
--- a/test/fixtures/test-runner/output/hooks.js
+++ b/test/fixtures/test-runner/output/hooks.js
@@ -4,6 +4,8 @@ const common = require('../../../common');
 const assert = require('assert');
 const { test, describe, it, before, after, beforeEach, afterEach } = require('node:test');
 
+before((t) => t.diagnostic('before 1 called'));
+
 describe('describe hooks', () => {
   const testArr = [];
   before(function() {
@@ -91,6 +93,7 @@ describe('afterEach throws and test fails', () => {
 test('test hooks', async (t) => {
   const testArr = [];
 
+  t.before((t) => testArr.push('before ' + t.name));
   t.after(common.mustCall((t) => testArr.push('after ' + t.name)));
   t.beforeEach((t) => testArr.push('beforeEach ' + t.name));
   t.afterEach((t) => testArr.push('afterEach ' + t.name));
@@ -105,7 +108,7 @@ test('test hooks', async (t) => {
   });
 
   assert.deepStrictEqual(testArr, [
-    'beforeEach 1', '1', 'afterEach 1',
+    'beforeEach 1', 'before test hooks', '1', 'afterEach 1',
     'beforeEach 2', '2', 'afterEach 2',
     'beforeEach nested',
     'nested beforeEach nested 1', 'nested1', 'nested afterEach nested 1',
@@ -114,6 +117,13 @@ test('test hooks', async (t) => {
   ]);
 });
 
+test('t.before throws', async (t) => {
+  t.after(common.mustCall());
+  t.before(() => { throw new Error('before'); });
+  await t.test('1', () => {});
+  await t.test('2', () => {});
+});
+
 test('t.beforeEach throws', async (t) => {
   t.after(common.mustCall());
   t.beforeEach(() => { throw new Error('beforeEach'); });
@@ -149,3 +159,5 @@ test('t.after() is called if test body throws', (t) => {
   });
   throw new Error('bye');
 });
+
+before((t) => t.diagnostic('before 2 called'));
diff --git a/test/fixtures/test-runner/output/hooks.snapshot b/test/fixtures/test-runner/output/hooks.snapshot
index 50e866f3cdff5d..1007093e352a88 100644
--- a/test/fixtures/test-runner/output/hooks.snapshot
+++ b/test/fixtures/test-runner/output/hooks.snapshot
@@ -67,6 +67,7 @@ not ok 2 - before throws
     *
     *
     *
+    *
   ...
 # Subtest: after throws
     # Subtest: 1
@@ -307,6 +308,53 @@ ok 8 - test hooks
   ---
   duration_ms: *
   ...
+# Subtest: t.before throws
+    # Subtest: 1
+    not ok 1 - 1
+      ---
+      duration_ms: *
+      failureType: 'hookFailed'
+      error: 'failed running before hook'
+      code: 'ERR_TEST_FAILURE'
+      stack: |-
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+      ...
+    # Subtest: 2
+    not ok 2 - 2
+      ---
+      duration_ms: *
+      failureType: 'hookFailed'
+      error: 'failed running before hook'
+      code: 'ERR_TEST_FAILURE'
+      stack: |-
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+        *
+      ...
+    1..2
+not ok 9 - t.before throws
+  ---
+  duration_ms: *
+  failureType: 'subtestsFailed'
+  error: '2 subtests failed'
+  code: 'ERR_TEST_FAILURE'
+  ...
 # Subtest: t.beforeEach throws
     # Subtest: 1
     not ok 1 - 1
@@ -347,7 +395,7 @@ ok 8 - test hooks
         *
       ...
     1..2
-not ok 9 - t.beforeEach throws
+not ok 10 - t.beforeEach throws
   ---
   duration_ms: *
   failureType: 'subtestsFailed'
@@ -394,7 +442,7 @@ not ok 9 - t.beforeEach throws
         *
       ...
     1..2
-not ok 10 - t.afterEach throws
+not ok 11 - t.afterEach throws
   ---
   duration_ms: *
   failureType: 'subtestsFailed'
@@ -419,7 +467,6 @@ not ok 10 - t.afterEach throws
         *
         *
         *
-        *
       ...
     # Subtest: 2
     ok 2 - 2
@@ -427,7 +474,7 @@ not ok 10 - t.afterEach throws
       duration_ms: *
       ...
     1..2
-not ok 11 - afterEach when test fails
+not ok 12 - afterEach when test fails
   ---
   duration_ms: *
   failureType: 'subtestsFailed'
@@ -452,7 +499,6 @@ not ok 11 - afterEach when test fails
         *
         *
         *
-        *
       ...
     # Subtest: 2
     not ok 2 - 2
@@ -474,7 +520,7 @@ not ok 11 - afterEach when test fails
         *
       ...
     1..2
-not ok 12 - afterEach throws and test fails
+not ok 13 - afterEach throws and test fails
   ---
   duration_ms: *
   failureType: 'subtestsFailed'
@@ -482,7 +528,7 @@ not ok 12 - afterEach throws and test fails
   code: 'ERR_TEST_FAILURE'
   ...
 # Subtest: t.after() is called if test body throws
-not ok 13 - t.after() is called if test body throws
+not ok 14 - t.after() is called if test body throws
   ---
   duration_ms: *
   failureType: 'testCodeFailure'
@@ -493,16 +539,15 @@ not ok 13 - t.after() is called if test body throws
     *
     *
     *
-    *
-    *
-    *
   ...
 # - after() called
-1..13
-# tests 35
+1..14
+# before 1 called
+# before 2 called
+# tests 38
 # suites 8
 # pass 14
-# fail 19
+# fail 22
 # cancelled 2
 # skipped 0
 # todo 0