Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

test: convert test-debugger-pid to async/await #45179

Merged
merged 3 commits into from
Nov 1, 2022
Merged

test: convert test-debugger-pid to async/await #45179

merged 3 commits into from
Nov 1, 2022

Conversation

lukekarrys
Copy link
Member

This is a small pull request to convert test/sequential/test-debugger-pid.js to async/await. I used some of the other tests to get a sense of the style, and I believe this is a 1-to-1 replacement for the previous behavior, except without the use of Promise.then()

As a meta note, this is also to get familiar with the contributing process in light of #44828.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Oct 25, 2022
@Trott
Copy link
Member

Trott commented Oct 26, 2022

@nodejs/testing

Copy link
Member

@MoLow MoLow left a comment

Choose a reason for hiding this comment

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

LGTM, consider renaming to .mjs to use top level await

@Trott Trott added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Oct 27, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 27, 2022
@nodejs-github-bot
Copy link
Collaborator

@Trott Trott removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 27, 2022
Copy link
Member

@juanarbol juanarbol left a comment

Choose a reason for hiding this comment

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

lgtm

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 28, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 28, 2022
@nodejs-github-bot
Copy link
Collaborator

@theanarkh theanarkh added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 31, 2022
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 31, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/45179
✔  Done loading data for nodejs/node/pull/45179
----------------------------------- PR info ------------------------------------
Title      test: convert test-debugger-pid to async/await (#45179)
Author     Luke Karrys  (@lukekarrys)
Branch     lukekarrys:async-await-test -> nodejs:main
Labels     test, needs-ci
Commits    3
 - test: convert test-debugger-pid to async/await
 - fixup! test: convert test-debugger-pid to async/await
 - Update test/sequential/test-debugger-pid.js
Committers 2
 - Luke Karrys 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/45179
Reviewed-By: Moshe Atlow 
Reviewed-By: Juan José Arboleda 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/45179
Reviewed-By: Moshe Atlow 
Reviewed-By: Juan José Arboleda 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - Update test/sequential/test-debugger-pid.js
   ℹ  This PR was created on Tue, 25 Oct 2022 19:34:17 GMT
   ✔  Approvals: 2
   ✔  - Moshe Atlow (@MoLow): https://github.com/nodejs/node/pull/45179#pullrequestreview-1156524456
   ✔  - Juan José Arboleda (@juanarbol): https://github.com/nodejs/node/pull/45179#pullrequestreview-1160434955
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-10-28T21:32:56Z: https://ci.nodejs.org/job/node-test-pull-request/47540/
- Querying data for job/node-test-pull-request/47540/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/3362941395

@Trott Trott added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 31, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 31, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/45179
✔  Done loading data for nodejs/node/pull/45179
----------------------------------- PR info ------------------------------------
Title      test: convert test-debugger-pid to async/await (#45179)
Author     Luke Karrys  (@lukekarrys)
Branch     lukekarrys:async-await-test -> nodejs:main
Labels     test, needs-ci, commit-queue-failed
Commits    3
 - test: convert test-debugger-pid to async/await
 - fixup! test: convert test-debugger-pid to async/await
 - Update test/sequential/test-debugger-pid.js
Committers 2
 - Luke Karrys 
 - GitHub 
PR-URL: https://github.com/nodejs/node/pull/45179
Reviewed-By: Moshe Atlow 
Reviewed-By: Juan José Arboleda 
Reviewed-By: Rich Trott 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/45179
Reviewed-By: Moshe Atlow 
Reviewed-By: Juan José Arboleda 
Reviewed-By: Rich Trott 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Tue, 25 Oct 2022 19:34:17 GMT
   ✔  Approvals: 3
   ✔  - Moshe Atlow (@MoLow): https://github.com/nodejs/node/pull/45179#pullrequestreview-1156524456
   ✔  - Juan José Arboleda (@juanarbol): https://github.com/nodejs/node/pull/45179#pullrequestreview-1160434955
   ✔  - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/45179#pullrequestreview-1162669393
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-10-31T16:26:07Z: https://ci.nodejs.org/job/node-test-pull-request/47540/
- Querying data for job/node-test-pull-request/47540/
   ✔  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 45179
From https://github.com/nodejs/node
 * branch                  refs/pull/45179/merge -> FETCH_HEAD
✔  Fetched commits as 17ae2ab7509c..c24d335484aa
--------------------------------------------------------------------------------
[main 6e1191818e] test: convert test-debugger-pid to async/await
 Author: Luke Karrys 
 Date: Tue Oct 25 12:26:39 2022 -0700
 1 file changed, 21 insertions(+), 38 deletions(-)
[main 3bd0814972] fixup! test: convert test-debugger-pid to async/await
 Author: Luke Karrys 
 Date: Tue Oct 25 13:06:17 2022 -0700
 1 file changed, 3 insertions(+), 3 deletions(-)
[main adc9684fb7] Update test/sequential/test-debugger-pid.js
 Author: Rich Trott 
 Date: Fri Oct 28 14:28:35 2022 -0700
 1 file changed, 1 insertion(+), 1 deletion(-)
   ✔  Patches applied
There are 3 commits in the PR. Attempting autorebase.
Rebasing (2/5)
Rebasing (3/5)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test: convert test-debugger-pid to async/await

PR-URL: #45179
Reviewed-By: Moshe Atlow moshe@atlow.co.il
Reviewed-By: Juan José Arboleda soyjuanarbol@gmail.com
Reviewed-By: Rich Trott rtrott@gmail.com

[detached HEAD 0038303e05] test: convert test-debugger-pid to async/await
Author: Luke Karrys luke@lukekarrys.com
Date: Tue Oct 25 12:26:39 2022 -0700
1 file changed, 21 insertions(+), 38 deletions(-)
Rebasing (4/5)
Rebasing (5/5)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
Update test/sequential/test-debugger-pid.js

Co-authored-by: Antoine du Hamel duhamelantoine1995@gmail.com
PR-URL: #45179
Reviewed-By: Moshe Atlow moshe@atlow.co.il
Reviewed-By: Juan José Arboleda soyjuanarbol@gmail.com
Reviewed-By: Rich Trott rtrott@gmail.com

[detached HEAD 7804ba5eb3] Update test/sequential/test-debugger-pid.js
Author: Rich Trott rtrott@gmail.com
Date: Fri Oct 28 14:28:35 2022 -0700
1 file changed, 1 insertion(+), 1 deletion(-)

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/3365519222

@Trott Trott added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Nov 1, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 1, 2022
@nodejs-github-bot nodejs-github-bot merged commit 590cf56 into nodejs:main Nov 1, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 590cf56

RafaelGSS pushed a commit that referenced this pull request Nov 1, 2022
PR-URL: #45179
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Nov 1, 2022
lucshi pushed a commit to lucshi/node that referenced this pull request Nov 9, 2022
PR-URL: nodejs#45179
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Nov 10, 2022
PR-URL: #45179
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
PR-URL: #45179
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
PR-URL: #45179
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
PR-URL: #45179
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Jan 9, 2023
codebytere added a commit to electron/electron that referenced this pull request Jan 11, 2023
* chore: bump node in DEPS to v18.13.0

* child_process: validate arguments for null bytes

nodejs/node#44782

* bootstrap: merge main thread and worker thread initializations

nodejs/node#44869

* module: ensure relative requires work from deleted directories

nodejs/node#42384

* src: add support for externally shared js builtins

nodejs/node#44000

* lib: disambiguate `native module` to `binding`

nodejs/node#45673

* test: convert test-debugger-pid to async/await

nodejs/node#45179

* deps: upgrade to libuv 1.44.2

nodejs/node#42340

* src: fix cppgc incompatibility in v8

nodejs/node#43521

* src: use qualified `std::move` call in node_http2

nodejs/node#45555

* build: fix env.h for cpp20

nodejs/node#45516

* test: remove experimental-wasm-threads flag

nodejs/node#45074

* src: iwyu in cleanup_queue.cc

nodejs/node#44983

* src: add missing include for `std::all_of`

nodejs/node#45541

* deps: update ICU to 72.1

nodejs/node#45068

* chore: fixup patch indices

* chore: remove errant semicolons

- nodejs/node#44179
- nodejs/node#44193

* src: add support for externally shared js builtins

nodejs/node#44376

* chore: add missing GN filenames

* deps: update nghttp2 to 1.51.0

nodejs/node#45537

* chore: disable more Node.js snapshot tests

The Snapshot feature is currently disabled

* chore: disable ICU timezone tests

Node.js uses a different version of ICU than Electron so they
will often be out of sync.

* chore: disable threadpool event tracing test

Event tracing is not enabled in embedded Node.js

* chore: fixup patch indices

* chore: comments from review

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
* chore: bump node in DEPS to v18.13.0

* child_process: validate arguments for null bytes

nodejs/node#44782

* bootstrap: merge main thread and worker thread initializations

nodejs/node#44869

* module: ensure relative requires work from deleted directories

nodejs/node#42384

* src: add support for externally shared js builtins

nodejs/node#44000

* lib: disambiguate `native module` to `binding`

nodejs/node#45673

* test: convert test-debugger-pid to async/await

nodejs/node#45179

* deps: upgrade to libuv 1.44.2

nodejs/node#42340

* src: fix cppgc incompatibility in v8

nodejs/node#43521

* src: use qualified `std::move` call in node_http2

nodejs/node#45555

* build: fix env.h for cpp20

nodejs/node#45516

* test: remove experimental-wasm-threads flag

nodejs/node#45074

* src: iwyu in cleanup_queue.cc

nodejs/node#44983

* src: add missing include for `std::all_of`

nodejs/node#45541

* deps: update ICU to 72.1

nodejs/node#45068

* chore: fixup patch indices

* chore: remove errant semicolons

- nodejs/node#44179
- nodejs/node#44193

* src: add support for externally shared js builtins

nodejs/node#44376

* chore: add missing GN filenames

* deps: update nghttp2 to 1.51.0

nodejs/node#45537

* chore: disable more Node.js snapshot tests

The Snapshot feature is currently disabled

* chore: disable ICU timezone tests

Node.js uses a different version of ICU than Electron so they
will often be out of sync.

* chore: disable threadpool event tracing test

Event tracing is not enabled in embedded Node.js

* chore: fixup patch indices

* chore: comments from review

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
gecko19 pushed a commit to brightsign/electron that referenced this pull request Feb 28, 2023
* chore: bump node in DEPS to v18.13.0

* child_process: validate arguments for null bytes

nodejs/node#44782

* bootstrap: merge main thread and worker thread initializations

nodejs/node#44869

* module: ensure relative requires work from deleted directories

nodejs/node#42384

* src: add support for externally shared js builtins

nodejs/node#44000

* lib: disambiguate `native module` to `binding`

nodejs/node#45673

* test: convert test-debugger-pid to async/await

nodejs/node#45179

* deps: upgrade to libuv 1.44.2

nodejs/node#42340

* src: fix cppgc incompatibility in v8

nodejs/node#43521

* src: use qualified `std::move` call in node_http2

nodejs/node#45555

* build: fix env.h for cpp20

nodejs/node#45516

* test: remove experimental-wasm-threads flag

nodejs/node#45074

* src: iwyu in cleanup_queue.cc

nodejs/node#44983

* src: add missing include for `std::all_of`

nodejs/node#45541

* deps: update ICU to 72.1

nodejs/node#45068

* chore: fixup patch indices

* chore: remove errant semicolons

- nodejs/node#44179
- nodejs/node#44193

* src: add support for externally shared js builtins

nodejs/node#44376

* chore: add missing GN filenames

* deps: update nghttp2 to 1.51.0

nodejs/node#45537

* chore: disable more Node.js snapshot tests

The Snapshot feature is currently disabled

* chore: disable ICU timezone tests

Node.js uses a different version of ICU than Electron so they
will often be out of sync.

* chore: disable threadpool event tracing test

Event tracing is not enabled in embedded Node.js

* chore: fixup patch indices

* chore: comments from review

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
gecko19 pushed a commit to brightsign/electron that referenced this pull request Mar 15, 2023
* chore: bump node in DEPS to v18.13.0

* child_process: validate arguments for null bytes

nodejs/node#44782

* bootstrap: merge main thread and worker thread initializations

nodejs/node#44869

* module: ensure relative requires work from deleted directories

nodejs/node#42384

* src: add support for externally shared js builtins

nodejs/node#44000

* lib: disambiguate `native module` to `binding`

nodejs/node#45673

* test: convert test-debugger-pid to async/await

nodejs/node#45179

* deps: upgrade to libuv 1.44.2

nodejs/node#42340

* src: fix cppgc incompatibility in v8

nodejs/node#43521

* src: use qualified `std::move` call in node_http2

nodejs/node#45555

* build: fix env.h for cpp20

nodejs/node#45516

* test: remove experimental-wasm-threads flag

nodejs/node#45074

* src: iwyu in cleanup_queue.cc

nodejs/node#44983

* src: add missing include for `std::all_of`

nodejs/node#45541

* deps: update ICU to 72.1

nodejs/node#45068

* chore: fixup patch indices

* chore: remove errant semicolons

- nodejs/node#44179
- nodejs/node#44193

* src: add support for externally shared js builtins

nodejs/node#44376

* chore: add missing GN filenames

* deps: update nghttp2 to 1.51.0

nodejs/node#45537

* chore: disable more Node.js snapshot tests

The Snapshot feature is currently disabled

* chore: disable ICU timezone tests

Node.js uses a different version of ICU than Electron so they
will often be out of sync.

* chore: disable threadpool event tracing test

Event tracing is not enabled in embedded Node.js

* chore: fixup patch indices

* chore: comments from review

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
gecko19 pushed a commit to brightsign/electron that referenced this pull request Mar 15, 2023
* chore: bump node in DEPS to v18.13.0

* child_process: validate arguments for null bytes

nodejs/node#44782

* bootstrap: merge main thread and worker thread initializations

nodejs/node#44869

* module: ensure relative requires work from deleted directories

nodejs/node#42384

* src: add support for externally shared js builtins

nodejs/node#44000

* lib: disambiguate `native module` to `binding`

nodejs/node#45673

* test: convert test-debugger-pid to async/await

nodejs/node#45179

* deps: upgrade to libuv 1.44.2

nodejs/node#42340

* src: fix cppgc incompatibility in v8

nodejs/node#43521

* src: use qualified `std::move` call in node_http2

nodejs/node#45555

* build: fix env.h for cpp20

nodejs/node#45516

* test: remove experimental-wasm-threads flag

nodejs/node#45074

* src: iwyu in cleanup_queue.cc

nodejs/node#44983

* src: add missing include for `std::all_of`

nodejs/node#45541

* deps: update ICU to 72.1

nodejs/node#45068

* chore: fixup patch indices

* chore: remove errant semicolons

- nodejs/node#44179
- nodejs/node#44193

* src: add support for externally shared js builtins

nodejs/node#44376

* chore: add missing GN filenames

* deps: update nghttp2 to 1.51.0

nodejs/node#45537

* chore: disable more Node.js snapshot tests

The Snapshot feature is currently disabled

* chore: disable ICU timezone tests

Node.js uses a different version of ICU than Electron so they
will often be out of sync.

* chore: disable threadpool event tracing test

Event tracing is not enabled in embedded Node.js

* chore: fixup patch indices

* chore: comments from review

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants