Skip to content

test: do not force the process to exit - #29923

Closed
lpinca wants to merge 1 commit into
nodejs:masterfrom
lpinca:remove/process-exit
Closed

test: do not force the process to exit#29923
lpinca wants to merge 1 commit into
nodejs:masterfrom
lpinca:remove/process-exit

Conversation

@lpinca

Copy link
Copy Markdown
Member

Refs: #29873 (review)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Oct 10, 2019

@TrottTrott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. I'd prefer some details in the commit message, but that's not blocking. Maybe something like this?:

Using process.exit() in these tests is unnecessary and may mask other problems.

@lpinca
lpincaforce-pushed the remove/process-exit branch from b3b9211 to b9139a4CompareOctober 11, 2019 13:20
@TrottTrott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 11, 2019
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Using `process.exit()` in these tests is unnecessary and may mask other
problems.
Refs: nodejs#29873 (review)
@lpinca
lpincaforce-pushed the remove/process-exit branch from b9139a4 to 5527329CompareOctober 11, 2019 18:46
@lpinca

Copy link
Copy Markdown
MemberAuthor

Added a few more, PTAL.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Trott pushed a commit that referenced this pull request Oct 13, 2019
Using `process.exit()` in these tests is unnecessary and may mask other
problems.
Refs: #29873 (review)
PR-URL: #29923
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
@Trott

Copy link
Copy Markdown
Member

Landed in 19a8d22

@TrottTrott closed this Oct 13, 2019
@lpinca
lpinca deleted the remove/process-exit branch October 13, 2019 05:13
targos pushed a commit that referenced this pull request Oct 14, 2019
Using `process.exit()` in these tests is unnecessary and may mask other
problems.
Refs: #29873 (review)
PR-URL: #29923
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants

@lpinca@nodejs-github-bot@Trott@Fishrock123@antsmartian@cjihrig@devnexen@richardlau@BridgeAR@ZYSzys