Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: fix test-tty-get-color-depth #18478

Closed

Conversation

bzoz
Copy link
Contributor

@bzoz bzoz commented Jan 31, 2018

If getTTYfd() returns 0, and stdin is not writable (like on Windows), trying to create WriteStream will fail. This commit fixes that by skipping fd 0.

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

test

If getTTYfd returns 0, and stdin is not writable (like on Windows),
trying to create WriteStream will fail. This commit fixes that by
skipping fd 0.
@bzoz bzoz requested a review from BridgeAR January 31, 2018 15:50
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jan 31, 2018
@bzoz
Copy link
Contributor Author

bzoz commented Jan 31, 2018

@BridgeAR BridgeAR added fast-track PRs that do not need to wait for 48 hours to land. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jan 31, 2018
@BridgeAR
Copy link
Member

BridgeAR commented Feb 1, 2018

Landed in ef28619

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Feb 1, 2018
If getTTYfd returns 0, and stdin is not writable (like on Windows),
trying to create WriteStream will fail. This commit fixes that by
skipping fd 0.

PR-URL: nodejs#18478
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR BridgeAR closed this Feb 1, 2018
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 4, 2018
@MylesBorins
Copy link
Member

test that this modifies does not exist on v9.x, setting as don't land

@bzoz
Copy link
Contributor Author

bzoz commented Mar 28, 2018

@MylesBorins the affected test was back ported in #19230, so this PR should be also backported now

targos pushed a commit that referenced this pull request Mar 30, 2018
If getTTYfd returns 0, and stdin is not writable (like on Windows),
trying to create WriteStream will fail. This commit fixes that by
skipping fd 0.

PR-URL: #18478
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos mentioned this pull request Apr 4, 2018
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
If getTTYfd returns 0, and stdin is not writable (like on Windows),
trying to create WriteStream will fail. This commit fixes that by
skipping fd 0.

PR-URL: nodejs#18478
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fast-track PRs that do not need to wait for 48 hours to land. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants