Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.4k
test: print helpful err msg on test-dns-ipv6.js#3501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Conversation
test/internet/test-dns-ipv6.js Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style nit: line up first and second lines. e.g.
assert.ok(isIPv6(ip.address),'Invalid IPv6: '+ip.address.toString());There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated.
trevnorris commented Oct 23, 2015
one comment. otherwise LGTM. |
Trott commented Oct 24, 2015
LGTM |
thefourtheye commented Oct 25, 2015
Hmmm, then we should find the root cause and fix it. Do you have any instances to show? |
john-yan commented Oct 25, 2015
@thefourtheye we want to fix it too. We do have a case showing that this assertion fails but we don't know exactly what is wrong with the IP address that cause the failure, and we so far have no luck reproduce the problem. The fix is to add additional information to hopefully catch the wrong IP address when it happens again. |
The test sometimes fail on an assertion but no useful error message was generated for debugging. Modify the test to generate useful debugging message. PR-URL: nodejs#3501 Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Trott commented Oct 25, 2015
Landed in f4c0ed4 |
john-yan commented Oct 25, 2015
Hello @Trott , would you please also back port (cherry-pick) it to v4.x? Thanks. |
Trott commented Oct 25, 2015
I thought CI had already run on this but it looks like it didn't. Here it is now: https://ci.nodejs.org/job/node-test-pull-request/612/ Sorry about that, everyone. (Fortunately, it's a trivial change exceedingly unlikely to do any harm.) |
Trott commented Oct 25, 2015
thefourtheye commented Oct 25, 2015
Ha ha ha @nodejs/punished |
The test sometimes fail on an assertion but no useful error message was generated for debugging. Modify the test to generate useful debugging message. PR-URL: #3501 Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: Rich Trott <[email protected]>
The test sometimes fail on an assertion but no useful error message was generated for debugging. Modify the test to generate useful debugging message. PR-URL: #3501 Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: Rich Trott <[email protected]>
jasnell commented Oct 28, 2015
Landed in v4.x-staging in 9e05af7 |
The test sometimes fail on an assertion but no useful error message was generated for debugging. Modify the test to generate useful debugging message. PR-URL: #3501 Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Test sometimes fail on this assertion but no useful error message was generated for debugging. Modify the test to generate useful debugging message.