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

feat: add option to use posix exit code upon fatal signal #4989

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
8999b7b
Option to use posix exit code upon fatal signal
73rhodes May 23, 2023
3b99517
Update test/integration/options/posixExitCodes.spec.js
73rhodes Mar 15, 2024
2150c30
Address PR review comments mocha org
73rhodes Mar 18, 2024
0fb6ca3
Merge pull request #2 from zendesk/dderidder/X-mochajs-pr-review
73rhodes Mar 18, 2024
62528c5
Address PR review comments mocha org
73rhodes Mar 18, 2024
8a4f2bd
Merge pull request #3 from zendesk/dderidder/X-mochajs-pr-review
73rhodes Mar 18, 2024
817d890
Merge branch 'master' into master
73rhodes Mar 18, 2024
ad30057
Update docs/index.md
73rhodes Mar 25, 2024
592f071
Fix exit code when no signal caught
73rhodes Apr 11, 2024
34f2822
Coverage for posix exit code with normally failing tests
73rhodes Apr 11, 2024
3438da6
Merge pull request #4 from zendesk/dderidder/X-mochajs-pr-review-part2
73rhodes Apr 11, 2024
b6e6d30
Update docs/index.md
73rhodes Apr 15, 2024
d2991a1
Address PR comments; use os.constants
73rhodes Apr 15, 2024
7cf79d8
Merge pull request #5 from zendesk/dderidder/X-mochajs-pr-review-part3
73rhodes Apr 15, 2024
609f94f
Remove test that asserts the problematic behavior
73rhodes Apr 23, 2024
b71eaf3
Merge pull request #6 from zendesk/dderidder/X-mochajs-pr-review-part4
73rhodes Apr 23, 2024
26bebac
Merge branch 'mochajs:master' into master
73rhodes Apr 24, 2024
0aa939f
Omit win32 from signals test suite
73rhodes Apr 26, 2024
59e471d
Merge pull request #7 from zendesk/dderidder/X-win32-fixup
73rhodes Apr 26, 2024
6d237fc
Update docs/index.md
73rhodes May 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 14 additions & 2 deletions bin/mocha.js 100644 → 100755
Expand Up @@ -10,6 +10,7 @@
* @private
*/

