Support mocha's --posix-exit-codes option.#100
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @73rhodes, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request enhances ts-mocha by integrating support for Mocha's --posix-exit-codes option. This modification ensures that ts-mocha exits with standard POSIX-compliant codes when a fatal signal is encountered, which is crucial for robust error handling and preventing silent failures in automated CI/CD environments. The change also includes necessary documentation updates.
Highlights
- New Feature: POSIX Exit Codes: This pull request introduces support for Mocha's
--posix-exit-codesoption, enablingts-mochato exit with standard POSIX codes (128 + signal number) when a fatal signal is received, rather than the number of failed tests. - Improved CI/CD Reliability: The change addresses a critical issue where
ts-mochacould fail silently in CI/CD pipelines due to non-standard exit codes, particularly in scenarios involving out-of-memory errors or other fatal signals, by ensuring a clear, standard exit status. - Documentation Update: The
README.mdhas been updated to clearly document the newly supported--posix-exit-codescommand-line option, providing users with information on its purpose and usage.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces support for a --posix-exit-codes flag, which makes ts-mocha exit with POSIX-compliant codes when the underlying mocha process is terminated by a signal. The implementation is mostly correct, but I've pointed out a few areas for improvement. The most critical one is the use of process.exit() within a process.on('exit') handler, which should be avoided. I've also suggested some minor improvements for code clarity and documentation structure.
| if(process.env.POSIX_EXIT_CODES == 'true') { | ||
| console.warn(`ts-mocha exiting with code ${128 + os.constants.signals[signal]}`) | ||
| process.exitCode = 128 + os.constants.signals[signal]; | ||
| process.exit(process.exitCode); | ||
| } else { | ||
| console.warn(`ts-mocha killing process with ${signal}; process.exitCode ${process.exitCode}`); | ||
| process.kill(process.pid, signal); | ||
| } |
There was a problem hiding this comment.
This block has a few areas for improvement:
high: Callingprocess.exit()inside aprocess.on('exit')event handler is not recommended. The process is already exiting, so you should only setprocess.exitCodeand let Node.js handle the termination. The explicitprocess.exit()call is redundant and can prevent other'exit'listeners from running.medium: The expression128 + os.constants.signals[signal]is calculated multiple times. It's better to compute it once and store it in a constant for improved readability and efficiency.medium: It's a good practice in JavaScript to use strict equality (===) instead of loose equality (==) to prevent unexpected type coercion.
if (process.env.POSIX_EXIT_CODES === 'true') {
const exitCode = 128 + os.constants.signals[signal];
console.warn(`ts-mocha exiting with code ${exitCode}`);
process.exitCode = exitCode;
} else {
console.warn(`ts-mocha killing process with ${signal}; process.exitCode ${process.exitCode}`);
process.kill(process.pid, signal);
}
|
|
||
| `-p, --project <value>` - relative or absolute path to a `tsconfig.json` file (equivalent of `tsc -p <value>`) [default: "./tsconfig.json"] | ||
|
|
||
| `--posix-exit-codes` - exit with non-zero POSIX code if interrupted with fatal signal |
There was a problem hiding this comment.
While adding documentation for the new flag is great, its current placement interrupts the flow between the --project option and its corresponding example. For better readability, consider moving this new option to be grouped with the other option flags, for instance, after the --type-check flag's section.
|
|
||
| case "--posix-exit-codes": | ||
| process.env.POSIX_EXIT_CODES = true; | ||
| mochaArgs.push(arg); // add when supported by mocha |
There was a problem hiding this comment.
The comment // add when supported by mocha is confusing. The pull request description states this is for "mocha's new --posix-exit-codes command line option", which implies mocha does support this option. If that's the case, this comment is misleading and should be removed. If mocha does not support this option, passing it might cause an "unrecognized option" error.
mochaArgs.push(arg);
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for mocha's --posix-exit-codes command line option to ts-mocha, enabling standard POSIX exit codes instead of using the number of failed tests as the exit code.
- Adds documentation for the new
--posix-exit-codesoption in the README
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This PR adds support for mocha's new --posix-exit-codes command line option. When this option is given, mocha will exit using standard posix exit codes instead of using he number of failed tests as the exit code. This avoids situations like out-of-memory errors causing CICD pipelines that use ts-mocha from failing silently.