Skip to content

async_hooks,test: Only use IPv6 in http test - #18143

Closed
AndreasMadsen wants to merge 1 commit into
nodejs:masterfrom
AndreasMadsen:async-hooks-http-pi-fix
Closed

async_hooks,test: Only use IPv6 in http test#18143
AndreasMadsen wants to merge 1 commit into
nodejs:masterfrom
AndreasMadsen:async-hooks-http-pi-fix

Conversation

@AndreasMadsen

Copy link
Copy Markdown
Member

If IPv6 is not supported on a machine, the IPv6 handle will first be
created, this will then fail and default to an IPv4 handle. This causes
the graph to change, as there now is an extra handle.

Fix: #18003 (comment)

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

async_hooks

If IPv6 is not supported on a machine, the IPv6 handle will first be
created, this will then fail and default to an IPv4 handle. This causes
the graph to change, as there now is an extra handle.
@nodejs-github-botnodejs-github-bot added async_hooks Issues and PRs related to the async hooks subsystem. test Issues and PRs related to the tests. labels Jan 14, 2018
@AndreasMadsen

Copy link
Copy Markdown
MemberAuthor

@AndreasMadsenAndreasMadsen added fast-track PRs that do not need to wait for 48 hours to land. dont-land-on-v4.x http Issues or PRs related to the http subsystem. labels Jan 14, 2018
@AndreasMadsen

Copy link
Copy Markdown
MemberAuthor

Test failure is unrelated:

sequential/test-timers-setInterval-excludes-callback-duration

assert.js:45
throw new errors.AssertionError(obj);
^
AssertionError [ERR_ASSERTION]: false == true
at Timeout.setInterval [as _onTimeout] (/home/iojs/build/workspace/node-test-binary-arm/test/sequential/test-timers-setInterval-excludes-callback-duration.js:15:5)
at ontimeout (timers.js:455:11)
at tryOnTimeout (timers.js:304:5)
at Timer.listOnTimeout (timers.js:267:5)

@AndreasMadsen

Copy link
Copy Markdown
MemberAuthor

/cc @Trott@jasnell

@TrottTrott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. +1 to fast-tracking.

@Trott

Copy link
Copy Markdown
Member

Might another possibility (future enhancement, doesn't have to happen now or even ever) be to expect a different call graph in that condition rather than skipping the test?

@apapirovski

Copy link
Copy Markdown
Contributor

Landed in b12425d

apapirovski pushed a commit that referenced this pull request Jan 14, 2018
If IPv6 is not supported on a machine, the IPv6 handle will first be
created, this will then fail and default to an IPv4 handle. This causes
the graph to change, as there now is an extra handle.
PR-URL: #18143Fixes: #18003
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 17, 2018
If IPv6 is not supported on a machine, the IPv6 handle will first be
created, this will then fail and default to an IPv4 handle. This causes
the graph to change, as there now is an extra handle.
PR-URL: nodejs#18143
Backport-PR-URL: nodejs#18179Fixes: nodejs#18003
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
If IPv6 is not supported on a machine, the IPv6 handle will first be
created, this will then fail and default to an IPv4 handle. This causes
the graph to change, as there now is an extra handle.
Backport-PR-URL: #18179
PR-URL: #18143Fixes: #18003
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas pushed a commit that referenced this pull request Jan 31, 2018
If IPv6 is not supported on a machine, the IPv6 handle will first be
created, this will then fail and default to an IPv4 handle. This causes
the graph to change, as there now is an extra handle.
Backport-PR-URL: #18474
PR-URL: #18143Fixes: #18003
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

async_hooksIssues and PRs related to the async hooks subsystem.fast-trackPRs that do not need to wait for 48 hours to land.httpIssues or PRs related to the http subsystem.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@AndreasMadsen@Trott@apapirovski@jasnell@addaleax@BethGriggs@nodejs-github-bot