http2: improve tests and docs #42858

daeyeon · 2022-04-25T02:42:16Z

This commit documents the event parameters and http2stream.respond, and
adds some tests to ensure the actual behaviors are aligned with the docs.

Testing the Http2Server.sessionError event is added by updating
test/parallel/test-http2-options-max-headers-exceeds-nghttp2.js.
The event seemingly has not been tested so far.

ServerHttp2Session is exported to validate the session event and the
sessionError event.

nodejs-github-bot · 2022-04-25T02:42:20Z

Review requested:

@nodejs/http
@nodejs/http2
@nodejs/net

test/parallel/test-http2-options-max-headers-exceeds-nghttp2.js

mcollina

lgtm

ShogunPanda

LGTM!

RafaelGSS

LGTM.

daeyeon · 2022-05-14T14:56:58Z

@mcollina @ShogunPanda @RafaelGSS This PR needs a Jenkins CI test, doesn't it? Can someone get it started? Thank you!

nodejs-github-bot · 2022-05-14T16:56:08Z

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

daeyeon · 2022-05-18T16:26:30Z

Need a rerun of the Jenkins CI. The failures are seemingly unrelated to this PR.

nodejs-github-bot · 2022-05-18T20:08:32Z

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

nodejs-github-bot · 2022-05-23T14:28:46Z

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

daeyeon · 2022-05-24T07:26:04Z

Pls help me re-run the CI.

