Skip to content

Conversation

@Trott
Copy link
Member

@Trott Trott commented Mar 10, 2017

test-child-process-fork-stdio-string-variant was only testing 'pipe' for
its stdio value. Add inherit and ignore.

Also added a common.mustCall() to verify that the message event is
triggered.

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

test child_process

@Trott Trott added child_process Issues and PRs related to the child_process subsystem. test Issues and PRs related to the tests. labels Mar 10, 2017
Copy link
Member

Choose a reason for hiding this comment

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

The options object should be {stdio: stringVariant}.

Copy link
Member Author

Choose a reason for hiding this comment

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

<facepalm>...fixing...

Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

LGTM with @lpinca's nit addressed.

test-child-process-fork-stdio-string-variant was only testing 'pipe' for
its `stdio` value. Add `inherit` and `ignore`.

Also added a `common.mustCall()` to verify that the `message` event is
triggered.
@Trott
Copy link
Member Author

Trott commented Mar 10, 2017

@Trott
Copy link
Member Author

Trott commented Mar 13, 2017

Landed in 78cdd4b

@Trott Trott closed this Mar 13, 2017
Trott added a commit to Trott/io.js that referenced this pull request Mar 13, 2017
test-child-process-fork-stdio-string-variant was only testing 'pipe' for
its `stdio` value. Add `inherit` and `ignore`.

Also added a `common.mustCall()` to verify that the `message` event is
triggered.

PR-URL: nodejs#11783
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
@italoacasas
Copy link

This is not landing clearly in v7 staging, @Trott can backport?

@Trott
Copy link
Member Author

Trott commented Mar 14, 2017

@italoacasas This depends on #10866 which is semver-major. Adding dont-land-on-* labels.

jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
test-child-process-fork-stdio-string-variant was only testing 'pipe' for
its `stdio` value. Add `inherit` and `ignore`.

Also added a `common.mustCall()` to verify that the `message` event is
triggered.

PR-URL: nodejs#11783
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
@jasnell jasnell mentioned this pull request Apr 4, 2017
@Trott Trott deleted the ignore branch January 13, 2022 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

child_process Issues and PRs related to the child_process subsystem. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants