test_runner: fix suite diagnostic chanel end

MoLow · aduh95 · commit 16015f156539 · 2026-05-27T11:35:50.000+02:00
Signed-off-by: Moshe Atlow <moshe@atlow.co.il> PR-URL: #63533 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -1812,15 +1812,22 @@ class Suite extends Test {
         publishError(err);
       }
       this.fail(new ERR_TEST_FAILURE(err, kTestCodeFailure));
-    } finally {
-      if (testChannel.end.hasSubscribers) {
-        publishEnd();
-      }
     }
 
+    this.#publishEnd = publishEnd;
     this.buildPhaseFinished = true;
   }
 
+  #publishEnd = null;
+
+  #publishSuiteEnd() {
+    const publishEnd = this.#publishEnd;
+    this.#publishEnd = null;
+    if (publishEnd !== null && testChannel.end.hasSubscribers) {
+      publishEnd();
+    }
+  }
+
   #ctx;
   getCtx() {
     this.#ctx ??= new TestContext(this);
@@ -1872,6 +1879,7 @@ class Suite extends Test {
       }
     } finally {
       stopPromise?.[SymbolDispose]();
+      this.#publishSuiteEnd();
     }
 
     this.postRun();
diff --git a/test/parallel/test-runner-diagnostics-channel.js b/test/parallel/test-runner-diagnostics-channel.js
@@ -9,9 +9,14 @@ const { join } = require('path');
 
 const events = [];
 
-dc.subscribe('tracing:node.test:start', (data) => events.push({ event: 'start', name: data.name }));
-dc.subscribe('tracing:node.test:end', (data) => events.push({ event: 'end', name: data.name }));
-dc.subscribe('tracing:node.test:error', (data) => events.push({ event: 'error', name: data.name }));
+dc.subscribe('tracing:node.test:start', (data) => events.push({ event: 'start', name: data.name, type: data.type }));
+dc.subscribe('tracing:node.test:end', (data) => events.push({ event: 'end', name: data.name, type: data.type }));
+dc.subscribe('tracing:node.test:error', (data) => events.push({ event: 'error', name: data.name, type: data.type }));
+
+describe('suite end ordering', () => {
+  it('child a', async () => { await new Promise((r) => setTimeout(r, 5)); });
+  it('child b', () => {});
+});
 
 test('passing test fires start and end', async () => {});
 
@@ -54,6 +59,19 @@ process.on('exit', () => {
   const asyncEnd = events.filter((e) => e.event === 'end' && e.name === asyncTestName);
   assert.strictEqual(asyncStart.length, 1);
   assert.strictEqual(asyncEnd.length, 1);
+
+  const suiteNames = new Set(['suite end ordering', 'child a', 'child b']);
+  const suiteSequence = events
+    .filter((e) => suiteNames.has(e.name))
+    .map((e) => `${e.event}:${e.name}`);
+  assert.deepStrictEqual(suiteSequence, [
+    'start:suite end ordering',
+    'start:child a',
+    'end:child a',
+    'start:child b',
+    'end:child b',
+    'end:suite end ordering',
+  ]);
 });
 
 // Test bindStore context propagation
