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

Move from "process.exit" to "exit" #5313

Merged
merged 1 commit into from
Jan 15, 2018
Merged

Move from "process.exit" to "exit" #5313

merged 1 commit into from
Jan 15, 2018

Conversation

mjesun
Copy link
Contributor

@mjesun mjesun commented Jan 15, 2018

Changing all process.exit to exit. This prevents sudden truncation of Jest output when finishing test execution, if the output information is too big (I've seen issues when the JSON output is above 256 KiB). As per the Node docs, process.exit() will not wait up until all buffers are flushed, and will try to quit as soon as possible.

Tests are still passing since exit uses process.exit underneath.

@codecov-io
Copy link

codecov-io commented Jan 15, 2018

Codecov Report

Merging #5313 into master will increase coverage by 0.03%.
The diff coverage is 30%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5313      +/-   ##
==========================================
+ Coverage   61.24%   61.27%   +0.03%     
==========================================
  Files         205      205              
  Lines        6892     6893       +1     
  Branches        3        4       +1     
==========================================
+ Hits         4221     4224       +3     
+ Misses       2670     2668       -2     
  Partials        1        1
Impacted Files Coverage Δ
packages/jest-runner/src/test_worker.js 0% <ø> (ø) ⬆️
packages/jest-cli/src/watch.js 70.25% <0%> (ø) ⬆️
packages/jest-cli/src/test_scheduler.js 26.81% <0%> (+1.26%) ⬆️
packages/jest-cli/src/reporters/coverage_worker.js 50% <0%> (ø) ⬆️
packages/jest-cli/src/reporters/notify_reporter.js 8.69% <0%> (ø) ⬆️
packages/jest-cli/src/run_jest.js 52.54% <100%> (ø) ⬆️
packages/jest-runner/src/index.js 39.13% <50%> (+1.35%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cdc048d...8242e40. Read the comment docs.

@cpojer
Copy link
Member

cpojer commented Jan 15, 2018

Is it possible to test this somehow?

@mjesun
Copy link
Contributor Author

mjesun commented Jan 15, 2018

Tests already do test that (to some extent). Some of them spy or mock process.exit (e.g. here), which de-facto means testing the correct completion plus the exit module, all at once.

@cpojer cpojer merged commit 03cce3d into jestjs:master Jan 15, 2018
@mjesun mjesun changed the title Move from "process.exit" to "exit Move from "process.exit" to "exit" Jan 19, 2018
@mjesun mjesun deleted the safe-exit branch February 7, 2018 10:37
karlhorky added a commit to uselagoon/lagoon that referenced this pull request Jul 16, 2018
JLHwung added a commit to JLHwung/jest that referenced this pull request Nov 5, 2018
JLHwung added a commit to JLHwung/jest that referenced this pull request Mar 19, 2019
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants