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.

Sign up for GitHub

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

Jump to bottom

test: set stdin too for pseudo-tty tests #10149

Closed
wants to merge 2 commits into from
Closed

test: set stdin too for pseudo-tty tests #10149

wants to merge 2 commits into from

Conversation

addaleax
Copy link
Member

@addaleax addaleax commented Dec 6, 2016

Checklist
  • [probably] make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Description of change

Ref: #10037
Ref: #10146

/cc @Fishrock123 @jmdarling

@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Dec 6, 2016
@addaleax addaleax changed the title test: set stdin toofor pseudo-tty tests test: set stdin too for pseudo-tty tests Dec 6, 2016
@addaleax addaleax added test Issues and PRs related to the tests. tty Issues and PRs related to the tty subsystem. labels Dec 6, 2016
@addaleax addaleax mentioned this pull request Dec 6, 2016
Closed
2 tasks
Fishrock123
Fishrock123 approved these changes Dec 6, 2016
View reviewed changes
Copy link
Contributor

@Fishrock123 Fishrock123 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@addaleax Think we are fine assigning stdin to the same one? Seems good to me if you feel it's ok.

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

@addaleax
Copy link
Member Author

addaleax commented Dec 6, 2016

Think we are fine assigning stdin to the same one?

That reflects the most common situation best, so yeah, I think we’re good with that

addaleax and others added 2 commits December 9, 2016 15:14
@addaleax
test: set stdin too for pseudo-tty tests
e5ae21b 
Ref: nodejs#10037
Ref: nodejs#10146
@addaleax
test: add stdin-setrawmode.out file
29cf6f4 
Adds an accompanying .out file for test/pseudo-tty/stdin-setrawmode.js.
The test was originally merged without this file and an astute
observer found that it was causing an error message. See discussion
at nodejs#10037.
@addaleax addaleax mentioned this pull request Dec 9, 2016
Closed
2 tasks
@addaleax
Copy link
Member Author

addaleax commented Dec 9, 2016

Landed in b8fc9a3 and def6dfb

@addaleax addaleax closed this Dec 9, 2016
@addaleax addaleax deleted the test-pseudo-tty-stdin branch December 9, 2016 14:26
addaleax added a commit that referenced this pull request Dec 9, 2016
@addaleax
test: set stdin too for pseudo-tty tests
def6dfb 
Ref: #10037
Ref: #10146
PR-URL: #10149
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
addaleax pushed a commit that referenced this pull request Dec 9, 2016
@addaleax
test: add stdin-setrawmode.out file
b8fc9a3 
Adds an accompanying .out file for test/pseudo-tty/stdin-setrawmode.js.
The test was originally merged without this file and an astute
observer found that it was causing an error message. See discussion
at #10037.

PR-URL: #10149
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@addaleax addaleax mentioned this pull request Dec 9, 2016
Closed
3 tasks
Fishrock123 pushed a commit that referenced this pull request Dec 13, 2016
@addaleax @Fishrock123
test: set stdin too for pseudo-tty tests
f5347ab 
Ref: #10037
Ref: #10146
PR-URL: #10149
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Fishrock123 pushed a commit that referenced this pull request Dec 13, 2016
@Fishrock123
test: add stdin-setrawmode.out file
28c6df2 
Adds an accompanying .out file for test/pseudo-tty/stdin-setrawmode.js.
The test was originally merged without this file and an astute
observer found that it was causing an error message. See discussion
at #10037.

PR-URL: #10149
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@italoacasas italoacasas mentioned this pull request Dec 15, 2016
Merged
@sam-github sam-github mentioned this pull request Dec 24, 2016
Closed
1 task
targos pushed a commit that referenced this pull request Dec 26, 2016
@addaleax @targos
test: set stdin too for pseudo-tty tests
e057925 
Ref: #10037
Ref: #10146
PR-URL: #10149
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
targos pushed a commit that referenced this pull request Dec 26, 2016
@targos
test: add stdin-setrawmode.out file
b5c8b35 
Adds an accompanying .out file for test/pseudo-tty/stdin-setrawmode.js.
The test was originally merged without this file and an astute
observer found that it was causing an error message. See discussion
at #10037.

PR-URL: #10149
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@targos targos mentioned this pull request Dec 26, 2016
Merged
@MylesBorins MylesBorins mentioned this pull request Dec 27, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Jan 23, 2017
@MylesBorins
test: add stdin-setrawmode.out file
69c2ea9 
Adds an accompanying .out file for test/pseudo-tty/stdin-setrawmode.js.
The test was originally merged without this file and an astute
observer found that it was causing an error message. See discussion
at #10037.

PR-URL: #10149
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 23, 2017
@addaleax @MylesBorins
test: set stdin too for pseudo-tty tests
a5fdd6a 
Ref: #10037
Ref: #10146
PR-URL: #10149
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
@MylesBorins
test: add stdin-setrawmode.out file
7bbf0d0 
Adds an accompanying .out file for test/pseudo-tty/stdin-setrawmode.js.
The test was originally merged without this file and an astute
observer found that it was causing an error message. See discussion
at #10037.

PR-URL: #10149
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
@addaleax @MylesBorins
test: set stdin too for pseudo-tty tests
e3490bd 
Ref: #10037
Ref: #10146
PR-URL: #10149
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Feb 1, 2017
@MylesBorins
test: add stdin-setrawmode.out file
53404db 
Adds an accompanying .out file for test/pseudo-tty/stdin-setrawmode.js.
The test was originally merged without this file and an astute
observer found that it was causing an error message. See discussion
at #10037.

PR-URL: #10149
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 1, 2017
@addaleax @MylesBorins
test: set stdin too for pseudo-tty tests
42da81e 
Ref: #10037
Ref: #10146
PR-URL: #10149
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Italo A. Casas <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@jasnell jasnell jasnell approved these changes

@Fishrock123 Fishrock123 Fishrock123 approved these changes

@italoacasas italoacasas italoacasas approved these changes

Assignees
No one assigned
Labels
test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. tty Issues and PRs related to the tty subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@addaleax @bnoordhuis @jasnell @Fishrock123 @italoacasas @MylesBorins @targos @nodejs-github-bot