test: fix flaky test-https-server-close- tests by santigimeno · Pull Request #43216 · nodejs/node

santigimeno · 2022-05-26T18:20:30Z

Don't initiate the second connection until the first has been
established.

Refs: nodejs/reliability#289

nodejs-github-bot · 2022-05-28T18:55:51Z

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

nodejs-github-bot · 2022-06-10T19:19:19Z

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

nodejs-github-bot · 2022-06-12T05:38:52Z

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

lpinca · 2022-06-12T05:45:12Z

test/parallel/test-https-server-close-idle.js

-    client1Closed = true;
-  }));
+  client1.on('connect', common.mustCall(() => {
+    assert.strictEqual(connections, 1);


Suggested change

assert.strictEqual(connections, 1);

It seems that the 'connect' event can be emitted before the 'connection' event on the server. I think we can remove this as the second connection is initiated from the listener of this event anyway.

@santigimeno are you ok with this? I would like to see if CI is happy after this small change.

@lpinca sure thing. Just rebased and pushed with that change. Sorry for the delay. Thanks!

Don't initiate the second connection until the first has been established. Refs: nodejs/reliability#289

nodejs-github-bot · 2022-06-17T14:04:34Z

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

nodejs-github-bot · 2022-06-17T20:19:00Z

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

F3n67u · 2022-06-18T03:17:59Z

@santigimeno test-http-server-close-idle is also flaky. I filed a issue at #43466. Could this pr fix test-http-server-close-idle flaky and close #43466?

santigimeno · 2022-06-18T06:23:39Z

@santigimeno test-http-server-close-idle is also flaky. I filed a issue at #43466. Could this pr fix test-http-server-close-idle flaky and close #43466?

Yes, this PR fixes 4 very similar flaky tests included the one you mention. The PR title is confusing, I'll change it.

F3n67u · 2022-06-18T07:05:56Z

@santigimeno test-http-server-close-idle is also flaky. I filed a issue at #43466. Could this pr fix test-http-server-close-idle flaky and close #43466?

Yes, this PR fixes 4 very similar flaky tests included the one you mention. The PR title is confusing, I'll change it.

Great work!

nodejs-github-bot · 2022-06-18T07:18:45Z

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

F3n67u · 2022-06-19T11:36:33Z

Could we land this pr now? Those flaky tests often fail on my pr.

nodejs-github-bot · 2022-06-19T11:42:04Z

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

nodejs-github-bot · 2022-06-19T13:32:17Z

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

nodejs-github-bot · 2022-06-19T15:12:08Z

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

nodejs-github-bot · 2022-06-19T16:19:54Z

Landed in f8f2772

Don't initiate the second connection until the first has been established. Refs: nodejs/reliability#289 PR-URL: #43216 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels May 26, 2022

santigimeno force-pushed the santi/fix_test_https_close_idle branch from f3f6774 to 9f2d594 Compare May 26, 2022 18:31

lpinca approved these changes May 26, 2022

View reviewed changes

lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label May 28, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 28, 2022

lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 10, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 10, 2022

jasnell approved these changes Jun 12, 2022

View reviewed changes

lpinca reviewed Jun 12, 2022

View reviewed changes

This was referenced Jun 13, 2022

CI Reliability 2022-06-13 nodejs/reliability#299

Open

CI Reliability 2022-06-14 nodejs/reliability#300

Open

test: fix flaky test-https-server-close- tests

c0c4661

Don't initiate the second connection until the first has been established. Refs: nodejs/reliability#289

santigimeno force-pushed the santi/fix_test_https_close_idle branch from 9f2d594 to c0c4661 Compare June 17, 2022 13:11

lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2022

santigimeno added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2022

github-actions bot mentioned this pull request Jun 18, 2022

CI Reliability 2022-06-18 nodejs/reliability#304

Open

34 tasks

F3n67u mentioned this pull request Jun 18, 2022

Investigate flaky test-http-server-request-timeouts-mixed #43465

Open

F3n67u mentioned this pull request Jun 18, 2022

Investigate flaky test-http-server-close-idle #43466

Closed

santigimeno changed the title ~~test: fix flaky test-https-server-close-idle~~ test: fix flaky test-https-server-close- tests Jun 18, 2022

F3n67u added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 18, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 18, 2022

F3n67u added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Jun 18, 2022

github-actions bot mentioned this pull request Jun 19, 2022

CI Reliability 2022-06-19 nodejs/reliability#305

Open

30 tasks

F3n67u added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 19, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 19, 2022

lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 19, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 19, 2022

nodejs-github-bot merged commit f8f2772 into nodejs:main Jun 19, 2022

This was referenced Jun 20, 2022

CI Reliability 2022-06-20 nodejs/reliability#306

Open

CI Reliability 2022-06-21 nodejs/reliability#307

Open

CI Reliability 2022-06-22 nodejs/reliability#308

Open

CI Reliability 2022-06-23 nodejs/reliability#309

Open

targos mentioned this pull request Jul 12, 2022

v18.6.0 release proposal #43789

Merged

targos added the dont-land-on-v16.x label Jul 31, 2022

Uh oh!

Conversation

santigimeno commented May 26, 2022

Uh oh!

nodejs-github-bot commented May 28, 2022

Uh oh!

nodejs-github-bot commented Jun 10, 2022

Uh oh!

nodejs-github-bot commented Jun 12, 2022

Uh oh!

lpinca Jun 12, 2022

Choose a reason for hiding this comment

Uh oh!

lpinca Jun 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

santigimeno Jun 17, 2022

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jun 17, 2022

Uh oh!

nodejs-github-bot commented Jun 17, 2022

Uh oh!

F3n67u commented Jun 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

santigimeno commented Jun 18, 2022

Uh oh!

F3n67u commented Jun 18, 2022

Uh oh!

nodejs-github-bot commented Jun 18, 2022

Uh oh!

F3n67u commented Jun 19, 2022

Uh oh!

nodejs-github-bot commented Jun 19, 2022

Uh oh!

nodejs-github-bot commented Jun 19, 2022

Uh oh!

nodejs-github-bot commented Jun 19, 2022

Uh oh!

nodejs-github-bot commented Jun 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lpinca Jun 17, 2022 •

edited

Loading

F3n67u commented Jun 18, 2022 •

edited

Loading