Skip to content

test: add http2 tests - #1228

Merged
chimurai merged 1 commit into
masterfrom
http2-tests
May 9, 2026
Merged

test: add http2 tests#1228
chimurai merged 1 commit into
masterfrom
http2-tests

Conversation

@chimurai

@chimurai chimurai commented May 9, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Tests

    • Added end-to-end HTTP/2 proxy tests covering secure server proxying, HTTP/2 multiplexing, and mixed HTTP/1.1 scenarios.
  • Chores

    • Updated spell-check dictionary (adjusted word list).
    • Added a development dependency for generating self-signed certificates used by tests.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 9, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1069dcd6-2497-499f-a47b-a2f20789d9eb

📥 Commits

Reviewing files that changed from the base of the PR and between be5f4f7 and e12ab16.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (3)
  • cspell.json
  • package.json
  • test/e2e/http2-server.spec.ts
✅ Files skipped from review due to trivial changes (2)
  • cspell.json
  • package.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/e2e/http2-server.spec.ts

📝 Walkthrough

Walkthrough

This PR adds HTTP/2 server E2E test coverage to validate proxy middleware behavior over secure connections. The selfsigned package is introduced as a devDependency to generate per-test TLS certificates. Two test suites verify proxying to HTTP/1.1 and HTTP/2 mocked targets, with the latter also validating HTTP/2 stream multiplexing.

Changes

HTTP/2 E2E Testing with TLS

Layer / File(s) Summary
Dependencies & Spellcheck
package.json, cspell.json
Added selfsigned v5.5.0 as a devDependency; updated spellcheck dictionary to include selfsigned and remove trivago.
Test Imports & Utilities
test/e2e/http2-server.spec.ts
Imported HTTP/2, TLS certificate generation (selfsigned), mocking (mockttp), request testing (supertest), and test helpers; stubs NODE_TLS_REJECT_UNAUTHORIZED per test.
HTTP/2 → HTTP/1.1 Proxy Test
test/e2e/http2-server.spec.ts
E2E suite: per-test self-signed TLS, start HTTP/1.1 mock target, start HTTP/2 secure server with proxy middleware, assert /api/hello is proxied, teardown.
HTTP/2 → HTTP/2 Proxy & Multiplexing
test/e2e/http2-server.spec.ts
E2E suite: start HTTP/2-capable mock target and secure server, assert /api/hello proxying, and run multiplexing test with three concurrent requests validating separate responses.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 In tunnels of TLS the streams entwine,
Self-signed certs hum, a soft rabbit sign,
Proxy hops through HTTP/2 delight,
Three tiny requests dance in the night,
Tests pass — carrots crunch, everything's fine. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding HTTP/2 tests to the test suite.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch http2-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread test/e2e/http2-server.spec.ts Fixed
Comment thread test/e2e/http2-server.spec.ts Fixed
@pkg-pr-new

pkg-pr-new Bot commented May 9, 2026

Copy link
Copy Markdown
npm i https://pkg.pr.new/http-proxy-middleware@1228

commit: e12ab16

@coveralls

coveralls commented May 9, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 94.07%. remained the same — http2-tests into master

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/e2e/http2-server.spec.ts`:
- Line 11: The teardown assumes mockTargetServer was initialized and will throw
if setup aborted early; update uses of mockTargetServer (the let
mockTargetServer variable and any afterEach/teardown calls such as
stop/close/closeAsync) to allow it to be undefined by declaring it optional and
guard all cleanup calls with a null/undefined check (e.g., if (mockTargetServer)
await mockTargetServer.stop()) so teardown becomes resilient when beforeEach
fails early — apply the same guard to every cleanup site referenced (lines
around the afterEach/teardown blocks that use mockTargetServer, including the
spots corresponding to the previous line 53, 74, and 116).
- Around line 51-52: The test restores NODE_TLS_REJECT_UNAUTHORIZED by assigning
previousNodeTlsRejectUnauthorized back to
process.env.NODE_TLS_REJECT_UNAUTHORIZED, which will set the literal string
"undefined" when the prior value was absent; update the restore logic wherever
previousNodeTlsRejectUnauthorized is used (both teardown/cleanup sites around
the HTTP/2 server tests) to check if previousNodeTlsRejectUnauthorized ===
undefined and, if so, delete process.env.NODE_TLS_REJECT_UNAUTHORIZED, otherwise
reassign the saved string value to process.env.NODE_TLS_REJECT_UNAUTHORIZED to
correctly restore the prior state.
- Line 67: The tests are invoking request(http2ProxyServer!) without enabling
HTTP/2 and the multiplexing test creates independent requests instead of a
persistent agent; update all supertest calls that use request(http2ProxyServer!)
(including the calls in the hello and other endpoint tests) to pass the option {
http2: true } so supertest uses HTTP/2, and change the multiplexing test to
create and reuse a persistent agent (e.g., supertest.agent with { http2: true })
to issue multiple concurrent requests through the same HTTP/2 connection to
validate multiplexing behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 730bfa13-b21d-4a69-8ee5-00356ec5832a

📥 Commits

Reviewing files that changed from the base of the PR and between 04c9474 and be5f4f7.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (3)
  • cspell.json
  • package.json
  • test/e2e/http2-server.spec.ts

Comment thread test/e2e/http2-server.spec.ts Outdated
Comment thread test/e2e/http2-server.spec.ts Outdated
Comment thread test/e2e/http2-server.spec.ts Outdated
@chimurai
chimurai merged commit ba9f262 into master May 9, 2026
26 checks passed
@chimurai
chimurai deleted the http2-tests branch May 9, 2026 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants