Repository: nodejs/node
Pull request: 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

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: lib/internal/test_runner/test.js
patch: @@ -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();
     }
   }
 

filename: lib/internal/test_runner/utils.js
patch: @@ -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 = {

filename: test/parallel/test-runner-force-exit-flush.js
patch: @@ -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/);
+});