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: improve readline/emitKeypressEvents.js coverage #42908

Merged
merged 2 commits into from
May 22, 2022

Conversation

y1d7ng
Copy link
Contributor

@y1d7ng y1d7ng commented Apr 29, 2022

The previous test case did not cover the case where the stream add keypress listener before emitKeypressEvents and listens first and then removes the keypress listener.

Refer: https://coverage.nodejs.org/coverage-68fb0bf553e2af3e/lib/internal/readline/emitKeypressEvents.js.html

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Apr 29, 2022
@Trott
Copy link
Member

Trott commented May 2, 2022

@nodejs/testing @nodejs/readline This pull request needs some reviews.

@y1d7ng
Copy link
Contributor Author

y1d7ng commented May 17, 2022

@RaisinTen Can help review this pr? thanks~

Copy link
Contributor

@RaisinTen RaisinTen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nodejs-github-bot

This comment was marked as outdated.

@RaisinTen RaisinTen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 18, 2022
@y1d7ng
Copy link
Contributor Author

y1d7ng commented May 19, 2022

need to run CI again?

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@y1d7ng
Copy link
Contributor Author

y1d7ng commented May 20, 2022

CI seems to have failed several times, do I need to change anything?

@RaisinTen
Copy link
Contributor

Nope, no additional change required. Will rerun the build!

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@RaisinTen RaisinTen added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels May 22, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 22, 2022
@nodejs-github-bot nodejs-github-bot merged commit ddd2a18 into nodejs:master May 22, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in ddd2a18

bengl pushed a commit that referenced this pull request May 30, 2022
PR-URL: #42908
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
@bengl bengl mentioned this pull request May 31, 2022
juanarbol pushed a commit that referenced this pull request May 31, 2022
PR-URL: #42908
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
PR-URL: #42908
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
targos pushed a commit that referenced this pull request Jul 12, 2022
PR-URL: #42908
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
targos pushed a commit that referenced this pull request Jul 31, 2022
PR-URL: #42908
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
PR-URL: nodejs/node#42908
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants