Don't force-exit after tests have completed

Fixes #1718. With worker threads, this seems to stop AVA from detecting that the thread has exited, causing a hang. Also remove flush logic implemented in #1722. Let's hope that current Node.js versions are better at flushing IPC before exiting.
avajs · novemberborn · Dec 4, 2023 · Nov 12, 2023 · Nov 26, 2023 · Nov 26, 2023
commit e78d65c1f1d3244ad1a8a998b989dd826aab93f9
diff --git a/lib/fork.js b/lib/fork.js
@@ -122,11 +122,6 @@ export default function loadFork(file, options, execArgv = process.execArgv) {
 					break;
 				}
 
-				case 'ping': {
-					send({type: 'pong'});
-					break;
-				}
-
 				default: {
 					emitStateChange(message.ava);
 				}

diff --git a/lib/reporters/default.js b/lib/reporters/default.js
@@ -240,6 +240,12 @@ export default class Reporter {
 				break;
 			}
 
+			case 'exit-failed': {
+				this.write(colors.error(`${figures.cross} Failed to exit when running ${this.relativeFile(event.testFile)}`));
+
+				break;
+			}
+
 			case 'hook-finished': {
 				if (event.logs.length > 0) {
 					this.lineWriter.writeLine(`  ${this.prefixTitle(event.testFile, event.title)}`);

diff --git a/lib/worker/base.js b/lib/worker/base.js
@@ -23,35 +23,42 @@ import {isRunningInThread, isRunningInChildProcess} from './utils.cjs';
 const currentlyUnhandled = setUpCurrentlyUnhandled();
 let runner;
 
+let failedToExit = false;
+
+const expectExit = code => {
+	// Set the exit code, which is just handy.
+	process.exitCode = code;
+
+	// Note that this timer is unreferenced. It should not keep the event loop
+	// open. However if after a second the process is still running, assume
+	// something is keeping the event loop busy. Exit forcibly but try to convey
+	// the reason to the main process.
+	nowAndTimers.setTimeout(() => {
+		failedToExit = true;
+		channel.send({type: 'exit-failed', code});
+		process.exit(1);
+	}, 1000).unref();
+};
+
 // Override process.exit with an undetectable replacement
 // to report when it is called from a test (which it should never be).
-const {apply} = Reflect;
-const realExit = process.exit;
-
-async function exit(code, forceSync = false) {
-	const flushing = channel.flush();
-	if (!forceSync) {
-		await flushing;
+const handleProcessExit = (target, thisArg, args) => {
+	if (!failedToExit) {
+		const error = new Error('Unexpected process.exit()');
+		Error.captureStackTrace(error, handleProcessExit);
+		channel.send({type: 'process-exit', stack: error.stack});
 	}
 
-	apply(realExit, process, [code]);
-}
-
-const handleProcessExit = (fn, receiver, args) => {
-	const error = new Error('Unexpected process.exit()');
-	Error.captureStackTrace(error, handleProcessExit);
-	channel.send({type: 'process-exit', stack: error.stack});
-
 	// Make sure to extract the code only from `args` rather than e.g. `Array.prototype`.
 	// This level of paranoia is usually unwarranted, but we're dealing with test code
 	// that has already colored outside the lines.
 	const code = args.length > 0 ? args[0] : undefined;
 
 	// Force a synchronous exit as guaranteed by the real process.exit().
-	exit(code, true);
+	target.call(thisArg, code);
 };
 
-process.exit = new Proxy(realExit, {
+process.exit = new Proxy(process.exit, {
 	apply: handleProcessExit,
 });
 
@@ -101,7 +108,7 @@ const run = async options => {
 
 	runner.on('error', error => {
 		channel.send({type: 'internal-error', err: serializeError(error)});
-		exit(1);
+		expectExit(1);
 	});
 
 	runner.on('finish', async () => {
@@ -112,30 +119,32 @@ const run = async options => {
 			}
 		} catch (error) {
 			channel.send({type: 'internal-error', err: serializeError(error)});
-			exit(1);
+			expectExit(1);
 			return;
 		}
 
 		try {
 			await Promise.all(sharedWorkerTeardowns.map(fn => fn()));
 		} catch (error) {
 			channel.send({type: 'uncaught-exception', err: serializeError(error)});
-			exit(1);
+			expectExit(1);
 			return;
 		}
 
 		nowAndTimers.setImmediate(() => {
+			let exitCode = 0;
 			for (const rejection of currentlyUnhandled()) {
 				channel.send({type: 'unhandled-rejection', err: serializeError(rejection.reason, {testFile: options.file})});
+				exitCode = 1;
 			}
 
-			exit(0);
+			expectExit(exitCode);
 		});
 	});
 
 	process.on('uncaughtException', error => {
 		channel.send({type: 'uncaught-exception', err: serializeError(error, {testFile: options.file})});
-		exit(1);
+		expectExit(1);
 	});
 
 	// Store value to prevent required modules from modifying it.
@@ -248,11 +257,11 @@ const run = async options => {
 			channel.unref();
 		} else {
 			channel.send({type: 'missing-ava-import'});
-			exit(1);
+			expectExit(1);
 		}
 	} catch (error) {
 		channel.send({type: 'uncaught-exception', err: serializeError(error, {testFile: options.file})});
-		exit(1);
+		expectExit(1);
 	}
 };
 

diff --git a/lib/worker/channel.cjs b/lib/worker/channel.cjs
@@ -110,22 +110,6 @@ exports.peerFailed = selectAvaMessage(handle.channel, 'peer-failed');
 exports.send = handle.send.bind(handle);
 exports.unref = handle.unref.bind(handle);
 
-let pendingPings = Promise.resolve();
-async function flush() {
-	handle.ref();
-	const promise = pendingPings.then(async () => {
-		handle.send({type: 'ping'});
-		await selectAvaMessage(handle.channel, 'pong');
-		if (promise === pendingPings) {
-			handle.unref();
-		}
-	});
-	pendingPings = promise;
-	await promise;
-}
-
-exports.flush = flush;
-
 let channelCounter = 0;
 let messageCounter = 0;