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

win,test: fix test-stdin-from-file #1067

Merged
merged 1 commit into from Mar 5, 2015

Conversation

piscisaureus
Copy link
Contributor

The test-stdin-from-from-file test runs a subprocess that verifies stdin
can be piped from a file.

The subprocess additionally attempts to verify that the file descriptor
for stdin never gets closed. It used to do this by creating a TCP server
and asserting that the associated file descriptor is greater than two.
However this strategy doesn't work on windows, because servers don't
have an associated file descriptor. With this patch an ordinary file is
opened instead of creating a server.

R=@indutny
R=@iojs/platform-windows
cc @rvagg

@rvagg
Copy link
Member

rvagg commented Mar 5, 2015

// If stdin's fd was closed, the next open() call would return 0. - is this strictly true on all relevant platforms?

LGTM if this makes sense to you

@seishun
Copy link
Contributor

seishun commented Mar 5, 2015

The new test passes for me, if that helps.

@indutny
Copy link
Member

indutny commented Mar 5, 2015

LGTM

@piscisaureus
Copy link
Contributor Author

If stdin's fd was closed, the next open() call would return 0.

Yes. POSIX mandates that open() returns the lowest available file descriptor.
For files, msvcrt emulates this behavior.

The test-stdin-from-from-file test runs a subprocess that verifies stdin
can be piped from a file.

The subprocess additionally attempts to verify that the file descriptor
for stdin never gets closed. It used to do this by creating a TCP server
and asserting that the associated file descriptor is greater than two.
However this strategy doesn't work on windows, because servers don't
have an associated file descriptor. With this patch an ordinary file is
opened instead of creating a server.

PR: nodejs#1067
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
@piscisaureus piscisaureus merged commit abd3ecf into nodejs:v1.x Mar 5, 2015
@piscisaureus piscisaureus deleted the fix-test-stdin-from-file branch March 5, 2015 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants