Skip to content

Commit

Permalink
test: fix flaky test-http2-stream-destroy-event-order
Browse files Browse the repository at this point in the history
Alternative to #31590.

It appears that the issue here is that the test falsely assumed that
closing the client (which also currently destroys the socket rather
than gracefully shutting down the connection) would still leave
enough time for the server side to receive the stream error.

Address that by explicitly waiting for the server side to receive the
stream error before closing the client and the connection with it.

Refs: #31590
Refs: #20750

PR-URL: #31610
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
addaleax authored and codebytere committed Mar 17, 2020
1 parent c1cba92 commit 502f42c
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 6 deletions.
3 changes: 0 additions & 3 deletions test/parallel/parallel.status
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ test-http2-multistream-destroy-on-read-tls: PASS,FLAKY
# https://github.com/nodejs/node/issues/20750
test-http2-pipe: PASS,FLAKY
# https://github.com/nodejs/node/issues/20750
# https://github.com/nodejs/node/pull/31590
test-http2-stream-destroy-event-order: PASS,FLAKY
# https://github.com/nodejs/node/issues/20750
test-stream-pipeline-http2: PASS,FLAKY
# https://github.com/nodejs/node/issues/24497
test-timers-immediate-queue: PASS,FLAKY
Expand Down
5 changes: 2 additions & 3 deletions test/parallel/test-http2-stream-destroy-event-order.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ let req;
const server = http2.createServer();
server.on('stream', common.mustCall((stream) => {
stream.on('error', common.mustCall(() => {
client.close();
stream.on('close', common.mustCall(() => {
server.close();
}));
Expand All @@ -22,8 +23,6 @@ server.listen(0, common.mustCall(() => {
req = client.request();
req.resume();
req.on('error', common.mustCall(() => {
req.on('close', common.mustCall(() => {
client.close();
}));
req.on('close', common.mustCall());
}));
}));

0 comments on commit 502f42c

Please sign in to comment.