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

test: fix flaky test-http2-reset-flood by Trott · Pull Request #34318 · nodejs/node · GitHub

/ node Public

test: fix flaky test-http2-reset-flood - #34318

Closed
Trott wants to merge 2 commits into
nodejs:masterfrom
Trott:fix-29802
ConversationCommits (2)ChecksFiles changed
Closed

test: fix flaky test-http2-reset-flood#34318
Trott wants to merge 2 commits into
nodejs:masterfrom
Trott:fix-29802

Conversation

Trott commented Jul 12, 2020

Copy link
Copy Markdown
Member

Set allowHalfOpen: true in the client.

Fixes: #29802
Refs: #31806

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

test: fix flaky test-http2-reset-flood
9e95a46 
Set `allowHalfOpen: true` in the client.

Fixes: nodejs#29802
Refs: nodejs#31806
nodejs-github-bot added the test Issues and PRs related to the tests. label Jul 12, 2020

nodejs-github-bot commented Jul 12, 2020 •
edited by Trott
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32292/ ✅

Trott commented Jul 12, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

Stress test this PR (should be green): https://ci.nodejs.org/job/node-stress-single-test/111/ ✅

Stress test master branch (should be very red): https://ci.nodejs.org/job/node-stress-single-test/112/ ❌

nodejs-github-bot commented Jul 12, 2020 •
edited by Trott
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32295/ ✅

Trott added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jul 12, 2020

lpinca commented Jul 12, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Why allowHalfOpen was not needed before #31806? I mean #31806 was not meant to change net.Socket behavior no?

Trott commented Jul 14, 2020

Copy link
Copy Markdown
Member Author

Why allowHalfOpen was not needed before #31806? I mean #31806 was not meant to change net.Socket behavior no?

A similar comment was left by @addaleax on the original PR: #31806 (comment)

/cc @ronag

ronag commented Jul 14, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

I mean #31806 was not meant to change net.Socket behavior no?

It's a semver major... the timing of things might have changed. I believe onReadableStreamEnd was partly broken before and could cause 'finish' to be emitted after 'close' or something along those lines. Not sure anymore.

The full conversation is here https://github.com/nodejs/node/pull/31806/files#r386140400
Maybe related? #33137

addaleax commented Jul 14, 2020

Copy link
Copy Markdown
Member

Landed in 4195c31

addaleax closed this Jul 14, 2020
addaleax pushed a commit that referenced this pull request Jul 14, 2020
test: fix flaky test-http2-reset-flood
4195c31 
Set `allowHalfOpen: true` in the client.

Fixes: #29802
Refs: #31806

PR-URL: #34318
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2020
test: fix flaky test-http2-reset-flood
c72585b 
Set `allowHalfOpen: true` in the client.

Fixes: #29802
Refs: #31806

PR-URL: #34318
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
MylesBorins mentioned this pull request Jul 14, 2020
Merged
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
test: fix flaky test-http2-reset-flood
28ce378 
Set `allowHalfOpen: true` in the client.

Fixes: #29802
Refs: #31806

PR-URL: #34318
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
cjihrig pushed a commit that referenced this pull request Jul 23, 2020
test: fix flaky test-http2-reset-flood
9fa53b3 
Set `allowHalfOpen: true` in the client.

Fixes: #29802
Refs: #31806

PR-URL: #34318
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
test: fix flaky test-http2-reset-flood
de83997 
Set `allowHalfOpen: true` in the client.

Fixes: #29802
Refs: #31806

PR-URL: #34318
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
test: fix flaky test-http2-reset-flood
9741510 
Set `allowHalfOpen: true` in the client.

Fixes: #29802
Refs: #31806

PR-URL: #34318
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
codebytere mentioned this pull request Sep 28, 2020
Merged
Trott deleted the fix-29802 branch April 14, 2022 11:29
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

addaleax addaleax approved these changes
ronag ronag approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

flaky test-http2-reset-flood

5 participants

Back | FazBrowse Home | New Git URL