improve message for test-domain-http-server failure cases to include actual value

[twk-b]

improve assert failure message in parallel/test-domain-http-server failure cases to include actual value with more meaningufl message

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)

test

[BridgeAR]

Do we want to land this? I prefer the plain message from strictEqual as the value would already be in the returned error message here.

[gibfahn]

I think the changes here make things less clear. I didn't see the original issue, but if you could undo the existing changes and then remove the 'Data should not be undefined' message on Line 54 I think that would be a really useful change.

node/test/parallel/test-domain-http-server.js

Line 54 in fabd618

assert.notStrictEqual(data, undefined, 'Data should not be undefined');

@twk-b if you could push a commit to your branch that makes this change then that would be good.

[joyeecheung]

@BridgeAR Yeah...I kind of prefer that as well. This task is probably not a suitable one. Sorry about the confusion @twk-b

[jasnell]

LGTM but this will need line wrapping to pass linting before land

[twk-b]

I agree that the default messaging, in standard format, is probably a better user experience. When making the changes, I did not get why you would want to replace standarding assert messages. I assumed it was a ticket either created before assert.strictEqual didn't show the output. Or perhaps like as Gibson suggests, maybe the ticket creator misdirected to assert.strictEqual instead of assert.notStrictEqual (in line 53) which makes more sense... but I am too noob to guess what needs to be done without more direction... I see approved in the comments, am I to fix the line wrap issue?

…

On Mon, Oct 9, 2017 at 6:38 PM, James M Snell ***@***.***> wrote: ***@***.**** approved this pull request. LGTM but this will need line wrapping to pass linting before land — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16048 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIc_zHi6lXPpusRtTygTyzyTepATC5KCks5sqryRgaJpZM4Pw-L2> .

[tniessen]

I would prefer to follow up on #16048 (comment).

[tniessen]

@gibfahn PTAL

[tniessen]

@twk-b You might want to force-push with your name and email address set correctly to an email address associated with your GitHub account. This is not strictly necessary, but the commits will not be associated with your account otherwise. You can view the data of a commit of your branch using git show --format=fuller.

[tniessen]

Partial CI: https://ci.nodejs.org/job/node-test-commit-linuxone/9389/

[twk-b]

Thanks for the tip, I have updated my global config, and ran the following: git commit --amend --author="Bernie Lees <bernie@dcbl.ca>" git show --format=fuller # looks correct now git push origin --force Sorry, I am a mercurial kid. hopefully those are the right steps.. --amend push --force sounds evil...

[tniessen]

Landed in 018375c, thank you for your contribution! 🎉

hopefully those are the right steps.. --amend push --force sounds evil...

Yes, everything seems to be correct now, thanks for fixing this!