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: test_runner#mock:timers housekeeping/refactoring #55506

Merged

Conversation

badkeyy
Copy link
Contributor

@badkeyy badkeyy commented Oct 23, 2024

Cleanup and refactoring of test-runner-mock-timers (as mentioned in #55375 (review)):

  • Modularized test-runner-mock-timers: Split the existing file into three logically distinct parts:
    - test-runner-mock-timers
    - test-runner-mock-timers-scheduler
    - test-runner-mock-timers-date

  • Replaced hardcoded max timeout values: Removed hardcoded max timeout values in test-runner-mock-timers and replaced them with the max timeout value exposed in internal/timers. The --expose-internals flag was added to test-runner-mock-timers for this purpose.

@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 Issues and PRs related to the tests. labels Oct 23, 2024
@badkeyy badkeyy force-pushed the improvement/mock-timers-test-refactor branch from cd6b3be to e0cbc87 Compare October 23, 2024 20:38
Copy link

codecov bot commented Oct 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.41%. Comparing base (b0ffe9e) to head (15f0613).
Report is 64 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #55506      +/-   ##
==========================================
- Coverage   88.42%   88.41%   -0.01%     
==========================================
  Files         653      654       +1     
  Lines      187498   187552      +54     
  Branches    36100    36084      -16     
==========================================
+ Hits       165791   165833      +42     
+ Misses      14957    14955       -2     
- Partials     6750     6764      +14     

see 48 files with indirect coverage changes

@ovflowd ovflowd 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 Oct 28, 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 28, 2024
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/55506
✔  Done loading data for nodejs/node/pull/55506
----------------------------------- PR info ------------------------------------
Title      test: test_runner#mock:timers housekeeping/refactoring (#55506)
Author     Julian Gassner <julian@gassner.online> (@badkeyy)
Branch     badkeyy:improvement/mock-timers-test-refactor -> nodejs:main
Labels     test, needs-ci, commit-queue-squash
Commits    3
 - test: split up test-runner-mock-timers test
 - test: use timeout_max value for timer mocking test instead of hardcod…
 - test: removed double describe
Committers 1
 - Julian Gassner <julian.gassner@tum.de>
PR-URL: https://github.com/nodejs/node/pull/55506
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/55506
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 23 Oct 2024 20:25:14 GMT
   ✔  Approvals: 3
   ✔  - Erick Wendel (@ErickWendel): https://github.com/nodejs/node/pull/55506#pullrequestreview-2393008154
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/55506#pullrequestreview-2397352614
   ✔  - Claudio Wunder (@ovflowd): https://github.com/nodejs/node/pull/55506#pullrequestreview-2400008659
   ✔  Last GitHub CI successful
   ✘  No Jenkins CI runs detected
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/11561209244

@nodejs-github-bot
Copy link
Collaborator

@ovflowd
Copy link
Member

ovflowd commented Oct 28, 2024

Ah didn't see Jenkins didn't run yet. Just triggered it.

@ovflowd ovflowd 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 30, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 30, 2024
@nodejs-github-bot nodejs-github-bot merged commit 6dea41d into nodejs:main Oct 30, 2024
67 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 6dea41d

Ceres6 pushed a commit to Ceres6/node that referenced this pull request Oct 30, 2024
PR-URL: nodejs#55506
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
RafaelGSS pushed a commit that referenced this pull request Nov 1, 2024
PR-URL: #55506
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
PR-URL: nodejs#55506
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

6 participants