const os = require('os');
const {loadOptions} = require('../lib/cli/options');
const {
unparseNodeFlags,
Expand All @@ -22,6 +23,9 @@ const {aliases} = require('../lib/cli/run-option-metadata');

const mochaArgs = {};
const nodeArgs = {};
const EXIT_SUCCESS = 0;
const EXIT_FAILURE = 1;
const SIGNAL_OFFSET= 128;
let hasInspect = false;

const opts = loadOptions(process.argv.slice(2));
Expand Down Expand Up @@ -109,7 +113,15 @@ if (mochaArgs['node-option'] || Object.keys(nodeArgs).length || hasInspect) {
proc.on('exit', (code, signal) => {
process.on('exit', () => {
if (signal) {
process.kill(process.pid, signal);
const numericSignal =
typeof signal === 'string' ? os.constants.signals[signal] : signal;
if (mochaArgs['posix-exit-codes'] === true) {
process.exit(SIGNAL_OFFSET + numericSignal);
} else {
process.kill(process.pid, signal);
}
} else if (code !== 0 && mochaArgs['posix-exit-codes'] === true) {
process.exit(EXIT_FAILURE);
} else {
process.exit(code);
73rhodes marked this conversation as resolved.
Show resolved Hide resolved
}
Expand All @@ -126,7 +138,7 @@ if (mochaArgs['node-option'] || Object.keys(nodeArgs).length || hasInspect) {
// be needed.
if (!args.parallel || args.jobs < 2) {
// win32 does not support SIGTERM, so use next best thing.
if (require('os').platform() === 'win32') {
if (os.platform() === 'win32') {
proc.kill('SIGKILL');
} else {
// using SIGKILL won't cleanly close the output streams, which can result
Expand Down
12 changes: 12 additions & 0 deletions docs/index.md
Expand Up @@ -936,6 +936,18 @@ Define a global variable name. For example, suppose your app deliberately expose

By using this option in conjunction with `--check-leaks`, you can specify a whitelist of known global variables that you _expect_ to leak into global scope.

### `--posix-exit-codes`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Threading #4989 (comment) & from #4989 (comment):

(which might better be called --unix-exit-codes based on the discussion here)

Yeah, the requested behavior has evolved a good bit. I wonder if a better name would be --standard-exit-codes? That way the word "standard" is intentionally a little ambiguous, to match how this adheres more to the general community standard of how test runners should exit, rather than the precise posix or unix standards...

This isn't a blocker for me. The long-term plan is to make this the default in a future Mocha version anyway. So I'll happily defer to you & the rest of the team. 😄

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've no strong opinion on the naming and can change it if that's needed to get final approval, but I'll leave it as-is for now considering that the gnu docs I referenced mention this kind of exit code is standard for posix systems... lmk if I should change it though.


Exits with standard POSIX exit codes instead of the number of failed tests.
73rhodes marked this conversation as resolved.
Show resolved Hide resolved

Those exit codes are:

- `0`: if all tests passed
- `1`: if any test failed
- `128 + <signal>` if given a signal, such as:
- 134: `SIGABRT` (`128 + 6`)
- 143: `SIGTERM` (`128 + 15`)

### `--retries <n>`

Retries failed tests `n` times.
Expand Down
1 change: 1 addition & 0 deletions lib/cli/run-option-metadata.js
Expand Up @@ -45,6 +45,7 @@ const TYPES = (exports.types = {
'list-reporters',
'no-colors',
'parallel',
'posix-exit-codes',
'recursive',
'sort',
'watch'
Expand Down
4 changes: 4 additions & 0 deletions lib/cli/run.js
73rhodes marked this conversation as resolved.
Show resolved Hide resolved
Expand Up @@ -190,6 +190,10 @@ exports.builder = yargs =>
description: 'Run tests in parallel',
group: GROUPS.RULES
},
'posix-exit-codes': {
description: 'Use POSIX and UNIX shell exit codes as Mocha\'s return value',
group: GROUPS.RULES
},
recursive: {
description: 'Look for tests in subdirectories',
group: GROUPS.FILES
Expand Down
23 changes: 23 additions & 0 deletions test/integration/fixtures/failing.fixture.js
@@ -0,0 +1,23 @@
'use strict';

// One passing test and three failing tests

var assert = require('assert');

describe('suite', function () {
it('test1', function () {
assert(true);
});

it('test2', function () {
assert(false);
});

it('test3', function () {
assert(false);
});

it('test4', function () {
assert(false);
});
});
7 changes: 7 additions & 0 deletions test/integration/fixtures/signals-sigabrt.fixture.js
@@ -0,0 +1,7 @@
'use strict';

describe('signal suite', function () {
it('test SIGABRT', function () {
process.kill(process.pid, 'SIGABRT');
});
});
7 changes: 7 additions & 0 deletions test/integration/fixtures/signals-sigterm.fixture.js
@@ -0,0 +1,7 @@
'use strict';

describe('signal suite', function () {
it('test SIGTERM', function () {
process.kill(process.pid, 'SIGTERM');
});
});
65 changes: 65 additions & 0 deletions test/integration/options/posixExitCodes.spec.js
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
@@ -0,0 +1,65 @@
'use strict';

var helpers = require('../helpers');
var runMocha = helpers.runMocha;
var os = require('os');

const EXIT_SUCCESS = 0;
const EXIT_FAILURE = 1;
const SIGNAL_OFFSET = 128;

describe('--posix-exit-codes', function () {
if (os.platform() !== 'win32') { // SIGTERM is not supported on Windows
describe('when enabled, and with node options', function () {
var args = ['--no-warnings', '--posix-exit-codes'];

it('should exit with correct POSIX shell code on SIGABRT', function (done) {
var fixture = 'signals-sigabrt.fixture.js';
runMocha(fixture, args, function postmortem(err, res) {
if (err) {
return done(err);
}
expect(res.code, 'to be', SIGNAL_OFFSET + os.constants.signals.SIGABRT);
done();
});
});

it('should exit with correct POSIX shell code on SIGTERM', function (done) {
var fixture = 'signals-sigterm.fixture.js';
runMocha(fixture, args, function postmortem(err, res) {
if (err) {
return done(err);
}
expect(res.code, 'to be', SIGNAL_OFFSET + os.constants.signals.SIGTERM);
done();
});
});

it('should exit with code 1 if there are test failures', function (done) {
var fixture = 'failing.fixture.js';
runMocha(fixture, args, function postmortem(err, res) {
if (err) {
return done(err);
}
expect(res.code, 'to be', EXIT_FAILURE);
done();
});
});
});
}

73rhodes marked this conversation as resolved.
Show resolved Hide resolved
describe('when not enabled, and with node options', function () {
it('should exit with the number of failed tests', function (done) {
73rhodes marked this conversation as resolved.
Show resolved Hide resolved
var fixture = 'failing.fixture.js'; // contains three failing tests
var numFailures = 3;
var args = ['--no-warnings'];
runMocha(fixture, args, function postmortem(err, res) {
if (err) {
return done(err);
}
expect(res.code, 'to be', numFailures);
done();
});
});
});
});