Skip to content

Commit

Permalink
Detect process.exit() called from tests
Browse files Browse the repository at this point in the history
Fixes #861.

Co-authored-by: Mark Wubben <mark@novemberborn.net>
  • Loading branch information
gibson042 and novemberborn committed Sep 4, 2022
1 parent 4b03662 commit ea597d8
Show file tree
Hide file tree
Showing 11 changed files with 130 additions and 13 deletions.
10 changes: 10 additions & 0 deletions lib/reporters/default.js
Expand Up @@ -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)}`);
Expand Down
14 changes: 14 additions & 0 deletions lib/reporters/tap.js
Expand Up @@ -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`);

Expand Down Expand Up @@ -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});
Expand Down
6 changes: 6 additions & 0 deletions lib/run-status.js
Expand Up @@ -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);
Expand Down Expand Up @@ -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++;
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/watcher.js
Expand Up @@ -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':
Expand Down
47 changes: 36 additions & 11 deletions lib/worker/base.js
Expand Up @@ -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);
Expand All @@ -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({
Expand All @@ -50,7 +75,7 @@ const run = async options => {
checkSelectedByLineNumbers = () => false;
}

const runner = new Runner({
runner = new Runner({
checkSelectedByLineNumbers,
experiments: options.experiments,
failFast: options.failFast,
Expand Down
11 changes: 9 additions & 2 deletions 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();
});
19 changes: 19 additions & 0 deletions test/helpers/exec.js
Expand Up @@ -69,7 +69,10 @@ export const fixture = async (args, options = {}) => {
const stats = {
failed: [],
failedHooks: [],
internalErrors: [],
processExits: [],
passed: [],
selectedTestCount: 0,
sharedWorkerErrors: [],
skipped: [],
todo: [],
Expand All @@ -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)});
Expand Down
3 changes: 3 additions & 0 deletions test/test-process-exit/fixtures/ava.config.js
@@ -0,0 +1,3 @@
export default {
files: ['*.js'],
};
3 changes: 3 additions & 0 deletions test/test-process-exit/fixtures/package.json
@@ -0,0 +1,3 @@
{
"type": "module"
}
17 changes: 17 additions & 0 deletions 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();
});
12 changes: 12 additions & 0 deletions 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);
});

0 comments on commit ea597d8

Please sign in to comment.