Repository: avajs/ava (js)

Issue:
Override `process.exit` to give some context when it's called unexpectedly in a test.
<!-- Issuehunt Badges -->

[<img alt="Issuehunt badges" src="https://issuehunt.io/r/avajs/ava/issues/861/badge.svg" />](https://issuehunt.io/r/avajs/ava/issues/861)

<!-- /Issuehunt Badges -->

Just had an issue on Gitter where a users code was causing `process.exit(1)` to be called. He got the "test.js exited with a non-zero exit code message", but there was no helpful information.

I say we try and capture that somehow:

``` js
var oldExit = process.exit;
process.exit = function (code) {
  if (code > 0) {
    var stack = new Error().stack;
    // log it? send it to parent?
    // ...
  }
  oldExit.apply(process, arguments);
};
```
<!-- Issuehunt content -->



> There is a $60.00 open bounty on this issue. Add more on [Issuehunt](https://issuehunt.io/r/avajs/ava/issues/861).
> - Checkout the [Issuehunt explorer](https://issuehunt.io/r/avajs/ava/) to discover more funded issues.
> - Need some help from other developers? [Add your repositories](https://issuehunt.io/r/new) on Issuehunt to raise funds.


<!-- /Issuehunt content-->

Pull request description:
Detect process.exit() called from tests
Fixes #861

Patch:
diff --git a/lib/api.js b/lib/api.js
index 1ebe27ce..dcab0fab 100644
--- a/lib/api.js
+++ b/lib/api.js
@@ -246,14 +246,16 @@ export default class Api extends Emittery {
 				}
 			}));
 
-			// Resolve the correct concurrency value.
-			let concurrency = Math.min(os.cpus().length, isCi ? 2 : Number.POSITIVE_INFINITY);
-			if (apiOptions.concurrency > 0) {
-				concurrency = apiOptions.concurrency;
-			}
-
+			// Resolve the correct concurrency value. Note that `os.cpus()` can return empty arrays on
+			// platforms not officially supported by Node.js. Use 1 as a minimum.
+			// See <https://github.com/nodejs/node/issues/38190>.
+			let concurrency = Math.max(1, os.cpus().length);
 			if (apiOptions.serial) {
 				concurrency = 1;
+			} else if (apiOptions.concurrency > 0) {
+				concurrency = apiOptions.concurrency;
+			} else if (isCi) {
+				concurrency = 2;
 			}
 
 			const deregisteredSharedWorkers = [];
diff --git a/lib/reporters/default.js b/lib/reporters/default.js
index 8a612dee..61430b0e 100644
--- a/lib/reporters/default.js
+++ b/lib/reporters/default.js
@@ -233,6 +233,16 @@ export default class Reporter {
 				break;
 			}
 
+			case 'process-exit': {
+				this.write(colors.error(`${figures.cross} Exiting due to process.exit() when running ${this.relativeFile(event.testFile)}`));
+
+				this.lineWriter.writeLine();
+				this.lineWriter.writeLine(colors.errorStack(event.stack));
+				this.lineWriter.writeLine();
+
+				break;
+			}
+
 			case 'hook-finished': {
 				if (event.logs.length > 0) {
 					this.lineWriter.writeLine(`  ${this.prefixTitle(event.testFile, event.title)}`);
diff --git a/lib/reporters/tap.js b/lib/reporters/tap.js
index 51a55cdb..089a7431 100644
--- a/lib/reporters/tap.js
+++ b/lib/reporters/tap.js
@@ -128,6 +128,17 @@ export default class TapReporter {
 		}
 	}
 
+	writeProcessExit(evt) {
+		const error = new Error(`Exiting due to process.exit() when running ${this.relativeFile(evt.testFile)}`);
+		error.stack = evt.stack;
+
+		for (const [testFile, tests] of evt.pendingTests) {
+			for (const title of tests) {
+				this.writeTest({testFile, title, err: error}, {passed: false, todo: false, skip: false});
+			}
+		}
+	}
+
 	writeTimeout(evt) {
 		const error = new Error(`Exited because no new tests completed within the last ${evt.period}ms of inactivity`);
 
@@ -158,6 +169,9 @@ export default class TapReporter {
 				this.filesWithMissingAvaImports.add(evt.testFile);
 				this.writeCrash(evt, `No tests found in ${this.relativeFile(evt.testFile)}, make sure to import "ava" at the top of your test file`);
 				break;
+			case 'process-exit':
+				this.writeProcessExit(evt);
+				break;
 			case 'selected-test':
 				if (evt.skip) {
 					this.writeTest(evt, {passed: true, todo: false, skip: true});
diff --git a/lib/run-status.js b/lib/run-status.js
index 7d6b26c0..6867cb03 100644
--- a/lib/run-status.js
+++ b/lib/run-status.js
@@ -62,6 +62,7 @@ export default class RunStatus extends Emittery {
 		worker.onStateChange(data => this.emitStateChange(data));
 	}
 
+	// eslint-disable-next-line complexity
 	emitStateChange(event) {
 		const {stats} = this;
 		const fileStats = stats.byFile.get(event.testFile);
@@ -134,6 +135,10 @@ export default class RunStatus extends Emittery {
 				event.pendingTests = this.pendingTests;
 				this.pendingTests = new Map();
 				break;
+			case 'process-exit':
+				event.pendingTests = this.pendingTests;
+				this.pendingTests = new Map();
+				break;
 			case 'uncaught-exception':
 				stats.uncaughtExceptions++;
 				fileStats.uncaughtExceptions++;
@@ -175,6 +180,7 @@ export default class RunStatus extends Emittery {
 			|| this.stats.failedHooks > 0
 			|| this.stats.failedTests > 0
 			|| this.stats.failedWorkers > 0
+			|| this.stats.remainingTests > 0
 			|| this.stats.sharedWorkerErrors > 0
 			|| this.stats.timeouts > 0
 			|| this.stats.uncaughtExceptions > 0
diff --git a/lib/watcher.js b/lib/watcher.js
index 58d59e24..00784e31 100644
--- a/lib/watcher.js
+++ b/lib/watcher.js
@@ -294,6 +294,7 @@ export default class Watcher {
 				switch (evt.type) {
 					case 'hook-failed':
 					case 'internal-error':
+					case 'process-exit':
 					case 'test-failed':
 					case 'uncaught-exception':
 					case 'unhandled-rejection':
diff --git a/lib/worker/base.js b/lib/worker/base.js
index a9ed9e4a..bf4302e2 100644
--- a/lib/worker/base.js
+++ b/lib/worker/base.js
@@ -19,6 +19,41 @@ import {flags, refs, sharedWorkerTeardowns} from './state.cjs';
 import {isRunningInThread, isRunningInChildProcess} from './utils.cjs';
 
 const currentlyUnhandled = setUpCurrentlyUnhandled();
+let runner;
+
+// 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) {
+	dependencyTracking.flush();
+	const flushing = channel.flush();
+	if (!forceSync) {
+		await flushing;
+	}
+
+	apply(realExit, process, [code]);
+}
+
+const handleProcessExit = (fn, receiver, args) => {
+	const error = new Error('Unexpected process.exit()');
+	Error.captureStackTrace(error, handleProcessExit);
+	const {stack} = serializeError('', true, error);
+	channel.send({type: 'process-exit', 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);
+};
+
+process.exit = new Proxy(realExit, {
+	apply: handleProcessExit,
+});
 
 const run = async options => {
 	setOptions(options);
@@ -29,16 +64,6 @@ const run = async options => {
 		global.console = Object.assign(global.console, new console.Console({stdout, stderr, colorMode: true}));
 	}
 
-	async function exit(code) {
-		if (!process.exitCode) {
-			process.exitCode = code;
-		}
-
-		dependencyTracking.flush();
-		await channel.flush();
-		process.exit(); // eslint-disable-line unicorn/no-process-exit
-	}
-
 	let checkSelectedByLineNumbers;
 	try {
 		checkSelectedByLineNumbers = lineNumberSelection({
@@ -50,7 +75,7 @@ const run = async options => {
 		checkSelectedByLineNumbers = () => false;
 	}
 
-	const runner = new Runner({
+	runner = new Runner({
 		checkSelectedByLineNumbers,
 		experiments: options.experiments,
 		failFast: options.failFast,


Test patch:
diff --git a/test-tap/fixture/fail-fast/multiple-files/fails.cjs b/test-tap/fixture/fail-fast/multiple-files/fails.cjs
index f405ec69..c43c6c70 100644
--- a/test-tap/fixture/fail-fast/multiple-files/fails.cjs
+++ b/test-tap/fixture/fail-fast/multiple-files/fails.cjs
@@ -1,13 +1,20 @@
 const test = require('../../../../entrypoints/main.cjs');
 
+// Allow some time for all workers to launch.
+const grace = new Promise(resolve => {
+	setTimeout(resolve, 500);
+});
+
 test('first pass', t => {
 	t.pass();
 });
 
-test('second fail', t => {
+test('second fail', async t => {
+	await grace;
 	t.fail();
 });
 
-test('third pass', t => {
+test('third pass', async t => {
+	await grace;
 	t.pass();
 });
diff --git a/test/helpers/exec.js b/test/helpers/exec.js
index 01c1300a..4e4dbd7e 100644
--- a/test/helpers/exec.js
+++ b/test/helpers/exec.js
@@ -69,7 +69,10 @@ export const fixture = async (args, options = {}) => {
 	const stats = {
 		failed: [],
 		failedHooks: [],
+		internalErrors: [],
+		processExits: [],
 		passed: [],
+		selectedTestCount: 0,
 		sharedWorkerErrors: [],
 		skipped: [],
 		todo: [],
@@ -92,7 +95,23 @@ export const fixture = async (args, options = {}) => {
 				break;
 			}
 
+			case 'internal-error': {
+				const {testFile} = statusEvent;
+				const statObject = {file: normalizePath(workingDir, testFile)};
+				errors.set(statObject, statusEvent.err);
+				stats.internalErrors.push(statObject);
+				break;
+			}
+
+			case 'process-exit': {
+				const {testFile} = statusEvent;
+				const statObject = {file: normalizePath(workingDir, testFile)};
+				stats.processExits.push(statObject);
+				break;
+			}
+
 			case 'selected-test': {
+				stats.selectedTestCount++;
 				if (statusEvent.skip) {
 					const {title, testFile} = statusEvent;
 					stats.skipped.push({title, file: normalizePath(workingDir, testFile)});
diff --git a/test/test-process-exit/fixtures/ava.config.js b/test/test-process-exit/fixtures/ava.config.js
new file mode 100644
index 00000000..1669aaef
--- /dev/null
+++ b/test/test-process-exit/fixtures/ava.config.js
@@ -0,0 +1,3 @@
+export default {
+	files: ['*.js'],
+};
diff --git a/test/test-process-exit/fixtures/package.json b/test/test-process-exit/fixtures/package.json
new file mode 100644
index 00000000..bedb411a
--- /dev/null
+++ b/test/test-process-exit/fixtures/package.json
@@ -0,0 +1,3 @@
+{
+	"type": "module"
+}
diff --git a/test/test-process-exit/fixtures/process-exit.js b/test/test-process-exit/fixtures/process-exit.js
new file mode 100644
index 00000000..dd9e77b7
--- /dev/null
+++ b/test/test-process-exit/fixtures/process-exit.js
@@ -0,0 +1,17 @@
+import test from 'ava';
+
+test('good', t => {
+	t.pass();
+});
+
+test('process.exit', async t => {
+	t.pass();
+	await new Promise(resolve => {
+		setImmediate(resolve);
+	});
+	process.exit(0); // eslint-disable-line unicorn/no-process-exit
+});
+
+test('still good', t => {
+	t.pass();
+});
diff --git a/test/test-process-exit/test.js b/test/test-process-exit/test.js
new file mode 100644
index 00000000..46cf741e
--- /dev/null
+++ b/test/test-process-exit/test.js
@@ -0,0 +1,12 @@
+import test from '@ava/test';
+
+import {fixture} from '../helpers/exec.js';
+
+test('process.exit is intercepted', async t => {
+	const result = await t.throwsAsync(fixture(['process-exit.js']));
+	t.true(result.failed);
+	t.like(result, {timedOut: false, isCanceled: false, killed: false});
+	t.is(result.stats.selectedTestCount, 3);
+	t.is(result.stats.passed.length, 2);
+	t.is(result.stats.processExits.length, 1);
+});