test_runner: use validateStringArray for `timers.enable()` by deokjinkim · Pull Request #49534 · nodejs/node

deokjinkim · 2023-09-07T14:23:37Z

apis which is argument of timers.enable() is string array. So use validatStringArray instead of validateArray. And
options is optional, so update JSDoc.
In document, some default value of timers are missed such as setImmediate and clearImmediate. Next, milliseconds which is argument of `timers.tick() is optional and default is 1.

nodejs-github-bot · 2023-09-07T14:23:42Z

Review requested:

@nodejs/test_runner

aduh95 · 2023-09-07T15:41:19Z

    }

-    validateArray(timers, 'timers');
+    validateStringArray(timers, 'timers');


I assume this is semver-major, correct? If so, can you please take it as its own PR?

@aduh95 the MockTimers is currently experimental, I don't think we have to mark this as semver-major PRs that contain breaking changes and should be released in the next major version.

node/doc/api/test.md

Lines 1560 to 1568 in c86e700

## Class: `MockTimers`



> Stability: 1 - Experimental

Actually I'm not sure this PR is semver-major or not. How about checking jenkins-ci's result according to below collaborator guide?

node/doc/contributing/collaborator-guide.md

Lines 901 to 908 in 7bf29b5

* `semver-{minor,major}`

* be conservative – that is, if a change has the remote _chance_ of breaking

something, go for semver-major

* when adding a semver label, add a comment explaining why you're adding it

* minor vs. patch: roughly: "does it add a new method / does it add a new

section to the docs"

* major vs. everything else: run last versions tests against this version, if

they pass, **probably** minor or patch

deokjinkim · 2023-09-08T13:54:29Z

cc @ErickWendel PTAL when you are available.

nodejs-github-bot · 2023-09-08T14:10:26Z

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

lpinca · 2024-01-27T20:19:15Z

This needs a rebase.

nodejs-github-bot · 2024-01-29T14:23:52Z

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

nodejs-github-bot · 2024-01-30T11:23:39Z

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

aduh95 · 2024-01-30T14:54:11Z

@deokjinkim please do not add request-ci Add this label to start a Jenkins CI on a PR. label on PRs that already have a running CI, it adds up unnecessary load to the CI infrastructure, and won't make the PR land sooner. At the very least, please cancel the running CI first.

nodejs-github-bot · 2024-01-31T23:29:34Z

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

nodejs-github-bot · 2024-02-02T14:50:24Z

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

nodejs-github-bot · 2024-02-02T19:19:22Z

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

deokjinkim · 2024-02-03T10:35:25Z

This needs a rebase.

@lpinca I rebased this PR. Could you review again?

aduh95

Can you add a test that's failing on main and passing on this branch please?

nodejs-github-bot · 2024-08-16T17:38:55Z

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

nodejs-github-bot · 2024-08-19T02:35:12Z

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

nodejs-github-bot · 2024-08-19T03:40:52Z

Landed in 4f94397

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 Sep 7, 2023

aduh95 reviewed Sep 7, 2023

View reviewed changes

atlowChemi reviewed Sep 7, 2023

View reviewed changes

Comment thread doc/api/test.md Outdated

deokjinkim added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 8, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 8, 2023

This was referenced Sep 9, 2023

CI Reliability 2023-09-09 nodejs/reliability#660

Open

CI Reliability 2023-09-10 nodejs/reliability#661

Open

CI Reliability 2023-09-12 nodejs/reliability#663

Open

lpinca approved these changes Jan 27, 2024

View reviewed changes

deokjinkim force-pushed the 230907_revise_mock_timers branch 2 times, most recently from ba15590 to 40e1c33 Compare January 29, 2024 14:16

deokjinkim added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 29, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 29, 2024

deokjinkim added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 30, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 30, 2024

aduh95 reviewed May 11, 2024

View reviewed changes

Comment thread doc/api/test.md

MoLow approved these changes May 14, 2024

View reviewed changes

Lxxyx approved these changes May 22, 2024

View reviewed changes

cjihrig added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 16, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 16, 2024

benjamingr approved these changes Aug 17, 2024

View reviewed changes

cjihrig added commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Aug 19, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 19, 2024

nodejs-github-bot merged commit 4f94397 into nodejs:main Aug 19, 2024

RafaelGSS mentioned this pull request Aug 19, 2024

v22.7.0 proposal #54452

Merged

targos mentioned this pull request Sep 30, 2024

v20.18.0 release proposal #55170

Merged

	## Class: `MockTimers`

	<!-- YAML
	added:
	- v20.4.0
	-->

	> Stability: 1 - Experimental

	* `semver-{minor,major}`
	* be conservative – that is, if a change has the remote _chance_ of breaking
	something, go for semver-major
	* when adding a semver label, add a comment explaining why you're adding it
	* minor vs. patch: roughly: "does it add a new method / does it add a new
	section to the docs"
	* major vs. everything else: run last versions tests against this version, if
	they pass, probably minor or patch

Uh oh!

Conversation

deokjinkim commented Sep 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 7, 2023

Uh oh!

aduh95 Sep 7, 2023

Choose a reason for hiding this comment

Uh oh!

atlowChemi Sep 7, 2023

Choose a reason for hiding this comment

Uh oh!

deokjinkim Sep 8, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

deokjinkim commented Sep 8, 2023

Uh oh!

nodejs-github-bot commented Sep 8, 2023

Uh oh!

lpinca commented Jan 27, 2024

Uh oh!

nodejs-github-bot commented Jan 29, 2024

Uh oh!

nodejs-github-bot commented Jan 30, 2024

Uh oh!

aduh95 commented Jan 30, 2024

Uh oh!

nodejs-github-bot commented Jan 31, 2024

Uh oh!

nodejs-github-bot commented Feb 2, 2024

Uh oh!

nodejs-github-bot commented Feb 2, 2024

Uh oh!

deokjinkim commented Feb 3, 2024

Uh oh!

aduh95 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 16, 2024

Uh oh!

nodejs-github-bot commented Aug 19, 2024

Uh oh!

nodejs-github-bot commented Aug 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

deokjinkim commented Sep 7, 2023 •

edited

Loading