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

test: fix flaky test-https-connect-address-family by Trott · Pull Request #7605 · nodejs/node · GitHub

/ node Public

test: fix flaky test-https-connect-address-family - #7605

Closed
Trott wants to merge 3 commits into
nodejs:masterfrom
Trott:ipv6localhost
ConversationCommits (3)ChecksFiles changed
Closed

test: fix flaky test-https-connect-address-family#7605
Trott wants to merge 3 commits into
nodejs:masterfrom
Trott:ipv6localhost

Conversation

Trott commented Jul 8, 2016

Copy link
Copy Markdown
Member
Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test https

Description of change

Skip test if localhost does not resolve to ::1.

Fixes: #7288

Trott added https Issues or PRs related to the https subsystem. test Issues and PRs related to the tests. labels Jul 8, 2016
Trott mentioned this pull request Jul 8, 2016
Closed

Trott commented Jul 8, 2016

Copy link
Copy Markdown
Member Author

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

Trott commented Jul 8, 2016

Copy link
Copy Markdown
Member Author

One build failure on Raspberry Pi but otherwise CI is good.

rvagg commented Jul 8, 2016

Copy link
Copy Markdown
Member

nice, lgtm

bnoordhuis commented Jul 8, 2016

Copy link
Copy Markdown
Member

LGTM

gibfahn commented Jul 8, 2016 •
edited
Loading

Copy link
Copy Markdown
Member

@Trott would the same fix work for test-tls-connect-address-family? Assuming it would, would you want to do a separate PR for it?

Trott commented Jul 8, 2016

Copy link
Copy Markdown
Member Author

@gibfahn Whoops, yes, I'll add that file too...

Trott commented Jul 8, 2016

Copy link
Copy Markdown
Member Author

Added a second commit to addres test-tls-connect-address-family the same way. PTAL.

Trott commented Jul 11, 2016

Copy link
Copy Markdown
Member Author

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

thefourtheye reviewed Jul 11, 2016
View reviewed changes
Comment thread test/parallel/test-https-connect-address-family.js Outdated

thefourtheye Jul 11, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Wouldn't it be better to do

addresses.some((val) => val.address === '::1')

Trott Jul 11, 2016 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I thought I was going with the de facto standard in the project code as it exists, but it looks like you are correct. There are over 150 instances where the braces are already omitted and only around 20 cases where there are braces included even though they can be removed.

So, yeah, I'll remove these.

thefourtheye commented Jul 11, 2016

Copy link
Copy Markdown
Contributor

LGTM

Trott added 3 commits July 11, 2016 13:42
test: fix flaky test-https-connect-address-family
f5a0511 
Skip test if localhost does not resolve to ::1.

Fixes: nodejs#7288
test: fix flaky test-tls-connect-address-family
00836cf 
Skip test if localhost does not resolve to ::1.

Fixes: nodejs#7288
squash: arrow function braces
366eaac

Trott commented Jul 11, 2016

Copy link
Copy Markdown
Member Author

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

Trott added a commit to Trott/io.js that referenced this pull request Jul 11, 2016
test: fix flaky test-*-connect-address-family
2d77cba 
Skip tests if localhost does not resolve to ::1.

Fixes: nodejs#7288
PR-URL: nodejs#7605
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

Trott commented Jul 11, 2016

Copy link
Copy Markdown
Member Author

Landed in 2d77cba

Trott closed this Jul 11, 2016
cjihrig mentioned this pull request Aug 8, 2016
Closed
cjihrig pushed a commit that referenced this pull request Aug 10, 2016
test: fix flaky test-*-connect-address-family
e56db14 
Skip tests if localhost does not resolve to ::1.

Fixes: #7288
PR-URL: #7605
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

Conflicts:
	test/parallel/test-https-connect-address-family.js
	test/parallel/test-tls-connect-address-family.js
cjihrig mentioned this pull request Aug 11, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Sep 28, 2016
test: fix flaky test-*-connect-address-family
855a738 
Skip tests if localhost does not resolve to ::1.

Fixes: #7288
PR-URL: #7605
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
bnoordhuis mentioned this pull request Sep 28, 2016
Closed
rvagg pushed a commit that referenced this pull request Oct 18, 2016
test: fix flaky test-*-connect-address-family
28f4784 
Skip tests if localhost does not resolve to ::1.

Fixes: #7288
PR-URL: #7605
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
test: fix flaky test-*-connect-address-family
3dbcc3d 
Skip tests if localhost does not resolve to ::1.

Fixes: #7288
PR-URL: #7605
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins mentioned this pull request Oct 26, 2016
Closed
Trott deleted the ipv6localhost branch January 13, 2022 22:43
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

No reviews

Assignees

No one assigned

Labels

https Issues or PRs related to the https subsystem. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Investigate flaky test-https-connect-address-family.js and test-tls-connect-address-family

6 participants

Back | FazBrowse Home | New Git URL