FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: add test-spawn-cmd-named-pipe by orangemocha · Pull Request #2770 · nodejs/node · GitHub

/ node Public

test: add test-spawn-cmd-named-pipe - #2770

Closed
orangemocha wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:orangemocha-testCmdNamedPipe
ConversationCommits (1)ChecksFiles changed
Closed

test: add test-spawn-cmd-named-pipe#2770
orangemocha wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:orangemocha-testCmdNamedPipe

Conversation

orangemocha commented Sep 9, 2015

Copy link
Copy Markdown
Contributor

Adding a Windows test to verify that a node process spawned via
cmd with named pipes can access its stdio streams.

Ref: nodejs/node-v0.x-archive#7345

This test was orphaned in joyent/node:master. It is still useful.

mscdex added child_process Issues and PRs related to the child_process subsystem. test Issues and PRs related to the tests. labels Sep 9, 2015
cjihrig reviewed Sep 9, 2015
View reviewed changes
Comment thread test/parallel/test-spawn-cmd-named-pipe.js Outdated

cjihrig Sep 9, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you remove this section.

orangemocha commented Sep 9, 2015

Copy link
Copy Markdown
Contributor Author

Thanks @cjihrig for all the feedback! I will revamp this test according to your suggestions.

Can the Copyright notice really be removed? I don't think so. Even with a permissive license, one of the clauses is to preserve the copyright notice.

cjihrig commented Sep 9, 2015

Copy link
Copy Markdown
Contributor

Yea, the copyright in all the files was removed a long time ago in io.js. I'm not a lawyer, but I think I remember something about only needing the license in the root of the project.

orangemocha commented Sep 9, 2015

Copy link
Copy Markdown
Contributor Author

Ok, I'll take your word for it.

thefourtheye reviewed Sep 10, 2015
View reviewed changes
Comment thread test/parallel/test-spawn-cmd-named-pipe.js Outdated

thefourtheye Sep 10, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Please use the format as in #2109

orangemocha force-pushed the orangemocha-testCmdNamedPipe branch from 0e0886b to e9003b2 Compare September 11, 2015 14:59

orangemocha commented Sep 11, 2015

Copy link
Copy Markdown
Contributor Author

Updated. PTAL.

cjihrig commented Sep 17, 2015

Copy link
Copy Markdown
Contributor

The commented out console.log()s should probably go. You could optionally change a few lets to consts. Other than that, LGTM if the CI is happy.

thefourtheye reviewed Sep 17, 2015
View reviewed changes
Comment thread test/parallel/test-spawn-cmd-named-pipe.js Outdated

thefourtheye Sep 17, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Const

orangemocha force-pushed the orangemocha-testCmdNamedPipe branch from e9003b2 to 46e1d9f Compare September 17, 2015 11:06

orangemocha commented Sep 17, 2015

Copy link
Copy Markdown
Contributor Author

Updated according to feedback. Thanks again for all the help! Much more slick now 👍

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

orangemocha commented Sep 17, 2015

Copy link
Copy Markdown
Contributor Author

argh.. linter errors

test: add test-spawn-cmd-named-pipe
ab7d0b8 
Adding a Windows test to verify that a node process spawned via
cmd with named pipes can access its stdio streams.

Ref: nodejs/node-v0.x-archive#7345
orangemocha force-pushed the orangemocha-testCmdNamedPipe branch from 46e1d9f to ab7d0b8 Compare September 17, 2015 11:11

orangemocha commented Sep 17, 2015

Copy link
Copy Markdown
Contributor Author

Fixed whitespace. One more try: https://ci.nodejs.org/job/node-test-pull-request/334/

RickAllMighty8195 mentioned this pull request Feb 11, 2024
Open
Megoy-Tambayan mentioned this pull request Feb 11, 2024
Open
Dixiejane mentioned this pull request Feb 11, 2024
Open
JCsomeShots mentioned this pull request Feb 11, 2024
Open
jj-crypto mentioned this pull request Feb 11, 2024
Open
ZRDK mentioned this pull request Feb 11, 2024
Open
buzzlightyear726 mentioned this pull request Feb 11, 2024
Open
Renukaloch mentioned this pull request Feb 11, 2024
Open
MaxMood96 mentioned this pull request Feb 11, 2024
Open
davidmather mentioned this pull request Feb 11, 2024
Open
mrod0101 mentioned this pull request Feb 11, 2024
Closed
CxsLucyfer mentioned this pull request Feb 11, 2024
Open
dewaynesmith3711 mentioned this pull request Feb 11, 2024
Open
kingjay66 mentioned this pull request Feb 11, 2024
Open
zhangpan0907 mentioned this pull request Feb 11, 2024
Open
Supermajestcd mentioned this pull request Feb 12, 2024
Open
MatheusKael mentioned this pull request Feb 12, 2024
Open
SanchitMahajan236 mentioned this pull request Feb 12, 2024
Open
terrorizer1980 mentioned this pull request Feb 12, 2024
Open
myvali998 mentioned this pull request Feb 12, 2024
Open
ambai mentioned this pull request Feb 12, 2024
Open
dewaynesmith3711 mentioned this pull request Feb 12, 2024
Open
BryanLaura mentioned this pull request Feb 12, 2024
Open
nicholaschoi168 mentioned this pull request Feb 12, 2024
Open
jefeaguila mentioned this pull request Feb 12, 2024
Open
Michaela813 mentioned this pull request Feb 12, 2024
Open
chrismilleruk mentioned this pull request Feb 12, 2024
Open
Michaela813 mentioned this pull request Feb 12, 2024
Open
JoanKalanzi1 mentioned this pull request Feb 12, 2024
Open
BryanLaura mentioned this pull request Feb 12, 2024
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

child_process Issues and PRs related to the child_process subsystem. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL