Fix Node.JS test failures #442
Merged
Conversation
This file contains 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
The behaviour of the `'readable'` event changed, or was tightened up,
in
nodejs/node#17979
such that it is _always_ called on the next tick. This change appears
first in Node.JS v10.0).
Since we rely on `'readable' in the multiplexing, it means that we
have to be more careful about when we wait for the next event loop to
come around in tests.
The tests tend to be much more brittle than live code with respect to
the order things happen, since they attempt to nail down precise
states or orderings (e.g., "the `unpipe` happened exactly between
these writes").
When a channel is closed, its buffer is unpiped from the connection muxer as part of cleaning up. In NodeJS v10, this happens before any frames that have just been written to the channel are picked up by the muxer -- e.g., the CloseOK response. Instead of directly unpiping, end() the channel buffer, and let it be unpiped once all its frames are read. So far as I can think it through, this is correct behaviour anyway, and it only worked before by coincidence.
squaremo
force-pushed
the
issue/mux-test-failure
branch
from
8d6594b to
c973adb
Compare
This was referenced Dec 2, 2018
This was referenced Jun 6, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The behaviour of the
'readable'event changed, or was tightened up,in
such that it is always called on the next tick. This change appears
first in Node.JS v10.0).
Since we rely on `'readable' in the multiplexing, it means that we
have to be more careful about when we wait for the next event loop to
come around in tests.
The tests tend to be much more brittle than live code with respect to
the order things happen, since they attempt to nail down precise
states or orderings (e.g., "the
unpipehappened exactly betweenthese writes").
It also means we have to be more careful when cleaning up channels --
one test fails in NodeJS v10 because the cleanup unpipes it before the
last frame can be read.