Skip to content

test: stdio pipe behavior tests#18614

Closed
bzoz wants to merge 2 commits intonodejs:masterfrom
JaneaSystems:bartek-stdio-pipe-tests
Closed

test: stdio pipe behavior tests#18614
bzoz wants to merge 2 commits intonodejs:masterfrom
JaneaSystems:bartek-stdio-pipe-tests

Conversation

@bzoz
Copy link
Contributor

@bzoz bzoz commented Feb 7, 2018

Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read by another process does not deadlocks Node.js. This was reported in #10836 and was fixed in v8.3.0. The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to another process stdout does not crash Node as reported in #17493. It was fixed in #18019.

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, net

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Feb 7, 2018
@bzoz
Copy link
Contributor Author

bzoz commented Feb 7, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request/13006/

This was referenced Feb 7, 2018
Closed
Closed
@Fishrock123
Copy link
Contributor

Fishrock123 commented Feb 10, 2018

@bzoz linter failed. :)

@bzoz
test: stdio pipe behavior tests
8aeee9c 
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
nodejs#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
nodejs#17493. It was fixed in
nodejs#18019.
@bzoz bzoz force-pushed the bartek-stdio-pipe-tests branch from 10e9898 to 8aeee9c Compare February 12, 2018 12:56
@bzoz
Copy link
Contributor Author

bzoz commented Feb 12, 2018

Fixed, PTAL.

CI: https://ci.nodejs.org/job/node-test-pull-request/13123/

@bzoz
Copy link
Contributor Author

bzoz commented Feb 14, 2018

Last CI is strange, another run: https://ci.nodejs.org/job/node-test-pull-request/13174/

jasnell
jasnell reviewed Feb 14, 2018
View reviewed changes
test/parallel/test-stdio-pipe-access.js Outdated
switch (who) {
case 'runner':
for (let num = 0; num < numTries; ++num) {
console.log('Try: ' + num);
Copy link
Member

@jasnell jasnell Feb 14, 2018

Choose a reason for hiding this comment

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

if the console.log output is a part of the test, can you add a comment indicating so. If it is not, I'd prefer it to be removed.

@bzoz
Copy link
Contributor Author

bzoz commented Feb 15, 2018

Updated, PTAL

@bzoz
Copy link
Contributor Author

bzoz commented Feb 15, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request/13179/

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 16, 2018
BridgeAR
BridgeAR approved these changes Feb 16, 2018
View reviewed changes
Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

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

LGTM if CI is green.

@BridgeAR
Copy link
Member

BridgeAR commented Feb 17, 2018

Landed in 1bda746 🎉

@BridgeAR BridgeAR closed this Feb 17, 2018
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Feb 17, 2018
@bzoz @BridgeAR
test: stdio pipe behavior tests
1bda746 
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
nodejs#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
nodejs#17493. It was fixed in
nodejs#18019.

PR-URL: nodejs#18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
@bzoz @MylesBorins
test: stdio pipe behavior tests
13ff508 
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins MylesBorins mentioned this pull request Feb 21, 2018
Merged
yhwang pushed a commit to yhwang/node that referenced this pull request Feb 22, 2018
@bzoz @yhwang
test: stdio pipe behavior tests
4f1b745 
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
nodejs#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
nodejs#17493. It was fixed in
nodejs#18019.

PR-URL: nodejs#18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
addaleax pushed a commit that referenced this pull request Feb 26, 2018
@bzoz @addaleax
test: stdio pipe behavior tests
a3aebea 
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
addaleax pushed a commit that referenced this pull request Feb 26, 2018
@bzoz @addaleax
test: stdio pipe behavior tests
b34c92e 
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins
Copy link
Contributor

MylesBorins commented Feb 26, 2018 •
edited
Loading

This still seems to be breaking windows, would someone be willing to manually backport and figure out what is going on?

nvm I seem to have jumped the gun... please feel free to ignore

MylesBorins pushed a commit that referenced this pull request Feb 26, 2018
@bzoz @MylesBorins
test: stdio pipe behavior tests
df0d78a 
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@addaleax addaleax mentioned this pull request Feb 27, 2018
Closed
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
@bzoz @MayaLekova
test: stdio pipe behavior tests
ed2d7cd 
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
nodejs#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
nodejs#17493. It was fixed in
nodejs#18019.

PR-URL: nodejs#18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Aug 7, 2018
@bzoz @MylesBorins
test: stdio pipe behavior tests
c05a9b0 
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Aug 7, 2018
@bzoz @MylesBorins
test: stdio pipe behavior tests
0797dc6 
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Aug 9, 2018
@bzoz @MylesBorins
test: stdio pipe behavior tests
698e7e6 
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins MylesBorins mentioned this pull request Aug 16, 2018
Merged
MylesBorins pushed a commit that referenced this pull request Aug 16, 2018
@bzoz @MylesBorins
test: stdio pipe behavior tests
83e704d 
Add two regression tests for stdio over pipes.

test-stdio-pipe-access tests if accessing stdio pipe that is being read
by another process does not deadlocks Node.js. This was reported in
#10836 and was fixed in v8.3.0.
The deadlock would happen intermittently, so we run the test 5 times.

test-stdio-pipe-redirect tests if redirecting one child process stdin to
another process stdout does not crash Node as reported in
#17493. It was fixed in
#18019.

PR-URL: #18614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@bzoz @Fishrock123 @BridgeAR @MylesBorins @jasnell @addaleax @nodejs-github-bot