Fix flakiness of pipeflood test by Trott · Pull Request #3636 · nodejs/node · GitHub

Continuation of #2862

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

Removed flaky status for the test. Rerun CI just because: https://ci.nodejs.org/job/node-test-pull-request/664/

Well, this is a bummer, but it looks like this does not fix the pipeflood test problem on Windows: https://ci.nodejs.org/job/node-test-binary-windows/189/RUN_SUBSET=1,VS_VERSION=vs2015,label=win2012r2/tapTestReport/test.tap-228/

Although it's failing differently so maybe it can be tweaked...

Wrapping the req.socket.on('data',...) business in setImmediate() (so that any events already in the event queue will fire first?), the test works reliably on Windows now.

With it wrapped and set to run 16 times, no failures:

With it not wrapped and set to run 16 times, failures:

With changes to get rid of ES6-isms, etc., Node v0.10.20 fails on the test still and node v0.10.21 passes, which is as it should be.

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

Now it's failing on the Pi. (╯°□°）╯︵ ┻━┻

https://ci.nodejs.org/job/node-test-binary-arm/431/RUN_SUBSET=1,nodes=pi2-raspbian-wheezy/console

Switched to common.platformTimeout() instead of hardcoding the same timeouts for arm as for everything else. CI run looks better. https://ci.nodejs.org/job/node-test-pull-request/686/ Only failures are for other flaky tests and build-bot failures.

/cc @dnakamura @rvagg @isaacs

I vaguely remember a PR to make all Errors be called with new. May be worthwhile to make it consistent?

@evanlucas Sure, we can do that. .... Done!

"the mechanism" or "the flood prevention mechanism"?

I think this test could live in test/parallel but I'd appreciate it if you did the move in a separate commit.

OK, made all the changes suggested by @bnoordhuis, rebased, force pushed. PTAL

CI: https://ci.nodejs.org/job/node-test-pull-request/708/ Hope it doesn't have any surprises...

This extends fixes for test-https-pipeline-flood to hopefully fully eliminate its flakiness on Windows in our continuous integration process. PR-URL: #3636 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

landed in v4.x-staging in 5254fda...9294523

Would be nice to have this landed in 0.12.X as it is the last persistent failure on windows that keeps builds from being green

PR-URL: #3636 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

This extends fixes for test-https-pipeline-flood to hopefully fully eliminate its flakiness on Windows in our continuous integration process. PR-URL: #3636 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

PR-URL: #3636 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

This extends fixes for test-https-pipeline-flood to hopefully fully eliminate its flakiness on Windows in our continuous integration process. PR-URL: #3636 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

PR-URL: #3636 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

This extends fixes for test-https-pipeline-flood to hopefully fully eliminate its flakiness on Windows in our continuous integration process. PR-URL: #3636 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

This extends fixes for test-https-pipeline-flood to hopefully fully eliminate its flakiness on Windows in our continuous integration process. PR-URL: nodejs#3636 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

@Trott I'm wondering why its set as dont-land-on-v4.x and the lts-watch-v0.12 was removed. Sounds like we are not backporting but not clear why from what I've read in the issue.

@mhdawson The explanation is in #4730. Ultimately, it's a no-op.

Trott mentioned this pull request Nov 2, 2015

Fix flaky test test-http-pipeline-flood #2862

Closed

mscdex added http Issues or PRs related to the http subsystem. test Issues and PRs related to the tests. labels Nov 3, 2015

Trott force-pushed the pipeflood_fix branch from 23a0aa8 to 456a455 Compare November 7, 2015 03:32

Trott changed the title Pipeflood fix Fix flakiness of pipeflood test Nov 7, 2015

evanlucas reviewed Nov 8, 2015
View reviewed changes

Trott force-pushed the pipeflood_fix branch from 33c581a to 5f3446a Compare November 8, 2015 15:56

This was referenced Nov 8, 2015

tools: enforce throw new Error() with lint rule #3714

Closed

Flaky tests nodejs/build#248

Closed

Trott force-pushed the pipeflood_fix branch from 5f3446a to b5510ae Compare November 11, 2015 06:01

bnoordhuis reviewed Nov 11, 2015
View reviewed changes

Devin Nakamura and others added 2 commits November 11, 2015 20:01

test: fix flaky test test-http-pipeline-flood

8612e36

test: refactor test-http-pipeline-flood

f003c84

Trott force-pushed the pipeflood_fix branch from da95dbf to f003c84 Compare November 12, 2015 06:18

MylesBorins added lts-landed-on-v4.x and removed lts-watch-v4.x labels Nov 17, 2015

mhdawson added the lts-watch-v0.12 label Nov 25, 2015

rvagg pushed a commit that referenced this pull request Dec 4, 2015

test: fix flaky test test-http-pipeline-flood

b084554

PR-URL: #3636 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins added land-on-v4.x and removed lts-landed-on-v4.x labels Dec 17, 2015

jasnell mentioned this pull request Dec 17, 2015

Release 4.2.4 Planning #4321

Closed

jasnell pushed a commit that referenced this pull request Dec 17, 2015

test: fix flaky test test-http-pipeline-flood

6c23f67

PR-URL: #3636 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

jasnell pushed a commit that referenced this pull request Dec 23, 2015

test: fix flaky test test-http-pipeline-flood

52a9422

PR-URL: #3636 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Trott added dont-land-on-v4.x and removed land-on-v4.x labels Jan 18, 2016

Trott mentioned this pull request Jan 18, 2016

Add missing commits to v4.x-staging #4730

Closed

stefanmb mentioned this pull request Apr 28, 2016

test: test-http-pipeline-flood failures on Win32 #6450

Closed

Trott deleted the pipeflood_fix branch January 13, 2022 22:29

AnnAngela mentioned this pull request Dec 21, 2023

ESM loader cannot detect module system properly in folder whose name ends with node_modules #51245

Closed

Fix flakiness of pipeflood test by Trott · Pull Request #3636 · nodejs/node · GitHub

Uh oh!

Uh oh!

Fix flakiness of pipeflood test - #3636

Fix flakiness of pipeflood test#3636Trott wants to merge 2 commits intonodejs:masternodejs/node:masterfrom Trott:pipeflood_fixTrott/io.js:pipeflood_fixCopy head branch name to clipboard

Conversation

Trott commented Nov 2, 2015

Uh oh!

Trott commented Nov 2, 2015

Uh oh!

Trott commented Nov 2, 2015

Uh oh!

Trott commented Nov 3, 2015

Uh oh!

Trott commented Nov 3, 2015

Uh oh!

Trott commented Nov 7, 2015

Uh oh!

Trott commented Nov 7, 2015

Uh oh!

Trott commented Nov 7, 2015

Uh oh!

Trott commented Nov 7, 2015

Uh oh!

Trott commented Nov 7, 2015

Uh oh!

evanlucas Nov 8, 2015

Choose a reason for hiding this comment

Uh oh!

Trott Nov 8, 2015

Choose a reason for hiding this comment

Uh oh!

bnoordhuis Nov 11, 2015

Choose a reason for hiding this comment

Uh oh!

bnoordhuis commented Nov 11, 2015

Uh oh!

Trott commented Nov 12, 2015

Uh oh!

Trott commented Nov 12, 2015

Uh oh!

MylesBorins commented Nov 17, 2015

Uh oh!

mhdawson commented Nov 25, 2015

Uh oh!

mhdawson commented Mar 9, 2016

Uh oh!

Trott commented Mar 9, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Fix flakiness of pipeflood test#3636
Trott wants to merge 2 commits into
nodejs:masterfrom
Trott:pipeflood_fix