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-rebase https://stackoverflow.com/a/30194975 https://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 <43253032+shockerqt@users.noreply.github.com>
 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 moshe@atlow.co.il
[detached HEAD 34e98d1da4] test_runner: fixed test shorthands return type

Author: Shocker 43253032+shockerqt@users.noreply.github.com

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 moshe@atlow.co.il
[detached HEAD 6de8d8de15] test_runner: added tests for test shorthands

Author: shocker fmju96@gmail.com

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 moshe@atlow.co.il
[detached HEAD 4d4c40489d] test_runner: change tests to use describe/it

Author: shocker fmju96@gmail.com

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