GeezerVison

View reviewed changes

atlowChemi added request-ci

Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review.

commit-queue-squash

PRs the Commit Queue should land as one squashed commit.

author ready PRs with CI started, the required approvals, and no outstanding review comments. labels

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

CI: https://ci.nodejs.org/job/node-test-pull-request/55140/

nodejs-github-bot commented Oct 23, 2023

Copy link

Copy Markdown

Collaborator

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mika-fischer force-pushed the fix-50307 branch 2 times, most recently from 02d7e2e to e65beea Compare

October 23, 2023 08:41

MoLow approved these changes

View reviewed changes

MoLow added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

CI: https://ci.nodejs.org/job/node-test-pull-request/55143/

nodejs-github-bot commented Oct 23, 2023

Copy link

Copy Markdown

Collaborator

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ErickWendel approved these changes

View reviewed changes

ErickWendel left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM. @mika-fischer congrats on your first PR on the nodejs repo!! It's really helpful!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

MoLow, atlowChemi, and GeoffreyBooth reacted with rocket emoji

All reactions

🚀 3 reactions

MoLow commented Oct 23, 2023

Copy link

Copy Markdown

Member

@mika-fischer the PR needs a rebase

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mika-fischer added 3 commits

October 23, 2023 14:24


          test_runner: test return value of mocked promisified timers

ea2a31e


          test_runner: fix mock timer promisified setInterval return value

88e13a7

promisified setInterval can be passed a value that is returned by the
async iterable. The mocked promisified setInterval used this value for
its own purposes. This brings the mocked version in line with the
original.

Fixes: nodejs#50307


          test_runner: fix mock timer setInterval tests

06d8b9d

Some tests relied on the custom return value and were broken by the
change to always return the value provided by the caller.

mika-fischer force-pushed the fix-50307 branch from e65beea to 06d8b9d Compare

October 23, 2023 12:29

mika-fischer commented Oct 23, 2023

Copy link

Copy Markdown

Contributor Author

LGTM. @mika-fischer congrats on your first PR on the nodejs repo!! It's really helpful!

Thanks @ErickWendel!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mika-fischer commented Oct 23, 2023

Copy link

Copy Markdown

Contributor Author

@mika-fischer the PR needs a rebase

done @MoLow

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

MoLow added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

CI: https://ci.nodejs.org/job/node-test-pull-request/55149/

nodejs-github-bot commented Oct 23, 2023

Copy link

Copy Markdown

Collaborator

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mika-fischer commented Oct 23, 2023

Copy link

Copy Markdown

Contributor Author

I'm not sure why some tests are failing, these seem unrelated to the changes in this PR. Let me know if there's anything I can do to move this along.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nodejs-github-bot commented Oct 23, 2023

Copy link

Copy Markdown

Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/55158/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

MoLow added the commit-queue PRs queued for automated landing through the Commit Queue. label

github-actions Bot mentioned this pull request

CI Reliability 2023-10-24 nodejs/reliability#696

Open

27 tasks

mika-fischer mentioned this pull request

test_runner: mock timer promisified setTimeout cleanup goes wrong losing some timers #50365

Closed

nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels

https://github.com/nodejs/node/actions/runs/6632107907

nodejs-github-bot commented Oct 24, 2023

Copy link

Copy Markdown

Collaborator

Commit Queue failed

- Loading data for nodejs/node/pull/50331
✔  Done loading data for nodejs/node/pull/50331
----------------------------------- PR info ------------------------------------
Title      test_runner: Fix mock timer promisified setInterval return value (#50331)
Author     Mika Fischer  (@mika-fischer, first-time contributor)
Branch     mika-fischer:fix-50307 -> nodejs:main
Labels     author ready, needs-ci, commit-queue-squash, test_runner
Commits    3
 - test_runner: test return value of mocked promisified timers
 - test_runner: fix mock timer promisified setInterval return value
 - test_runner: fix mock timer setInterval tests
Committers 1
 - Mika Fischer 
PR-URL: https://github.com/nodejs/node/pull/50331
Reviewed-By: Moshe Atlow 
Reviewed-By: Chemi Atlow 
Reviewed-By: Erick Wendel 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/50331
Reviewed-By: Moshe Atlow 
Reviewed-By: Chemi Atlow 
Reviewed-By: Erick Wendel 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - test_runner: test return value of mocked promisified timers
   ⚠  - test_runner: fix mock timer promisified setInterval return value
   ⚠  - test_runner: fix mock timer setInterval tests
   ℹ  This PR was created on Sun, 22 Oct 2023 20:11:55 GMT
   ✔  Approvals: 3
   ✔  - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/50331#pullrequestreview-1692229637
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/50331#pullrequestreview-1691892475
   ✔  - Erick Wendel (@erickwendel): https://github.com/nodejs/node/pull/50331#pullrequestreview-1692276984
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-10-23T18:44:37Z: https://ci.nodejs.org/job/node-test-pull-request/55158/
- Querying data for job/node-test-pull-request/55158/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

benjamingr approved these changes

View reviewed changes

github-actions Bot mentioned this pull request

CI Reliability 2023-10-25 nodejs/reliability#697

Open

16 tasks

MoLow added commit-queue PRs queued for automated landing through the Commit Queue. and removed commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. labels

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label

nodejs-github-bot merged commit 4f5db8b into nodejs:main

nodejs-github-bot commented Oct 25, 2023

Copy link

Copy Markdown

Collaborator

Landed in 4f5db8b

mika-fischer reacted with hooray emoji

All reactions

🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mika-fischer deleted the fix-50307 branch

October 25, 2023 07:29

mika-fischer commented Oct 25, 2023

Copy link

Copy Markdown

Contributor Author

Did the closure of #50307 get dropped somehow? Should I have specified it differently? Should I close the issue now manually?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

MoLow linked an issue