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_runner: add mock timers for nodeTimers.promises.scheduler.wait #55244

Conversation

ErickWendel
Copy link
Member

add mock timers for nodeTimers.promises.scheduler.wait

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Oct 3, 2024
@ErickWendel ErickWendel force-pushed the test-runner/mock-timers-add-scheduler-wait branch 2 times, most recently from 559ff1f to c240668 Compare October 3, 2024 01:07
@ErickWendel ErickWendel self-assigned this Oct 3, 2024
@RedYetiDev RedYetiDev added the timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. label Oct 3, 2024
@RedYetiDev
Copy link
Member

@ErickWendel I've unassigned you from the PR as there's really no need for assignments, you're the author.

@RedYetiDev RedYetiDev added the wip Issues and PRs that are still a work in progress. label Oct 3, 2024
@ErickWendel ErickWendel force-pushed the test-runner/mock-timers-add-scheduler-wait branch from c240668 to efbe21f Compare October 12, 2024 17:52
@ErickWendel ErickWendel marked this pull request as ready for review October 12, 2024 18:54
@nodejs nodejs deleted a comment from RedYetiDev Oct 12, 2024
@nodejs nodejs deleted a comment from RedYetiDev Oct 12, 2024
@nodejs nodejs deleted a comment from RedYetiDev Oct 12, 2024
Copy link

codecov bot commented Oct 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.41%. Comparing base (89a2f56) to head (231f650).
Report is 106 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #55244      +/-   ##
==========================================
+ Coverage   88.39%   88.41%   +0.02%     
==========================================
  Files         652      652              
  Lines      186565   186881     +316     
  Branches    36046    36065      +19     
==========================================
+ Hits       164916   165237     +321     
+ Misses      14908    14891      -17     
- Partials     6741     6753      +12     
Files with missing lines Coverage Δ
lib/internal/test_runner/mock/mock_timers.js 98.74% <100.00%> (+0.03%) ⬆️

... and 92 files with indirect coverage changes

@atlowChemi atlowChemi added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 13, 2024
@cjihrig
Copy link
Contributor

cjihrig commented Oct 13, 2024

Does this need docs?

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 13, 2024
@nodejs-github-bot
Copy link
Collaborator

@ErickWendel
Copy link
Member Author

Does this need docs?

I don't think so. In the current docs just say that all timers from node:timers are covered (in my initial code I put the list, but it was updated after removing it)

This adds support for nodetimers.promises.scheduler.wait on Mocktimers

Refs: nodejs#55244
@ErickWendel ErickWendel force-pushed the test-runner/mock-timers-add-scheduler-wait branch from efbe21f to 231f650 Compare October 14, 2024 12:57
@ErickWendel
Copy link
Member Author

just renamed the commit according to the standards

@ErickWendel ErickWendel added request-ci Add this label to start a Jenkins CI on a PR. and removed wip Issues and PRs that are still a work in progress. labels Oct 14, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 14, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@mcollina mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 15, 2024
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 15, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/55244
✔  Done loading data for nodejs/node/pull/55244
----------------------------------- PR info ------------------------------------
Title      test_runner: add mock timers for nodeTimers.promises.scheduler.wait (#55244)
Author     Erick Wendel <erick.workspace@gmail.com> (@ErickWendel)
Branch     ErickWendel:test-runner/mock-timers-add-scheduler-wait -> nodejs:main
Labels     timers, needs-ci, test_runner
Commits    1
 - test_runner: add support for scheduler.wait on mock timers
Committers 1
 - Erick Wendel <erick.workspace@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/55244
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/55244
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - test_runner: add support for scheduler.wait on mock timers
   ℹ  This PR was created on Thu, 03 Oct 2024 01:05:28 GMT
   ✔  Approvals: 3
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/55244#pullrequestreview-2364320039
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/55244#pullrequestreview-2364963382
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/55244#pullrequestreview-2364971775
   ✘  Last GitHub CI failed
   ℹ  Last Full PR CI on 2024-10-14T19:24:25Z: https://ci.nodejs.org/job/node-test-pull-request/63102/
- Querying data for job/node-test-pull-request/63102/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/11344079867

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 15, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 15, 2024
@nodejs-github-bot
Copy link
Collaborator

@mcollina mcollina added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Oct 15, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 15, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/55244
✔  Done loading data for nodejs/node/pull/55244
----------------------------------- PR info ------------------------------------
Title      test_runner: add mock timers for nodeTimers.promises.scheduler.wait (#55244)
Author     Erick Wendel <erick.workspace@gmail.com> (@ErickWendel)
Branch     ErickWendel:test-runner/mock-timers-add-scheduler-wait -> nodejs:main
Labels     timers, needs-ci, test_runner
Commits    1
 - test_runner: add support for scheduler.wait on mock timers
Committers 1
 - Erick Wendel <erick.workspace@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/55244
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/55244
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - test_runner: add support for scheduler.wait on mock timers
   ℹ  This PR was created on Thu, 03 Oct 2024 01:05:28 GMT
   ✔  Approvals: 3
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/55244#pullrequestreview-2364320039
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/55244#pullrequestreview-2364963382
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/55244#pullrequestreview-2364971775
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-10-15T15:33:07Z: https://ci.nodejs.org/job/node-test-pull-request/63118/
- Querying data for job/node-test-pull-request/63118/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/11350662769

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Oct 15, 2024
@RafaelGSS RafaelGSS added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Oct 15, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 15, 2024
@nodejs-github-bot nodejs-github-bot merged commit 2545b9e into nodejs:main Oct 15, 2024
79 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 2545b9e

aduh95 pushed a commit that referenced this pull request Oct 19, 2024
This adds support for nodetimers.promises.scheduler.wait on Mocktimers

Refs: #55244
PR-URL: #55244
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
@aduh95 aduh95 mentioned this pull request Oct 24, 2024
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
This adds support for nodetimers.promises.scheduler.wait on Mocktimers

Refs: nodejs#55244
PR-URL: nodejs#55244
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants