From 64ef108dd92237a249d6b49a903d75654f3df7b2 Mon Sep 17 00:00:00 2001
From: Colin Ihrig <cjihrig@gmail.com>
Date: Wed, 11 Oct 2023 21:37:00 -0400
Subject: [PATCH] test_runner,test: fix flaky test-runner-cli-concurrency.js

This test was flaky on Windows when trying to clean up the
tmp directory, probably because it relied on child processes
timing out and being killed.

This commit updates the test to check for debug output
from the test runner. This should be adequate because the
original change was effectively:

let concurrency = getOptionValue('--test-concurrency') || true;

The test runner now logs the value of the concurrency variable.

Fixes: https://github.com/nodejs/node/issues/50101
PR-URL: https://github.com/nodejs/node/pull/50108
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
---
 lib/internal/main/test_runner.js             | 14 ++++-
 test/parallel/test-runner-cli-concurrency.js | 55 +++++++-------------
 2 files changed, 30 insertions(+), 39 deletions(-)

diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js
index 9a87a8eac7d41a..7e6f3079a509a9 100644
--- a/lib/internal/main/test_runner.js
+++ b/lib/internal/main/test_runner.js
@@ -18,6 +18,9 @@ const {
   RegExpPrototypeExec,
   StringPrototypeSplit,
 } = primordials;
+let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => {
+  debug = fn;
+});
 
 prepareMainThreadExecution(false);
 markBootstrapComplete();
@@ -57,8 +60,15 @@ if (shardOption) {
   };
 }
 
-run({ concurrency, inspectPort, watch: getOptionValue('--watch'), setup: setupTestReporters, shard })
-.on('test:fail', (data) => {
+const options = {
+  concurrency,
+  inspectPort,
+  watch: getOptionValue('--watch'),
+  setup: setupTestReporters,
+  shard,
+};
+debug('test runner configuration:', options);
+run(options).on('test:fail', (data) => {
   if (data.todo === undefined || data.todo === false) {
     process.exitCode = kGenericUserError;
   }
diff --git a/test/parallel/test-runner-cli-concurrency.js b/test/parallel/test-runner-cli-concurrency.js
index 9d8dd4f8ce2f82..fbabaf08e27279 100644
--- a/test/parallel/test-runner-cli-concurrency.js
+++ b/test/parallel/test-runner-cli-concurrency.js
@@ -1,45 +1,26 @@
 'use strict';
-const common = require('../common');
-const tmpdir = require('../common/tmpdir');
-const { deepStrictEqual, strictEqual } = require('node:assert');
+require('../common');
+const fixtures = require('../common/fixtures');
+const assert = require('node:assert');
 const { spawnSync } = require('node:child_process');
-const { readdirSync, writeFileSync } = require('node:fs');
-const { join } = require('node:path');
-const { beforeEach, test } = require('node:test');
+const { test } = require('node:test');
+const cwd = fixtures.path('test-runner', 'default-behavior');
+const env = { ...process.env, 'NODE_DEBUG': 'test_runner' };
 
-function createTestFile(name) {
-  writeFileSync(join(tmpdir.path, name), `
-    const fs = require('node:fs');
-
-    fs.unlinkSync(__filename);
-    setTimeout(() => {}, 1_000_000_000);
-  `);
-}
-
-beforeEach(() => {
-  tmpdir.refresh();
-  createTestFile('test-1.js');
-  createTestFile('test-2.js');
+test('default concurrency', async () => {
+  const args = ['--test'];
+  const cp = spawnSync(process.execPath, args, { cwd, env });
+  assert.match(cp.stderr.toString(), /concurrency: true,/);
 });
 
-test('concurrency of one', () => {
-  const cp = spawnSync(process.execPath, ['--test', '--test-concurrency=1'], {
-    cwd: tmpdir.path,
-    timeout: common.platformTimeout(1000),
-  });
-
-  strictEqual(cp.stderr.toString(), '');
-  strictEqual(cp.error.code, 'ETIMEDOUT');
-  deepStrictEqual(readdirSync(tmpdir.path), ['test-2.js']);
+test('concurrency of one', async () => {
+  const args = ['--test', '--test-concurrency=1'];
+  const cp = spawnSync(process.execPath, args, { cwd, env });
+  assert.match(cp.stderr.toString(), /concurrency: 1,/);
 });
 
-test('concurrency of two', () => {
-  const cp = spawnSync(process.execPath, ['--test', '--test-concurrency=2'], {
-    cwd: tmpdir.path,
-    timeout: common.platformTimeout(1000),
-  });
-
-  strictEqual(cp.stderr.toString(), '');
-  strictEqual(cp.error.code, 'ETIMEDOUT');
-  deepStrictEqual(readdirSync(tmpdir.path), []);
+test('concurrency of two', async () => {
+  const args = ['--test', '--test-concurrency=2'];
+  const cp = spawnSync(process.execPath, args, { cwd, env });
+  assert.match(cp.stderr.toString(), /concurrency: 2,/);
 });