From e32521a7b9a800728b2ed5337070628043a3d418 Mon Sep 17 00:00:00 2001
From: Colin Ihrig <cjihrig@gmail.com>
Date: Sat, 28 Sep 2024 09:59:06 -0400
Subject: [PATCH] test_runner: close and flush destinations on forced exit

This commit updates the test runner to explicitly close and flush
all destination file streams when the --test-force-exit flag is
used.

Fixes: https://github.com/nodejs/node/issues/54327
PR-URL: https://github.com/nodejs/node/pull/55099
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
---
 lib/internal/test_runner/test.js              | 24 +++++++--
 lib/internal/test_runner/utils.js             |  5 +-
 test/parallel/test-runner-force-exit-flush.js | 49 +++++++++++++++++++
 3 files changed, 74 insertions(+), 4 deletions(-)
 create mode 100644 test/parallel/test-runner-force-exit-flush.js

diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
index 4331079f74e..993c98802ab 100644
--- a/lib/internal/test_runner/test.js
+++ b/lib/internal/test_runner/test.js
@@ -14,12 +14,14 @@ const {
   NumberPrototypeToFixed,
   ObjectDefineProperty,
   ObjectSeal,
+  Promise,
   PromisePrototypeThen,
   PromiseResolve,
   ReflectApply,
   RegExpPrototypeExec,
   SafeMap,
   SafePromiseAll,
+  SafePromiseAllReturnVoid,
   SafePromisePrototypeFinally,
   SafePromiseRace,
   SafeSet,
@@ -46,6 +48,7 @@ const {
   createDeferredCallback,
   countCompletedTest,
   isTestFailureError,
+  reporterScope,
 } = require('internal/test_runner/utils');
 const {
   createDeferredPromise,
@@ -973,10 +976,25 @@ class Test extends AsyncResource {
       // any remaining ref'ed handles, then do that now. It is theoretically
       // possible that a ref'ed handle could asynchronously create more tests,
       // but the user opted into this behavior.
-      this.reporter.once('close', () => {
-        process.exit();
-      });
+      const promises = [];
+
+      for (let i = 0; i < reporterScope.reporters.length; i++) {
+        const { destination } = reporterScope.reporters[i];
+
+        ArrayPrototypePush(promises, new Promise((resolve) => {
+          destination.on('unpipe', () => {
+            if (!destination.closed && typeof destination.close === 'function') {
+              destination.close(resolve);
+            } else {
+              resolve();
+            }
+          });
+        }));
+      }
+
       this.harness.teardown();
+      await SafePromiseAllReturnVoid(promises);
+      process.exit();
     }
   }
 
diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
index 67ce601c1df..35c7279dab3 100644
--- a/lib/internal/test_runner/utils.js
+++ b/lib/internal/test_runner/utils.js
@@ -144,7 +144,8 @@ function shouldColorizeTestFiles(destinations) {
 
 async function getReportersMap(reporters, destinations) {
   return SafePromiseAllReturnArrayLike(reporters, async (name, i) => {
-    const destination = kBuiltinDestinations.get(destinations[i]) ?? createWriteStream(destinations[i]);
+    const destination = kBuiltinDestinations.get(destinations[i]) ??
+      createWriteStream(destinations[i], { __proto__: null, flush: true });
 
     // Load the test reporter passed to --test-reporter
     let reporter = tryBuiltinReporter(name);
@@ -301,6 +302,8 @@ function parseCommandLine() {
       const { reporter, destination } = reportersMap[i];
       compose(rootReporter, reporter).pipe(destination);
     }
+
+    reporterScope.reporters = reportersMap;
   });
 
   globalTestOptions = {
diff --git a/test/parallel/test-runner-force-exit-flush.js b/test/parallel/test-runner-force-exit-flush.js
new file mode 100644
index 00000000000..ddc4ea97719
--- /dev/null
+++ b/test/parallel/test-runner-force-exit-flush.js
@@ -0,0 +1,49 @@
+'use strict';
+require('../common');
+const fixtures = require('../common/fixtures');
+const tmpdir = require('../common/tmpdir');
+const { match, strictEqual } = require('node:assert');
+const { spawnSync } = require('node:child_process');
+const { readFileSync } = require('node:fs');
+const { test } = require('node:test');
+
+function runWithReporter(reporter) {
+  const destination = tmpdir.resolve(`${reporter}.out`);
+  const args = [
+    '--test-force-exit',
+    `--test-reporter=${reporter}`,
+    `--test-reporter-destination=${destination}`,
+    fixtures.path('test-runner', 'reporters.js'),
+  ];
+  const child = spawnSync(process.execPath, args);
+  strictEqual(child.stdout.toString(), '');
+  strictEqual(child.stderr.toString(), '');
+  strictEqual(child.status, 1);
+  return destination;
+}
+
+tmpdir.refresh();
+
+test('junit reporter', () => {
+  const output = readFileSync(runWithReporter('junit'), 'utf8');
+  match(output, /<!-- tests 4 -->/);
+  match(output, /<!-- pass 2 -->/);
+  match(output, /<!-- fail 2 -->/);
+  match(output, /<!-- duration_ms/);
+  match(output, /<\/testsuites>/);
+});
+
+test('spec reporter', () => {
+  const output = readFileSync(runWithReporter('spec'), 'utf8');
+  match(output, /tests 4/);
+  match(output, /pass 2/);
+  match(output, /fail 2/);
+});
+
+test('tap reporter', () => {
+  const output = readFileSync(runWithReporter('tap'), 'utf8');
+  match(output, /# tests 4/);
+  match(output, /# pass 2/);
+  match(output, /# fail 2/);
+  match(output, /# duration_ms/);
+});