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: split promisified timers test for coverage purposes #37943

Merged
merged 1 commit into from
Mar 29, 2021

Conversation

Trott
Copy link
Member

@Trott Trott commented Mar 27, 2021

Because of lazy loading, running promisified timers tests for setTimeout
and setImmediate from the same file means that there is a piece of code
that doesn't get covered. Split into separate files to cover everything.

Refs: https://coverage.nodejs.org/coverage-290c158018ac0277/lib/timers.js.html#L269

@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 Mar 27, 2021
@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member

targos commented Mar 27, 2021

s/proimisifed/promisified/

@Trott Trott changed the title test: split proimisifed timers test for coverage purposes test: split promisifed timers test for coverage purposes Mar 27, 2021
@Trott Trott force-pushed the timers-promisified-coverage branch from 141c41e to cea86a0 Compare March 27, 2021 13:40
@Trott
Copy link
Member Author

Trott commented Mar 27, 2021

s/proimisifed/promisified/

@targos Fixed. Thanks!

@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member

targos commented Mar 27, 2021

s/promisifed/promisified/ ?

@Trott Trott changed the title test: split promisifed timers test for coverage purposes test: split promisified timers test for coverage purposes Mar 28, 2021
@Trott Trott force-pushed the timers-promisified-coverage branch from cea86a0 to 64e076e Compare March 28, 2021 04:17
@Trott
Copy link
Member Author

Trott commented Mar 28, 2021

s/promisifed/promisified/ ?

Yikes. Fixed (again). Thanks.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@RaisinTen RaisinTen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 28, 2021
Because of lazy loading, running promisified timers tests for setTimeout
and setImmediate from the same file means that there is a piece of code
that doesn't get covered. Split into separate files to cover everything.

Refs: https://coverage.nodejs.org/coverage-290c158018ac0277/lib/timers.js.html#L269

PR-URL: nodejs#37943
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@Trott Trott force-pushed the timers-promisified-coverage branch from 64e076e to 52c7539 Compare March 29, 2021 22:56
@Trott
Copy link
Member Author

Trott commented Mar 29, 2021

Landed in 52c7539

@Trott Trott merged commit 52c7539 into nodejs:master Mar 29, 2021
@Trott Trott deleted the timers-promisified-coverage branch March 29, 2021 22:56
ruyadorno pushed a commit that referenced this pull request Mar 30, 2021
Because of lazy loading, running promisified timers tests for setTimeout
and setImmediate from the same file means that there is a piece of code
that doesn't get covered. Split into separate files to cover everything.

Refs: https://coverage.nodejs.org/coverage-290c158018ac0277/lib/timers.js.html#L269

PR-URL: #37943
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@ruyadorno ruyadorno mentioned this pull request Mar 30, 2021
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. 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