Skip to content

test: fix test-net-happy-eyeballs without IPv6 support - #45856

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
LiviaMedeiros:net-test-happyeyeballs-noipv6
Feb 18, 2023
Merged

test: fix test-net-happy-eyeballs without IPv6 support#45856
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
LiviaMedeiros:net-test-happyeyeballs-noipv6

Conversation

@LiviaMedeiros

@LiviaMedeirosLiviaMedeiros commented Dec 14, 2022•
edited
Loading

Copy link
Copy Markdown
Member

The error observed on linux-6.1.0 with disabled CONFIG_IPV6 is EAFNOSUPPORT.

cc @ShogunPanda in case of any of:

  • EADDRNOTAVAIL is confirmed to be thrown on any other platform: in this case, it would make sense to simply leave the error unchecked. Confirmed, checking both errors.
  • undefined:undefined is not what should appear in error message: in this case, we probably need a fallback to DEFAULT_IPV4_ADDR/DEFAULT_IPV6_ADDR at

    node/lib/net.js

    Line 1040 in cf0a42c

    details=sockname.address+':'+sockname.port;

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Dec 14, 2022
@ShogunPanda

ShogunPanda commented Dec 14, 2022

Copy link
Copy Markdown
Contributor

@LiviaMedeiros This looks fine to me in general. What happens for other platforms?

@LiviaMedeiros

LiviaMedeiros commented Dec 14, 2022

Copy link
Copy Markdown
MemberAuthor

I didn't test it but from my understanding of connect(2), it must always return EAFNOSUPPORT first on any platform.
EADDRNOTAVAIL should happen if the family is supported but all ports are unavailable (e.g. busy).

If there is any uncertainty, might be better to skip those assertions.

@LiviaMedeirosLiviaMedeiros added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 14, 2022
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 14, 2022
@nodejs-github-bot

nodejs-github-bot commented Dec 14, 2022

Copy link
Copy Markdown
Collaborator

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

@LiviaMedeiros

LiviaMedeiros commented Dec 14, 2022

Copy link
Copy Markdown
MemberAuthor

Judging from CI results, I was wrong and the test can produce both EAFNOSUPPORT and EADDRNOTAVAIL.

Unless we really need to check the error, skipping that part.

@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
This was referenced Dec 17, 2022
Open
Open
Open
@nodejs-github-bot

nodejs-github-bot commented Dec 23, 2022

Copy link
Copy Markdown
Collaborator

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

@LiviaMedeiros
LiviaMedeirosforce-pushed the net-test-happyeyeballs-noipv6 branch from a117b3f to 5a2e778CompareFebruary 10, 2023 07:30
@LiviaMedeirosLiviaMedeiros added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 10, 2023
@github-actionsgithub-actionsBot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Feb 10, 2023
@github-actions

github-actionsBot commented Feb 10, 2023

Copy link
Copy Markdown
Contributor
Failed to start CI
- Validating Jenkins credentials
✖ Jenkins credentials invalid
https://github.com/nodejs/node/actions/runs/4141827344

@Trott

This comment was marked as resolved.

Sign in to view
@Trott

This comment was marked as resolved.

Sign in to view
@LiviaMedeirosLiviaMedeiros added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Feb 13, 2023
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 16, 2023
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@github-actionsgithub-actionsBot mentioned this pull request Feb 18, 2023
Open
32 tasks
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

nodejs-github-bot commented Feb 18, 2023

Copy link
Copy Markdown
Collaborator

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

@LiviaMedeirosLiviaMedeiros added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 18, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 18, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 109545a into nodejs:mainFeb 18, 2023
@nodejs-github-bot

nodejs-github-bot commented Feb 18, 2023

Copy link
Copy Markdown
Collaborator

Landed in 109545a

MylesBorins pushed a commit that referenced this pull request Feb 18, 2023
@LiviaMedeiros@MylesBorins
test: fix test-net-autoselectfamily for kernel without IPv6 support
673f76a
PR-URL: #45856
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
@MylesBorinsMylesBorins mentioned this pull request Feb 19, 2023
Merged
MylesBorins pushed a commit that referenced this pull request Feb 20, 2023
@LiviaMedeiros@MylesBorins
test: fix test-net-autoselectfamily for kernel without IPv6 support
6ad01fd
PR-URL: #45856
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
@danielleadams

danielleadams commented Apr 5, 2023

Copy link
Copy Markdown
Contributor

@LiviaMedeiros this broke tests when pulling into v18.x - do you mind opening a backport PR?

@LiviaMedeirosLiviaMedeiros mentioned this pull request Apr 6, 2023
Closed
@LiviaMedeiros

LiviaMedeiros commented Apr 6, 2023

Copy link
Copy Markdown
MemberAuthor

@danielleadams opened #47447
Keeping backport-requested-v18.x label as it would require second commit after #45777 (which depends on #45057 to land on v18.x, which has #45994 backport which has conflicts atm) lands on v18.x

LiviaMedeiros added a commit to LiviaMedeiros/node that referenced this pull request Aug 17, 2023
@LiviaMedeiros
test: fix test-net-autoselectfamily for kernel without IPv6 support
15b1c5a
PR-URL: nodejs#45856
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
@LiviaMedeiros

LiviaMedeiros commented Aug 17, 2023

Copy link
Copy Markdown
MemberAuthor

With #49016, this can be landed on v18.x.

@LiviaMedeirosLiviaMedeiros mentioned this pull request Aug 27, 2023
Merged
ruyadorno pushed a commit that referenced this pull request Aug 29, 2023
@LiviaMedeiros@ruyadorno
test: fix test-net-autoselectfamily for kernel without IPv6 support
c728b8a
PR-URL: #45856
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ShogunPandaShogunPandaShogunPanda approved these changes
@jasnelljasnelljasnell approved these changes
@cjihrigcjihrigcjihrig approved these changes

Assignees

No one assigned

Labels

needs-ciPRs that need a full CI run.testIssues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@LiviaMedeiros@ShogunPanda@nodejs-github-bot@Trott@danielleadams@jasnell@cjihrig