Improve parallel test child process silent by osukaa · Pull Request #15938 · nodejs/node

osukaa · 2017-10-06T17:36:09Z

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Affected core subsystem(s)

test/parallel/test-child-process-silent.js

cjihrig

LGTM pending CI.

BridgeAR · 2017-10-07T21:46:50Z

test/parallel/test-child-process-silent.js

  });
  let stderrData = false;
-  parent.stdout.on('data', function() {
+  parent.stderr.on('data', function() {


I am not sure why this was changed?

@BridgeAR the original seems wrong as there is already a listener for stdout above and this should actually work on stderr.

BridgeAR · 2017-10-07T21:48:19Z

test/parallel/test-child-process-silent.js

-    assert.ok(childSending, 'The child was able to send a message');
-    assert.ok(childReciveing, 'The child was able to receive a message');
+    assert.ok(childSending);
+    assert.ok(childReceiving);


Instead of just removing this it would probably be best to either change it to assert.strictEqual or to include the value itself in the error message. But now it would not be known what the value is.

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

BridgeAR · 2017-10-19T04:34:22Z

Landed in f720b82...53f0884

Thanks for the PR, and congratulations on becoming a Node.js Contributor 🎉 !

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: nodejs/node#15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

PR-URL: nodejs/node#15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

osukaa added 3 commits October 6, 2017 10:33

test: fix typo

18feb2f

test: fix stderr reference

4e72bec

test: remove literal messages to only use assert default message

dda7831

nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 6, 2017

mscdex added the child_process Issues and PRs related to the child_process subsystem. label Oct 6, 2017

Trott added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 6, 2017

cjihrig approved these changes Oct 7, 2017

View reviewed changes

BridgeAR reviewed Oct 7, 2017

View reviewed changes

lpinca approved these changes Oct 13, 2017

View reviewed changes

BridgeAR approved these changes Oct 18, 2017

View reviewed changes

BridgeAR self-assigned this Oct 18, 2017

BridgeAR pushed a commit that referenced this pull request Oct 19, 2017

test: fix typo

f720b82

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

BridgeAR pushed a commit that referenced this pull request Oct 19, 2017

test: fix stderr reference

bd74fb4

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

BridgeAR pushed a commit that referenced this pull request Oct 19, 2017

test: remove literal messages

53f0884

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

BridgeAR closed this Oct 19, 2017

MylesBorins pushed a commit that referenced this pull request Oct 23, 2017

test: fix typo

2b46049

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

MylesBorins pushed a commit that referenced this pull request Oct 23, 2017

test: fix stderr reference

7634652

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

MylesBorins pushed a commit that referenced this pull request Oct 23, 2017

test: remove literal messages

d472922

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 26, 2017

test: fix typo

3a95a90

PR-URL: nodejs/node#15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 26, 2017

test: fix stderr reference

5c470df

PR-URL: nodejs/node#15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

MylesBorins pushed a commit that referenced this pull request Nov 16, 2017

test: fix stderr reference

67c02a5

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

MylesBorins pushed a commit that referenced this pull request Nov 16, 2017

test: remove literal messages

3099d70

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

MylesBorins added the land-on-v6.x label Nov 16, 2017

MylesBorins pushed a commit that referenced this pull request Nov 16, 2017

test: fix typo

1f4cd7d

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

MylesBorins mentioned this pull request Nov 21, 2017

v6.12.1 proposal #17180

Merged

MylesBorins pushed a commit that referenced this pull request Nov 21, 2017

test: fix stderr reference

b206395

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

MylesBorins pushed a commit that referenced this pull request Nov 21, 2017

test: remove literal messages

4705989

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

MylesBorins pushed a commit that referenced this pull request Nov 21, 2017

test: fix typo

970a8ab

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

MylesBorins pushed a commit that referenced this pull request Nov 28, 2017

test: fix stderr reference

aa269ad

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

MylesBorins pushed a commit that referenced this pull request Nov 28, 2017

test: remove literal messages

6f15b01

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

MylesBorins pushed a commit that referenced this pull request Nov 28, 2017

test: fix typo

143d8a1

PR-URL: #15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017

test: fix typo

d70793a

PR-URL: nodejs/node#15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017

test: fix stderr reference

258e961

PR-URL: nodejs/node#15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve parallel test child process silent#15938

Improve parallel test child process silent#15938
osukaa wants to merge 3 commits intonodejs:masterfrom
osukaa:improve-parallel-test-child-process-silent

osukaa commented Oct 6, 2017

Uh oh!

cjihrig left a comment

Uh oh!

BridgeAR Oct 7, 2017

Uh oh!

lpinca Oct 13, 2017

Uh oh!

BridgeAR Oct 7, 2017

Uh oh!

BridgeAR commented Oct 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Uh oh!

Conversation

osukaa commented Oct 6, 2017

Checklist

Affected core subsystem(s)

Uh oh!

cjihrig left a comment

Choose a reason for hiding this comment

Uh oh!

BridgeAR Oct 7, 2017

Choose a reason for hiding this comment

Uh oh!

lpinca Oct 13, 2017

Choose a reason for hiding this comment

Uh oh!

BridgeAR Oct 7, 2017

Choose a reason for hiding this comment

Uh oh!

BridgeAR commented Oct 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants