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

test: skip test-dns-ipv6.js if ipv6 is unavailable by john-yan · Pull Request #3444 · nodejs/node · GitHub

/ node Public

test: skip test-dns-ipv6.js if ipv6 is unavailable - #3444

Closed
john-yan wants to merge 1 commit into
nodejs:masterfrom
john-yan:master
ConversationCommits (1)ChecksFiles changed
Closed

test: skip test-dns-ipv6.js if ipv6 is unavailable#3444
john-yan wants to merge 1 commit into
nodejs:masterfrom
john-yan:master

Conversation

john-yan commented Oct 19, 2015

Copy link
Copy Markdown

Test should be skipped if ipv6 is unavailable on the running system.

MylesBorins reviewed Oct 19, 2015
View reviewed changes
Comment thread test/internet/test-dns-ipv6.js Outdated

MylesBorins Oct 19, 2015

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

the entire test is ipv6. You might want to make copy specify the entire test has failed

john-yan Oct 19, 2015

Copy link
Copy Markdown
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

yeah, good catch. thanks.

john-yan mentioned this pull request Oct 19, 2015
Closed

john-yan commented Oct 19, 2015

Copy link
Copy Markdown
Author

Issue number: #3443

Fishrock123 commented Oct 19, 2015

Copy link
Copy Markdown
Contributor

LGTM

cjihrig reviewed Oct 19, 2015
View reviewed changes
Comment thread test/internet/test-dns-ipv6.js Outdated

cjihrig Oct 19, 2015

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

LGTM, but couldn't you move this a little closer to the top of the file?

MylesBorins Oct 19, 2015

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

if it is put ahead of some of the variable declarations in the file then the process exit event won't pass.

MylesBorins Oct 19, 2015

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

at least needs to be below these

https://github.com/john-yan/node/blob/master/test/internet/test-dns-ipv6.js#L10-L13

john-yan Oct 19, 2015

Copy link
Copy Markdown
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

done.

mscdex added dns Issues and PRs related to the dns subsystem. test Issues and PRs related to the tests. labels Oct 19, 2015

evanlucas commented Oct 19, 2015

Copy link
Copy Markdown
Contributor

LGTM

1 similar comment

jbergstroem commented Oct 19, 2015

Copy link
Copy Markdown
Member

LGTM

Fishrock123 reviewed Oct 20, 2015
View reviewed changes
Comment thread test/internet/test-dns-ipv6.js Outdated

Fishrock123 Oct 20, 2015

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

One last nit, the style we use is 1..0 # Skipped: :)

john-yan Oct 20, 2015

Copy link
Copy Markdown
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

updated. thanks.

jasnell pushed a commit that referenced this pull request Oct 20, 2015
test: skip test-dns-ipv6.js if ipv6 is unavailable
a1886cf 
Test should be skipped if ipv6 is unavailable on the running system.

Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #3444

jasnell commented Oct 20, 2015

Copy link
Copy Markdown
Member

Landed in a1886cf

jasnell closed this Oct 20, 2015
rvagg pushed a commit that referenced this pull request Oct 21, 2015
test: skip test-dns-ipv6.js if ipv6 is unavailable
bbbd81e 
Test should be skipped if ipv6 is unavailable on the running system.

Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #3444
rvagg mentioned this pull request Oct 21, 2015
Merged
jasnell pushed a commit that referenced this pull request Oct 26, 2015
test: skip test-dns-ipv6.js if ipv6 is unavailable
49549c9 
Test should be skipped if ipv6 is unavailable on the running system.

Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #3444

jasnell commented Oct 26, 2015

Copy link
Copy Markdown
Member

Landed in v4.x-staging in 49549c9

rvagg mentioned this pull request Oct 28, 2015
Closed
jasnell pushed a commit that referenced this pull request Oct 29, 2015
test: skip test-dns-ipv6.js if ipv6 is unavailable
7c7ef01 
Test should be skipped if ipv6 is unavailable on the running system.

Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #3444
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

dns Issues and PRs related to the dns 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.

8 participants

Back | FazBrowse Home | New Git URL