Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't force-exit after tests have completed #3260

Merged
merged 7 commits into from
Dec 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions lib/fork.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,6 @@ export default function loadFork(file, options, execArgv = process.execArgv) {
break;
}

case 'ping': {
send({type: 'pong'});
break;
}

default: {
emitStateChange(message.ava);
}
Expand Down
1 change: 1 addition & 0 deletions lib/reporters/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ export default class Reporter {
writePendingTests(evt) {
for (const [file, testsInFile] of evt.pendingTests) {
if (testsInFile.size === 0) {
this.lineWriter.writeLine(`Failed to exit when running ${this.relativeFile(file)}\n`);
continue;
}

Expand Down
4 changes: 4 additions & 0 deletions lib/run-status.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,10 @@ export default class RunStatus extends Emittery {

case 'worker-finished': {
stats.finishedWorkers++;
if (this.pendingTests.get(event.testFile)?.size === 0) {
this.pendingTests.delete(event.testFile);
}

break;
}

Expand Down
58 changes: 27 additions & 31 deletions lib/worker/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,35 +23,26 @@ import {isRunningInThread, isRunningInChildProcess} from './utils.cjs';
const currentlyUnhandled = setUpCurrentlyUnhandled();
let runner;

let forcingExit = false;

const forceExit = () => {
forcingExit = true;
process.exit(1);
};

// 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 (!forcingExit) {
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.apply(thisArg, args);
};

process.exit = new Proxy(realExit, {
process.exit = new Proxy(process.exit, {
apply: handleProcessExit,
});

Expand Down Expand Up @@ -101,7 +92,7 @@ const run = async options => {

runner.on('error', error => {
channel.send({type: 'internal-error', err: serializeError(error)});
exit(1);
forceExit();
});

runner.on('finish', async () => {
Expand All @@ -112,30 +103,35 @@ const run = async options => {
}
} catch (error) {
channel.send({type: 'internal-error', err: serializeError(error)});
exit(1);
forceExit();
return;
}

try {
await Promise.all(sharedWorkerTeardowns.map(fn => fn()));
} catch (error) {
channel.send({type: 'uncaught-exception', err: serializeError(error)});
exit(1);
forceExit();
return;
}

nowAndTimers.setImmediate(() => {
for (const rejection of currentlyUnhandled()) {
const unhandled = currentlyUnhandled();
if (unhandled.length === 0) {
return;
}

for (const rejection of unhandled) {
channel.send({type: 'unhandled-rejection', err: serializeError(rejection.reason, {testFile: options.file})});
}

exit(0);
forceExit();
});
});

process.on('uncaughtException', error => {
channel.send({type: 'uncaught-exception', err: serializeError(error, {testFile: options.file})});
exit(1);
forceExit();
});

// Store value to prevent required modules from modifying it.
Expand Down Expand Up @@ -248,11 +244,11 @@ const run = async options => {
channel.unref();
} else {
channel.send({type: 'missing-ava-import'});
exit(1);
forceExit();
}
} catch (error) {
channel.send({type: 'uncaught-exception', err: serializeError(error, {testFile: options.file})});
exit(1);
forceExit();
}
};

Expand Down
16 changes: 0 additions & 16 deletions lib/worker/channel.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 2 additions & 0 deletions test-tap/reporters/default.regular.v18.log
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@

null

---tty-stream-chunk-separator
✘ unhandled-rejection.cjs exited with a non-zero exit code: 1
---tty-stream-chunk-separator
─

Expand Down
2 changes: 2 additions & 0 deletions test-tap/reporters/default.regular.v20.log
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@

null

---tty-stream-chunk-separator
✘ unhandled-rejection.cjs exited with a non-zero exit code: 1
---tty-stream-chunk-separator
─

Expand Down
2 changes: 2 additions & 0 deletions test-tap/reporters/default.regular.v21.log
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@

null

---tty-stream-chunk-separator
✘ unhandled-rejection.cjs exited with a non-zero exit code: 1
---tty-stream-chunk-separator
─

Expand Down
4 changes: 3 additions & 1 deletion test-tap/reporters/tap.regular.v18.log
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,13 @@ not ok 27 - unhandled-rejection
formatted: 'null'
...
---tty-stream-chunk-separator
not ok 28 - unhandled-rejection.cjs exited with a non-zero exit code: 1
---tty-stream-chunk-separator

1..21
# tests 20
# pass 6
# skip 1
# fail 20
# fail 21

---tty-stream-chunk-separator
4 changes: 3 additions & 1 deletion test-tap/reporters/tap.regular.v20.log
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,13 @@ not ok 27 - unhandled-rejection
formatted: 'null'
...
---tty-stream-chunk-separator
not ok 28 - unhandled-rejection.cjs exited with a non-zero exit code: 1
---tty-stream-chunk-separator

1..21
# tests 20
# pass 6
# skip 1
# fail 20
# fail 21

---tty-stream-chunk-separator
4 changes: 3 additions & 1 deletion test-tap/reporters/tap.regular.v21.log
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,13 @@ not ok 27 - unhandled-rejection
formatted: 'null'
...
---tty-stream-chunk-separator
not ok 28 - unhandled-rejection.cjs exited with a non-zero exit code: 1
---tty-stream-chunk-separator

1..21
# tests 20
# pass 6
# skip 1
# fail 20
# fail 21

---tty-stream-chunk-separator
Loading