Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.3k
test: improve readline test coverage for tty#12064
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.
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
test: improve readline test coverage for tty #12064
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Fishrock123 commented Mar 27, 2017
So, to be clear... this test file runes with stdio as pipes, not ttys. By the look of the code this doesn't actually touch TTY at all, maybe that should be removed form the commit message? |
Fishrock123 commented Mar 27, 2017
claudiorodriguez commented Mar 27, 2017
@Fishrock123 sorry, what I meant is that it improves coverage for |
Fishrock123 commented Mar 27, 2017
if |
claudiorodriguez commented Mar 27, 2017
@Fishrock123 sure thing, I'll wait for the CI run to complete then fix that |
a36f53e to 4062b64Compareclaudiorodriguez commented Apr 18, 2017
@Fishrock123 completely forgot about this, does the new message seem alright to you? |
BridgeAR left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG if CI is green. This needs a rebase though
BridgeAR commented Aug 26, 2017
@claudiorodriguez would you be so kind and rebase this? |
4062b64 to 41f86f9Compareclaudiorodriguez commented Sep 5, 2017
@BridgeAR rebased, cheers |
BridgeAR left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still LGTM but it would be nice if my two comments would be addressed before landing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use common.mustCall instead of called in all of these functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super tiny nit - would you be so kind and upper case all beginnings of comments?
Adds the following tests for tty readline: - go to beginning and end of line - wordLeft - wordRight - deleteWordLeft - deleteWordRight
41f86f9 to 28adcc4Compareclaudiorodriguez commented Sep 9, 2017
@BridgeAR comments addressed, cheers |
BridgeAR commented Sep 20, 2017
BridgeAR commented Sep 23, 2017
Landed in 7a95392 |
Adds the following tests for tty readline: - go to beginning and end of line - wordLeft - wordRight - deleteWordLeft - deleteWordRight PR-URL: #12064 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Adds the following tests for tty readline: - go to beginning and end of line - wordLeft - wordRight - deleteWordLeft - deleteWordRight PR-URL: nodejs/node#12064 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Adds the following tests for tty readline: - go to beginning and end of line - wordLeft - wordRight - deleteWordLeft - deleteWordRight PR-URL: #12064 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Adds the following tests for tty readline: - go to beginning and end of line - wordLeft - wordRight - deleteWordLeft - deleteWordRight PR-URL: #12064 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Adds the following tests for tty readline: - go to beginning and end of line - wordLeft - wordRight - deleteWordLeft - deleteWordRight PR-URL: #12064 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
Adds the following tests for tty readline:
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test