test: stdin is not always a net.Socket

[Fishrock123]

Pull Request check-list

• Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
  this change (including linting)?
• Is the commit message formatted according to CONTRIBUTING.md?

Affected core subsystem(s)

test

Description of change

Refs: #5916 (comment)

<-ing a file into stdin actually results in a fs.ReadStream, rather an a tty.ReadStream, and as such does not inherit from net.Socket, unlike the other possible stdin options:

node/lib/internal/process/stdio.js

Lines 54 to 57 in 293fd04

	case'FILE':
	varfs=require('fs');
	stdin=newfs.ReadStream(null,{fd: fd,autoClose: false});
	break;

cc @Trott I think this should cover the known issue.

[cjihrig]

Can you move this comment up to line 2.

[cjihrig]

LGTM with a couple comments.

[Fishrock123]

Hmmm, doing this reliably in a must-fail environment is quite tricky.

[Fishrock123]

CI on windows: https://ci.nodejs.org/job/node-test-known-issues/2/ (That ci says to not run with all?)

[Fishrock123]

@cjihrig LGTY now?

[cjihrig]

Yea, LGTM

[cjihrig]

@Fishrock123 is there a particular issue on the issue tracker that this maps to? I noticed you Refs'ed a PR. I've been adding the repro-exists label to the referenced issues, but I'm not not if that would apply to a PR.

[phillipj]

@Fishrock123 did this pass linting for you? I'm getting this locally:

➜ make lint ./node tools/eslint/bin/eslint.js benchmark lib src test tools/doc \ tools/eslint-rules --rulesdir tools/eslint-rules /Users/phillipj/node/test/known_issues/test-stdin-is-always-net.socket.js 14:1 error Line 14 exceeds the maximum line length of 80 max-len ✖ 1 problem (1 error, 0 warnings)

[thefourtheye]

#5980 Fixes the linter error

[jasnell]

Is this applicable to v4?

[Fishrock123]

@jasnell do we put known-issue tests there? if so, yes this applies back as far as I can remember.

[jasnell]

Yep, we can port the known-issue tests back to v4 now

[addaleax]

@Fishrock123 I know this was a long, long time ago, but what exactly is the issue that this test is catching? It sounds like the underlying idea is that stdio streams should always inherit from net.Socket, but that isn’t really made explicit anywhere, is it?

[Fishrock123]

@addaleax There was this thing at one point where sometimes stdin wasnt a net.Socket... I think this checks when stdin doesn't actually exist that it still makes a socket?

It was supposed to be a consistency check.

---

Edit:

It sounds like the underlying idea is that stdio streams should always inherit from net.Socket

Yes.