node icon indicating copy to clipboard operation
node copied to clipboard

Published 20 hours ago • verified publisher icon nodejs

test_runner: use validateStringArray for `timers.enable()`

Open deokjinkim opened this issue 1 year ago • 12 comments

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.

deokjinkim avatar Sep 07 '23 14:09 deokjinkim

Review requested:

  • [ ] @nodejs/test_runner

nodejs-github-bot avatar Sep 07 '23 14:09 nodejs-github-bot

cc @ErickWendel PTAL when you are available.

deokjinkim avatar Sep 08 '23 13:09 deokjinkim

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

nodejs-github-bot avatar Sep 08 '23 14:09 nodejs-github-bot

This needs a rebase.

lpinca avatar Jan 27 '24 20:01 lpinca

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

nodejs-github-bot avatar Jan 29 '24 14:01 nodejs-github-bot

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

nodejs-github-bot avatar Jan 30 '24 11:01 nodejs-github-bot

@deokjinkim please do not add https://github.com/nodejs/node/labels/request-ci 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.

aduh95 avatar Jan 30 '24 14:01 aduh95

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

nodejs-github-bot avatar Jan 31 '24 23:01 nodejs-github-bot

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

nodejs-github-bot avatar Feb 02 '24 14:02 nodejs-github-bot

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

nodejs-github-bot avatar Feb 02 '24 19:02 nodejs-github-bot

This needs a rebase.

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

deokjinkim avatar Feb 03 '24 10:02 deokjinkim

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

Thank you for review. I added a test you mentioned :)

deokjinkim avatar May 14 '24 04:05 deokjinkim