Skip to content

Conversation

@Fishrock123
Copy link
Contributor

@Fishrock123Fishrock123 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

@Fishrock123Fishrock123 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

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
ContributorAuthor

@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]>
@evanlucasevanlucas mentioned this pull request Jul 18, 2016
@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]>
@MylesBorinsMylesBorins mentioned this pull request Oct 26, 2016
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testIssues and PRs related to the tests.ttyIssues and PRs related to the tty subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Fishrock123@cjihrig@MylesBorins