Skip to content

test: ensure callback runs in test-vm-sigint - #7768

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:mustcall
Closed

test: ensure callback runs in test-vm-sigint#7768
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:mustcall

Conversation

@Trott

Copy link
Copy Markdown
Member
Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test vm

Description of change

Make sure the child process close callback runs exactly one time.

/cc @addaleax

@TrottTrott added vm Issues and PRs related to the vm subsystem. test Issues and PRs related to the tests. labels Jul 16, 2016
@Trott

Copy link
Copy Markdown
MemberAuthor

@addaleax

Copy link
Copy Markdown
Member

LGTM, thanks

@mscdex

mscdex commented Jul 16, 2016

Copy link
Copy Markdown
Contributor

Minor nit, but perhaps s/insure/ensure/ in the commit message?

@addaleax

Copy link
Copy Markdown
Member

@cjihrig

Copy link
Copy Markdown
Contributor

LGTM even if the test is not ultimately fixed.

@Trott

Copy link
Copy Markdown
MemberAuthor

@mscdex s/insure/ensure/ done

@mscdexmscdex changed the title test: insure callback runs in test-vm-siginttest: ensure callback runs in test-vm-sigintJul 19, 2016
@Trott

Copy link
Copy Markdown
MemberAuthor

@addaleax Still looks good to you even though it doesn't fix the test? (I still think it's a minor improvement but I don't want to make assumptions that anyone agrees with that.)

@addaleax

Copy link
Copy Markdown
Member

Yep, I never expected this to fix the test in the first place. :)

Trott added a commit to Trott/io.js that referenced this pull request Jul 20, 2016
PR-URL: nodejs#7768
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott

Copy link
Copy Markdown
MemberAuthor

Landed in fd02c93

@TrottTrott closed this Jul 20, 2016
evanlucas pushed a commit that referenced this pull request Jul 21, 2016
PR-URL: #7768
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@evanlucasevanlucas mentioned this pull request Jul 21, 2016
@Trott
Trott deleted the mustcall branch January 13, 2022 22:43
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testIssues and PRs related to the tests.vmIssues and PRs related to the vm subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@Trott@addaleax@mscdex@cjihrig@MylesBorins