Timeout: test.parallel/test-inspector-multisession-ws (https://ci.nodejs.org/job/node-test-commit-linuxone/nodes=rhel8-s390x/32193/)

nodejs-github-bot · 2022-05-24T07:42:37Z

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

nodejs-github-bot · 2022-05-24T11:09:22Z

Commit Queue failed

- Loading data for nodejs/node/pull/42858
✔  Done loading data for nodejs/node/pull/42858
----------------------------------- PR info ------------------------------------
Title      http2: improve tests and docs (#42858)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     daeyeon:master.doc-220425.Mon.5754 -> nodejs:master
Labels     doc, http2, commit-queue-squash
Commits    3
 - http2: improve tests and docs
 - fix: add client.on('error') and drop :status
 - test: update test-http2-server-sessionerror.js
Committers 1
 - Daeyeon Jeong 
PR-URL: https://github.com/nodejs/node/pull/42858
Reviewed-By: Matteo Collina 
Reviewed-By: Paolo Insogna 
Reviewed-By: Rafael Gonzaga 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/42858
Reviewed-By: Matteo Collina 
Reviewed-By: Paolo Insogna 
Reviewed-By: Rafael Gonzaga 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - http2: improve tests and docs
   ⚠  - fix: add client.on('error') and drop :status
   ⚠  - test: update test-http2-server-sessionerror.js
   ℹ  This PR was created on Mon, 25 Apr 2022 02:42:16 GMT
   ✔  Approvals: 3
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/42858#pullrequestreview-952377018
   ✔  - Paolo Insogna (@ShogunPanda): https://github.com/nodejs/node/pull/42858#pullrequestreview-953188072
   ✔  - Rafael Gonzaga (@RafaelGSS): https://github.com/nodejs/node/pull/42858#pullrequestreview-953315250
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-05-24T07:42:37Z: https://ci.nodejs.org/job/node-test-pull-request/44145/
- Querying data for job/node-test-pull-request/44145/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

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

mcollina

lgtm

nodejs-github-bot · 2022-05-24T11:29:57Z

Landed in 714e2d7

daeyeon · 2022-05-24T11:41:38Z

Thanks!! @mcollina @RafaelGSS @ShogunPanda

This commit documents the event parameters and `http2stream.respond`, and adds some tests to ensure the actual behaviors are aligned with the docs. Testing the 'Http2Server.sessionError' event is added by updating `test/parallel/test-http2-options-max-headers-exceeds-nghttp2.js`. The event seemingly has not been tested so far. `ServerHttp2Session` is exported to validate the `session` event and the `sessionError` event. Signed-off-by: Daeyeon Jeong [email protected] PR-URL: #42858 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>

This commit documents the event parameters and `http2stream.respond`, and adds some tests to ensure the actual behaviors are aligned with the docs. Testing the 'Http2Server.sessionError' event is added by updating `test/parallel/test-http2-options-max-headers-exceeds-nghttp2.js`. The event seemingly has not been tested so far. `ServerHttp2Session` is exported to validate the `session` event and the `sessionError` event. Signed-off-by: Daeyeon Jeong [email protected] PR-URL: nodejs/node#42858 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Paolo Insogna <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>

nodejs-github-bot added doc Issues and PRs related to the documentations. http2 Issues or PRs related to the http2 subsystem. labels Apr 25, 2022

daeyeon force-pushed the master.doc-220425.Mon.5754 branch 2 times, most recently from 870e7a5 to 31dd9d6 Compare April 25, 2022 06:00

daeyeon changed the title ~~doc: improve doc for http2~~ http2: improve tests and docs Apr 25, 2022

daeyeon force-pushed the master.doc-220425.Mon.5754 branch from 31dd9d6 to eda399a Compare April 25, 2022 09:06

RafaelGSS reviewed Apr 25, 2022

View reviewed changes

test/parallel/test-http2-options-max-headers-exceeds-nghttp2.js Show resolved Hide resolved

mcollina approved these changes Apr 25, 2022

View reviewed changes

ShogunPanda approved these changes Apr 26, 2022

View reviewed changes

RafaelGSS approved these changes Apr 26, 2022

View reviewed changes

daeyeon force-pushed the master.doc-220425.Mon.5754 branch 7 times, most recently from 4ba9b20 to f5120d0 Compare April 28, 2022 00:40

RafaelGSS added the request-ci Add this label to start a Jenkins CI on a PR. label May 14, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 14, 2022

This was referenced May 15, 2022

CI Reliability 2022-05-15 nodejs/reliability#278

Open

CI Reliability 2022-05-16 nodejs/reliability#279

Open

CI Reliability 2022-05-17 nodejs/reliability#280

Open

CI Reliability 2022-05-18 nodejs/reliability#281

Open

This was referenced May 19, 2022

CI Reliability 2022-05-19 nodejs/reliability#282

Open

CI Reliability 2022-05-20 nodejs/reliability#283

Open

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 23, 2022

github-actions bot mentioned this pull request May 24, 2022

CI Reliability 2022-05-24 nodejs/reliability#287

Open

20 tasks

mcollina 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. labels May 24, 2022

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 May 24, 2022

mcollina approved these changes May 24, 2022

View reviewed changes

mcollina added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels May 24, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 24, 2022

nodejs-github-bot merged commit 714e2d7 into nodejs:master May 24, 2022

daeyeon deleted the master.doc-220425.Mon.5754 branch May 24, 2022 11:41

bengl mentioned this pull request May 31, 2022

v18.3.0 proposal #43266

Merged

targos mentioned this pull request Aug 3, 2022

v16.17.0 release proposal #44098

Merged

Uh oh!

http2: improve tests and docs #42858

http2: improve tests and docs #42858

Uh oh!

Conversation

daeyeon commented Apr 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Apr 25, 2022

Uh oh!

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

ShogunPanda left a comment

Choose a reason for hiding this comment

Uh oh!

RafaelGSS left a comment

Choose a reason for hiding this comment

Uh oh!

daeyeon commented May 14, 2022

Uh oh!

nodejs-github-bot commented May 14, 2022

Uh oh!

daeyeon commented May 18, 2022

Uh oh!

nodejs-github-bot commented May 18, 2022

Uh oh!

nodejs-github-bot commented May 23, 2022

Uh oh!

daeyeon commented May 24, 2022

Uh oh!

nodejs-github-bot commented May 24, 2022

Uh oh!

nodejs-github-bot commented May 24, 2022

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented May 24, 2022

Uh oh!

daeyeon commented May 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

daeyeon commented Apr 25, 2022 •

edited

Loading