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.

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: refactor readline interface test #31423

Conversation

BridgeAR
Copy link
Member

This refactoring is mainly there to improve the readability of the tests and to reduce code lines.

Multiple tests where misplaced and this is now corrected as well. The misplacement makes the diff not that easy to read but functionality wise it's 100% identical as before.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jan 20, 2020
@BridgeAR
Copy link
Member Author

@nodejs/readline @nodejs/testing PTAL. This file is the most important test file we have for all readline tests.

@Trott
Copy link
Member

Trott commented Jan 23, 2020

Needs a rebase.

This reduces the code lines required to run the tests by abstracting
the constructor call. It also moves tests out of a for loop that
where miss placed.
@BridgeAR BridgeAR force-pushed the 2020-01-20-test-refactor-readline-interface branch from 69c953f to 6a285f3 Compare January 24, 2020 14:22
@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 24, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

BridgeAR added a commit that referenced this pull request Feb 5, 2020
This reduces the code lines required to run the tests by abstracting
the constructor call. It also moves tests out of a for loop that
where miss placed.

PR-URL: #31423
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
BridgeAR added a commit that referenced this pull request Feb 5, 2020
@BridgeAR
Copy link
Member Author

BridgeAR commented Feb 5, 2020

Landed in f295ba5, 1fb4bd1 🎉

@BridgeAR BridgeAR closed this Feb 5, 2020
codebytere pushed a commit that referenced this pull request Feb 17, 2020
This reduces the code lines required to run the tests by abstracting
the constructor call. It also moves tests out of a for loop that
where miss placed.

PR-URL: #31423
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
codebytere pushed a commit that referenced this pull request Feb 17, 2020
@codebytere codebytere mentioned this pull request Feb 17, 2020
@targos targos added backport-requested-v12.x and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-blocked-v12.x labels Apr 25, 2020
@targos
Copy link
Member

targos commented Apr 25, 2020

Would you like to backport this to v12.x? I'm preparing the next minor on this branch: https://github.com/targos/node/commits/prepare-minor

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants