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

Support posix-exit-codes for child and in-process modes #11

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

73rhodes
Copy link

@73rhodes 73rhodes commented Jul 10, 2024

Overview

Mocha has two ways of running: as a child process or in the main process. If there are any node command-line options (with or without mocha command line options) it will spawn a child process to run mocha and pass the node-specific options to that child process.

Reviewers of our PR to the upstream mocha repository requested that the --posix-exit-codes option should be supported in both modes of operation (mocha as a child process AND mocha in the main process). Originally I had it only working for mocha as a child process because that satisfies our use-case where we run mocha via nyc which passes node-specific options.

This PR also adds some additional unit tests to round out the coverage for these four scenarios:

  • posix exit codes with mocha as a child process
  • posix exit codes with mocha in the main process
  • normal exit codes with mocha as a child process
  • normal exit codes with mocha in the main process

Note that in ALL cases, when a fatal signal (ie. SIGABRT, SIGTERM) kills the mocha process, the exit code should now be [128 + numeric value of signal] in compliance with standard posix & bash command behaviour.

@73rhodes 73rhodes force-pushed the dderidder/fix-exit-codes-pr-update branch 5 times, most recently from 33f2e8d to e532714 Compare July 11, 2024 02:20
@73rhodes 73rhodes changed the title WIP - support posix-exit-codes option for both child-process and in-p… Support posix-exit-codes for child and in-process modes Jul 11, 2024
@73rhodes 73rhodes marked this pull request as ready for review July 11, 2024 02:35
@73rhodes 73rhodes force-pushed the dderidder/fix-exit-codes-pr-update branch from e532714 to df38431 Compare July 11, 2024 02:48
Copy link

@SophiaRoland SophiaRoland left a comment

Choose a reason for hiding this comment

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

LGTM

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'];
describe('when enabled', function () {
Copy link

Choose a reason for hiding this comment

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

Does the test need coverage for exiting with success code 0?

@73rhodes 73rhodes merged commit 9ee844a into master Aug 2, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants