Sitelet https://web.archive.org/web/20220602170309/https://github.com/nodejs/node/pull/42908
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

OneNail
Copy link
Contributor

@OneNail OneNail 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 test labels Apr 29, 2022
@Trott
Copy link
Member

@Trott Trott commented May 2, 2022

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

@OneNail
Copy link
Contributor Author

@OneNail OneNail commented May 17, 2022

@RaisinTen Can help review this pr? thanks~

Copy link
Member

@RaisinTen RaisinTen left a comment

LGTM

@nodejs-github-bot

This comment was marked as outdated.

@RaisinTen RaisinTen added the author ready label May 18, 2022
@OneNail
Copy link
Contributor Author

@OneNail OneNail 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.

@OneNail
Copy link
Contributor Author

@OneNail OneNail commented May 20, 2022

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

@RaisinTen
Copy link
Member

@RaisinTen RaisinTen commented May 20, 2022

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
Contributor

@nodejs-github-bot nodejs-github-bot commented May 22, 2022

@RaisinTen RaisinTen added commit-queue commit-queue-squash labels May 22, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue label May 22, 2022
@nodejs-github-bot nodejs-github-bot merged commit ddd2a18 into nodejs:master May 22, 2022
53 checks passed
@nodejs-github-bot
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot commented May 22, 2022

Landed in ddd2a18

bengl pushed a commit that referenced this issue May 30, 2022
PR-URL: #42908
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
@bengl bengl mentioned this pull request May 31, 2022
juanarbol pushed a commit that referenced this issue May 31, 2022
PR-URL: #42908
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready commit-queue-squash needs-ci test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants