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

Fix tty tests #7613

Merged

Fishrock123 merged 2 commits into nodejs:master from Fishrock123:fix-tty-tests

Jul 11, 2016

Contributor

Fishrock123 commented Jul 8, 2016 •

edited

Loading

Checklist

make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Affected core subsystem(s)

test, tty

Description of change

Found these while investigating #7360.

Turns out test-tty-wrap.js hasn't worked for over a year.
test-tty-stdout-end.js works because the same functionality just so happens to also exist on PipeWraps.

Commit 1 is to preserve the file history correctly.

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

R: @nodejs/testing I suppose

Fishrock123 added test tty labels

Contributor

cjihrig commented Jul 8, 2016

LGTM. CI is green.

Fishrock123 added 2 commits

July 11, 2016 15:22


          test: move parallel/test-tty-* to pseudo-tty/

f3a3888

Refs: nodejs#7360
PR-URL: nodejs#7613
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>


          test: fix old tty tests

aab3066

test-tty-wrap hasn’t worked since StreamBase was introduced, I think.
test-tty-stdout-end also happens to works with PipeWrap-s.

Refs: nodejs#7360
PR-URL: nodejs#7613
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Fishrock123 force-pushed the fix-tty-tests branch from abb6fe8 to aab3066 Compare

July 11, 2016 13:25

Fishrock123 merged commit aab3066 into nodejs:master

MylesBorins mentioned this pull request

Audit commits not found on v4.x-staging #7661

Closed

MylesBorins added the lts-watch-v4.x label

Contributor

MylesBorins commented Jul 12, 2016

@Fishrock123 lts?

Contributor Author

Fishrock123 commented Jul 12, 2016

@thealphanerd You'd also need to backport 88804b8

The test should pass.

Maybe that entire PR should be backported? unsure

evanlucas pushed a commit that referenced this pull request


          test: move parallel/test-tty-* to pseudo-tty/

37dc795

Refs: #7360
PR-URL: #7613
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

evanlucas pushed a commit that referenced this pull request


          test: fix old tty tests

test-tty-wrap hasn’t worked since StreamBase was introduced, I think.
test-tty-stdout-end also happens to works with PipeWrap-s.

Refs: #7360
PR-URL: #7613
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

evanlucas mentioned this pull request

v6.3.1 Release Proposal #7782

Merged

insightfuls mentioned this pull request

tty: add ref() so process.stdin.ref() etc. work #7360

Closed

3 tasks

Contributor

MylesBorins commented Aug 30, 2016

@Fishrock123 this is not landing cleanly on v4.x-staging

MylesBorins pushed a commit that referenced this pull request


          test: move parallel/test-tty-* to pseudo-tty/

0bf410b

Refs: #7360
PR-URL: #7613
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

MylesBorins pushed a commit that referenced this pull request


          test: fix old tty tests

test-tty-wrap hasn’t worked since StreamBase was introduced, I think.
test-tty-stdout-end also happens to works with PipeWrap-s.

Refs: #7360
PR-URL: #7613
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

MylesBorins added land-on-v4.x and removed lts-watch-v4.x labels

MylesBorins pushed a commit that referenced this pull request


          test: move parallel/test-tty-* to pseudo-tty/

b613d2a

Refs: #7360
PR-URL: #7613
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

MylesBorins pushed a commit that referenced this pull request


          test: fix old tty tests

7a100b3

test-tty-wrap hasn’t worked since StreamBase was introduced, I think.
test-tty-stdout-end also happens to works with PipeWrap-s.

Refs: #7360
PR-URL: #7613
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

rvagg pushed a commit that referenced this pull request


          test: move parallel/test-tty-* to pseudo-tty/

ed11714

Refs: #7360
PR-URL: #7613
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

rvagg pushed a commit that referenced this pull request


          test: fix old tty tests

a6db44c

test-tty-wrap hasn’t worked since StreamBase was introduced, I think.
test-tty-stdout-end also happens to works with PipeWrap-s.

Refs: #7360
PR-URL: #7613
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

MylesBorins pushed a commit that referenced this pull request


          test: move parallel/test-tty-* to pseudo-tty/

22990d8

Refs: #7360
PR-URL: #7613
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

MylesBorins pushed a commit that referenced this pull request


          test: fix old tty tests

cd9e8e0

test-tty-wrap hasn’t worked since StreamBase was introduced, I think.
test-tty-stdout-end also happens to works with PipeWrap-s.

Refs: #7360
PR-URL: #7613
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

MylesBorins mentioned this pull request

V4.6.2 proposal #9298

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test tty