From 00dc6d9d97dcba61aa52ae2eb315ec8e2a81e1f8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Drobniak?= <michdrob@gmail.com>
Date: Fri, 1 Mar 2024 13:58:11 +0100
Subject: [PATCH] test_runner: improve `--test-name-pattern` to allow matching
 single test

Try to match a test by name prefixed with all its ancestors
to ensure uniqueness of the name

Fixes: https://github.com/nodejs/node/issues/46728
PR-URL: https://github.com/nodejs/node/pull/51577
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
---
 doc/api/test.md                               | 17 +++++++
 lib/internal/test_runner/test.js              | 26 ++++++++++-
 .../test-runner/output/name_pattern.js        | 14 +++++-
 .../test-runner/output/name_pattern.snapshot  | 46 +++++++++++++++++--
 4 files changed, 95 insertions(+), 8 deletions(-)

diff --git a/doc/api/test.md b/doc/api/test.md
index 2657afa09fc79a..b22e4348ea8167 100644
--- a/doc/api/test.md
+++ b/doc/api/test.md
@@ -267,6 +267,23 @@ allows regular expression flags to be used. In the previous example, starting
 Node.js with `--test-name-pattern="/test [4-5]/i"` would match `Test 4` and
 `Test 5` because the pattern is case-insensitive.
 
+To match a single test with a pattern, you can prefix it with all its ancestor
+test names separated by space, to ensure it is unique.
+For example, given the following test file:
+
+```js
+describe('test 1', (t) => {
+  it('some test');
+});
+
+describe('test 2', (t) => {
+  it('some test');
+});
+```
+
+Starting Node.js with `--test-name-pattern="test 1 some test"` would match
+only `some test` in `test 1`.
+
 Test name patterns do not change the set of files that the test runner executes.
 
 ## Extraneous asynchronous activity
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
index 299e9a18b4b81d..e39c61c3390dd7 100644
--- a/lib/internal/test_runner/test.js
+++ b/lib/internal/test_runner/test.js
@@ -14,6 +14,7 @@ const {
   PromisePrototypeThen,
   PromiseResolve,
   SafePromisePrototypeFinally,
+  StringPrototypeTrim,
   ReflectApply,
   RegExpPrototypeExec,
   SafeMap,
@@ -362,8 +363,29 @@ class Test extends AsyncResource {
   }
 
   matchesTestNamePatterns() {
-    return ArrayPrototypeSome(testNamePatterns, (re) => RegExpPrototypeExec(re, this.name) !== null) ||
-      this.parent?.matchesTestNamePatterns();
+    const matchesByNameOrParent = ArrayPrototypeSome(testNamePatterns, (re) =>
+      RegExpPrototypeExec(re, this.name) !== null,
+    ) ||
+    this.parent?.matchesTestNamePatterns();
+
+    if (matchesByNameOrParent) return true;
+
+    const testNameWithAncestors = StringPrototypeTrim(this.getTestNameWithAncestors());
+    if (!testNameWithAncestors) return false;
+
+    return ArrayPrototypeSome(testNamePatterns, (re) => RegExpPrototypeExec(re, testNameWithAncestors) !== null);
+  }
+
+  /**
+   * Returns a name of the test prefixed by name of all its ancestors in ascending order, separated by a space
+   * Ex."grandparent parent test"
+   *
+   * It's needed to match a single test with non-unique name by pattern
+   */
+  getTestNameWithAncestors() {
+    if (!this.parent) return '';
+
+    return `${this.parent.getTestNameWithAncestors()} ${this.name}`;
   }
 
   hasConcurrency() {
diff --git a/test/fixtures/test-runner/output/name_pattern.js b/test/fixtures/test-runner/output/name_pattern.js
index 10e7619b9cfcb9..0057d8572de958 100644
--- a/test/fixtures/test-runner/output/name_pattern.js
+++ b/test/fixtures/test-runner/output/name_pattern.js
@@ -1,4 +1,4 @@
-// Flags: --test-name-pattern=enabled --test-name-pattern=yes --test-name-pattern=/pattern/i
+// Flags: --test-name-pattern=enabled --test-name-pattern=yes --test-name-pattern=/pattern/i --test-name-pattern=/^DescribeForMatchWithAncestors\sNestedDescribeForMatchWithAncestors\sNestedTest$/
 'use strict';
 const common = require('../../../common');
 const {
@@ -65,3 +65,15 @@ describe('no', function() {
     it('yes', () => {});
   });
 });
+
+describe('DescribeForMatchWithAncestors', () => {
+  it('NestedTest', () => common.mustNotCall());
+
+  describe('NestedDescribeForMatchWithAncestors', () => {
+    it('NestedTest', common.mustCall());
+  });
+})
+
+describe('DescribeForMatchWithAncestors', () => {
+  it('NestedTest', () => common.mustNotCall());
+})
diff --git a/test/fixtures/test-runner/output/name_pattern.snapshot b/test/fixtures/test-runner/output/name_pattern.snapshot
index bd53cc1fd2b89e..0326f01d29adc6 100644
--- a/test/fixtures/test-runner/output/name_pattern.snapshot
+++ b/test/fixtures/test-runner/output/name_pattern.snapshot
@@ -171,12 +171,48 @@ ok 15 - no
   duration_ms: *
   type: 'suite'
   ...
-1..15
-# tests 21
-# suites 10
-# pass 13
+# Subtest: DescribeForMatchWithAncestors
+    # Subtest: NestedTest
+    ok 1 - NestedTest # SKIP test name does not match pattern
+      ---
+      duration_ms: *
+      ...
+    # Subtest: NestedDescribeForMatchWithAncestors
+        # Subtest: NestedTest
+        ok 1 - NestedTest
+          ---
+          duration_ms: *
+          ...
+        1..1
+    ok 2 - NestedDescribeForMatchWithAncestors
+      ---
+      duration_ms: *
+      type: 'suite'
+      ...
+    1..2
+ok 16 - DescribeForMatchWithAncestors
+  ---
+  duration_ms: *
+  type: 'suite'
+  ...
+# Subtest: DescribeForMatchWithAncestors
+    # Subtest: NestedTest
+    ok 1 - NestedTest # SKIP test name does not match pattern
+      ---
+      duration_ms: *
+      ...
+    1..1
+ok 17 - DescribeForMatchWithAncestors
+  ---
+  duration_ms: *
+  type: 'suite'
+  ...
+1..17
+# tests 24
+# suites 13
+# pass 14
 # fail 0
 # cancelled 0
-# skipped 8
+# skipped 10
 # todo 0
 # duration_ms *