Skip to content

Conversation

Fishrock123
Copy link
Contributor

@Fishrock123 Fishrock123 commented Jul 8, 2016

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 Fishrock123 added test Issues and PRs related to the tests. tty Issues and PRs related to the tty subsystem. labels Jul 8, 2016
@cjihrig
Copy link
Contributor

cjihrig commented Jul 8, 2016

LGTM. CI is green.

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 <[email protected]>
@MylesBorins
Copy link
Contributor

@Fishrock123 lts?

@Fishrock123
Copy link
Contributor Author

@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 Jul 15, 2016
evanlucas pushed a commit that referenced this pull request Jul 15, 2016
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 <[email protected]>
@MylesBorins
Copy link
Contributor

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

MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
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 <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
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 <[email protected]>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
rvagg pushed a commit that referenced this pull request Oct 18, 2016
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 <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
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 <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Oct 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants