test_runner: todo, only, skip are expected to return a Promise

[shockerqt]

todo, only and skip are shorthands for test, so all of them should return the same type.

Currently todo() returns undefined while test({todo: true }) returns a Promise that resolves to undefined.

[nodejs-github-bot]

Review requested:

• @nodejs/test_runner

[MoLow]

The implementation looks ok, can you add a test, or modify existing tests?

[shockerqt]

I added a test, I'm not really sure if the structure is correct

[atlowChemi]

@shockerqt FYI - The first commit has to be prefixed with test_runner: 🙏🏽

node/doc/contributing/pull-requests.md

Lines 158 to 175 in 821c487

	#### Commit message guidelines
	A good commit message should describe what changed and why.
	1. The first line should:
	* contain a short description of the change (preferably 50 characters or
	less, and no more than 72 characters)
	* be entirely in lowercase with the exception of proper nouns, acronyms, and
	the words that refer to code, like function/variable names
	* be prefixed with the name of the changed [subsystem](#appendix-subsystems)
	and start with an imperative verb. Check the output of `git log --oneline
	files/you/changed` to find out what subsystems your changes touch.
	Examples:
	* `net: add localAddress and localPort to Socket`
	* `src: fix typos in async_wrap.h`

[shockerqt]

I don't know if there is a way to edit the first commit, I read the guidelines too late 😔

[atlowChemi]

I don't know if there is a way to edit the first commit, I read the guidelines too late 😔

There might be another solution I am not aware of, but you could rebase, and reword the first commit:
https://git-scm.com/docs/git-rebase
https://stackoverflow.com/a/30194975
https://coderwall.com/p/_thg9a/reword-a-git-commit-message

[shockerqt]

I don't know if there is a way to edit the first commit, I read the guidelines too late 😔

There might be another solution I am not aware of, but you could rebase, and reword the first commit: https://git-scm.com/docs/git-rebasehttps://stackoverflow.com/a/30194975https://coderwall.com/p/_thg9a/reword-a-git-commit-message

Thanks! I managed to fix the message of the first commit

[atlowChemi]

LGTM

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/48555 ✔ Done loading data for nodejs/node/pull/48555 ----------------------------------- PR info ------------------------------------ Title test_runner: todo, only, skip are expected to return a Promise (#48555) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch shockerqt:main -> nodejs:main Labels author ready, needs-ci, test_runner Commits 3 - test_runner: fixed `test` shorthands return type - test_runner: added tests for test shorthands - test_runner: change tests to use `describe/it` Committers 1 - shocker PR-URL: https://github.com/nodejs/node/pull/48555 Reviewed-By: Moshe Atlow ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/48555 Reviewed-By: Moshe Atlow -------------------------------------------------------------------------------- ℹ This PR was created on Mon, 26 Jun 2023 04:10:55 GMT ✔ Approvals: 1 ✔ - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/48555#pullrequestreview-1500033494 ✘ This PR needs to wait 20 more hours to land (or 0 hours if there is one more approval) ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-07-02T06:53:42Z: https://ci.nodejs.org/job/node-test-pull-request/52583/ - Querying data for job/node-test-pull-request/52583/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu

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

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/48555 ✔ Done loading data for nodejs/node/pull/48555 ----------------------------------- PR info ------------------------------------ Title test_runner: todo, only, skip are expected to return a Promise (#48555) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch shockerqt:main -> nodejs:main Labels author ready, needs-ci, test_runner Commits 3 - test_runner: fixed `test` shorthands return type - test_runner: added tests for test shorthands - test_runner: change tests to use `describe/it` Committers 1 - shocker PR-URL: https://github.com/nodejs/node/pull/48555 Reviewed-By: Moshe Atlow ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/48555 Reviewed-By: Moshe Atlow -------------------------------------------------------------------------------- ℹ This PR was created on Mon, 26 Jun 2023 04:10:55 GMT ✔ Approvals: 1 ✔ - Moshe Atlow (@MoLow) (TSC): https://github.com/nodejs/node/pull/48555#pullrequestreview-1500033494 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-07-02T07:59:05Z: https://ci.nodejs.org/job/node-test-pull-request/52583/ - Querying data for job/node-test-pull-request/52583/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 48555 From https://github.com/nodejs/node * branch refs/pull/48555/merge -> FETCH_HEAD ✔ Fetched commits as 1aabfa8732fb..cf59fa00fb6c -------------------------------------------------------------------------------- [main b350b9bf63] test_runner: fixed `test` shorthands return type Author: Shocker <[email protected]> Date: Sun Jun 25 23:47:06 2023 -0400 1 file changed, 1 insertion(+), 3 deletions(-) [main 8dd5fc58ea] test_runner: added tests for test shorthands Author: shocker Date: Mon Jun 26 07:20:14 2023 -0400 1 file changed, 34 insertions(+) create mode 100644 test/parallel/test-runner-typechecking.js [main 1aa8125728] test_runner: change tests to use `describe/it` Author: shocker Date: Mon Jun 26 21:07:55 2023 -0400 1 file changed, 26 insertions(+), 24 deletions(-) ✔ Patches applied There are 3 commits in the PR. Attempting autorebase. Rebasing (2/6) 
Executing: git node land --amend --yes
 --------------------------------- New Message ----------------------------------
 test_runner: fixed test shorthands return type
test.todo, test.only and test.skip are expected to return the
 same as test. This commit corrects the inconsistent behavior of
 these shorthands.
Fixes: #48557
 PR-URL: #48555
 Reviewed-By: Moshe Atlow [email protected]
[detached HEAD 34e98d1da4] test_runner: fixed test shorthands return type
 Author: Shocker [email protected]
 Date: Sun Jun 25 23:47:06 2023 -0400
 1 file changed, 1 insertion(+), 3 deletions(-)
 Rebasing (3/6)
 Rebasing (4/6)
Executing: git node land --amend --yes
 --------------------------------- New Message ----------------------------------
 test_runner: added tests for test shorthands
Added test to check that the return type of the todo, only and skip shorthands
 are consistent with the return type of test.
PR-URL: #48555
 Reviewed-By: Moshe Atlow [email protected]
[detached HEAD 6de8d8de15] test_runner: added tests for test shorthands
 Author: shocker [email protected]
 Date: Mon Jun 26 07:20:14 2023 -0400
 1 file changed, 34 insertions(+)
 create mode 100644 test/parallel/test-runner-typechecking.js
 Rebasing (5/6)
 Rebasing (6/6)
Executing: git node land --amend --yes
 --------------------------------- New Message ----------------------------------
 test_runner: change tests to use describe/it
PR-URL: #48555
 Reviewed-By: Moshe Atlow [email protected]
[detached HEAD 4d4c40489d] test_runner: change tests to use describe/it
 Author: shocker [email protected]
 Date: Mon Jun 26 21:07:55 2023 -0400
 1 file changed, 26 insertions(+), 24 deletions(-)
Successfully rebased and updated refs/heads/main.
ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

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

[nodejs-github-bot]

Landed in d312bd9