FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: add quic teardown lifecycle tests by mannie-exe · Pull Request #62047 · nodejs/node · GitHub

/ node Public

test: add quic teardown lifecycle tests - #62047

Closed
mannie-exe wants to merge 1 commit into
nodejs:mainfrom
inherent-design:quic/test-teardown-lifecycle
ConversationCommits (1)ChecksFiles changed
Closed

test: add quic teardown lifecycle tests#62047
mannie-exe wants to merge 1 commit into
nodejs:mainfrom
inherent-design:quic/test-teardown-lifecycle

Conversation

mannie-exe commented Mar 1, 2026 •
edited
Loading

Copy link
Copy Markdown
  • test/common/quic/helpers.mjs (similar to quic: For streams allow ReadableStream as body #60237):
    • checkQuic
    • defaultCerts
    • createQuicPair
  • test/parallel/test-quic-session-close.mjs: 5 subtests
  • test/parallel/test-quic-session-destroy.mjs: 5 subtests
  • test/parallel/test-quic-endpoint-close.mjs: 7 subtests

session.close() is broken right now; handle.gracefulClose() never fires kFinishClose back to JS so await session.closed hangs forever. Tests are written to the documented contract so they'll fail on current main, which is the point. session.destroy() works fine.

Refs: #60309, #57119

#60122 covers 2.3.1, 2.3.3, and 1.2

test: add quic teardown lifecycle tests
46d7cdd 
Adds shared test helpers (checkQuic, defaultCerts, createQuicPair)
and 17 subtests covering session close, session destroy, and
endpoint close/destroy behavior.

session.close() hangs on current main because handle.gracefulClose()
never fires kFinishClose back to JS. Tests assert the documented
contract and will fail until that is fixed.

Refs: nodejs#60122
Refs: nodejs#60309
Refs: nodejs#57119
nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Mar 1, 2026
mohityadav8 reviewed Mar 1, 2026
View reviewed changes

mohityadav8 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

mannie-exe commented Mar 16, 2026

Copy link
Copy Markdown
Author

Hey @jasnell would you mind lmk if this looks alright or if I need to make any changes whenever you have time?

jasnell commented Mar 16, 2026

Copy link
Copy Markdown
Member

I'll take a look first thing tomorrow morning

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

This pull request has been marked as stale due to 90 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

github-actions Bot added the stale label Jul 28, 2026

jasnell commented Aug 8, 2026

Copy link
Copy Markdown
Member

I believe this one is out of date and likely needs to be revisited. Specifically, I think the current tests already subsume these additions

jasnell closed this Aug 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
mohityadav8 mohityadav8 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. stale test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL