-
Notifications
You must be signed in to change notification settings - Fork 29.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: http2 session operations after destroy #15758
Conversation
92210c1
to
0b978f5
Compare
0b978f5
to
579ce40
Compare
Performed git rebase with upstream/master |
Ci will need to be re-run for this. It's not running any subtasks. |
579ce40
to
3a0df5f
Compare
The CI is failing on slower platforms https://ci.nodejs.org/job/node-test-commit/13125//console |
common.expectsError(() => session.settings(), invalidSessionError); | ||
common.expectsError(() => session.shutdown(), invalidSessionError); | ||
common.expectsError(() => session.rstStream(), invalidSessionError); | ||
}, common.platformTimeout(100), stream.session); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As with the other PR, please undo this setTimeout()
and common.platformTimeout()
change unless you can demonstrate a need. In general, please don't make changes to tests because of unrelated failures in CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched to setImmediate
in the new commit
It looks like this deletes a test file. Is that a mistake? If not, can you explain? |
3a0df5f
to
23d4933
Compare
@Trott The file |
common.platformTimeout() has been removed. Thanks!
Refs #14985 PR-URL: #15758 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
Landed in abbdcaa |
Refs #14985 PR-URL: nodejs/node#15758 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
Refs #14985 PR-URL: #15758 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
This PR adds checks for invalid session errors for session operations when they're called after session.destroy() in HTTP2
Refs #14985
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
test, http2