Skip to content
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

test: add useful info to error msg and refactor #18541

Closed

Conversation

chinhuang007
Copy link
Contributor

Add useful info about process.domain to error meesages in the
uncaughtException event listener and the beforeExit event listener.

Refactor code such as using template literals, and also make sure
uncaughtException listner is detached after firing once to avoid
endless loop in case of exception throw in the beforeExit event
listner.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Feb 3, 2018
assert.strictEqual(
process.domain, null,
'domains stack should be empty in uncaughtException handler');
`Domains stack should be empty in uncaughtException handler
but the value of process.domain is ${JSON.stringify(process.domain)}`);
Copy link
Member

Choose a reason for hiding this comment

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

This will print the message on two lines and add 4 spaces at the beginning of the second one. I think it's better to use a single line.

Copy link
Member

Choose a reason for hiding this comment

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

If a single line of code exceeds 80 chars, you can keep it as a single line of output like this:

'Domains stack should be empty in uncaughtException handler' +
`but the value of process.domain is ${JSON.stringify(process.domain)}`);

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, you can simply remove the message (3rd) argument entirely and the default behavior will print the value of process.domain. If you think the contents of the message are valuable, you can put them as a comment right above the call to assert.strictEqual().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @lpinca @Trott, for the comments. The message is on one line now.

@chinhuang007 chinhuang007 force-pushed the add-useful-info-domain-test branch from 5c2f6b7 to 94166e9 Compare February 5, 2018 23:14
@BridgeAR
Copy link
Member

BridgeAR commented Feb 6, 2018

Add useful info about process.domain to error meesages in the
uncaughtException event listener and the beforeExit event listener.

Refactor code such as using template literals, and also make sure
uncaughtException listner is detached after firing once to avoid
endless loop in case of exception throw in the beforeExit event
listner.
@chinhuang007 chinhuang007 force-pushed the add-useful-info-domain-test branch from 94166e9 to 6d1103a Compare February 7, 2018 22:33
@chinhuang007
Copy link
Contributor Author

The CI failure yesterday doesn't seem related to my change. Just rebased. Can someone start a new CI please?

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 10, 2018
@BridgeAR
Copy link
Member

@chinhuang007 don't worry about those. Our CI often either has some infrastructure failures and or test flakes.

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Feb 12, 2018
Add useful info about process.domain to error meesages in the
uncaughtException event listener and the beforeExit event listener.

Refactor code such as using template literals, and also make sure
uncaughtException listner is detached after firing once to avoid
endless loop in case of exception throw in the beforeExit event
listner.

PR-URL: nodejs#18541
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR
Copy link
Member

Landed in c3ff899 🎉

@chinhuang007 congratulations on your first commit to Node.js!

@BridgeAR BridgeAR closed this Feb 12, 2018
@chinhuang007 chinhuang007 deleted the add-useful-info-domain-test branch February 12, 2018 19:35
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
Add useful info about process.domain to error meesages in the
uncaughtException event listener and the beforeExit event listener.

Refactor code such as using template literals, and also make sure
uncaughtException listner is detached after firing once to avoid
endless loop in case of exception throw in the beforeExit event
listner.

PR-URL: #18541
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
Add useful info about process.domain to error meesages in the
uncaughtException event listener and the beforeExit event listener.

Refactor code such as using template literals, and also make sure
uncaughtException listner is detached after firing once to avoid
endless loop in case of exception throw in the beforeExit event
listner.

PR-URL: #18541
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 21, 2018
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
Add useful info about process.domain to error meesages in the
uncaughtException event listener and the beforeExit event listener.

Refactor code such as using template literals, and also make sure
uncaughtException listner is detached after firing once to avoid
endless loop in case of exception throw in the beforeExit event
listner.

PR-URL: #18541
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
Add useful info about process.domain to error meesages in the
uncaughtException event listener and the beforeExit event listener.

Refactor code such as using template literals, and also make sure
uncaughtException listner is detached after firing once to avoid
endless loop in case of exception throw in the beforeExit event
listner.

PR-URL: nodejs#18541
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 7, 2018
Add useful info about process.domain to error meesages in the
uncaughtException event listener and the beforeExit event listener.

Refactor code such as using template literals, and also make sure
uncaughtException listner is detached after firing once to avoid
endless loop in case of exception throw in the beforeExit event
listner.

PR-URL: #18541
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 7, 2018
Add useful info about process.domain to error meesages in the
uncaughtException event listener and the beforeExit event listener.

Refactor code such as using template literals, and also make sure
uncaughtException listner is detached after firing once to avoid
endless loop in case of exception throw in the beforeExit event
listner.

PR-URL: #18541
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Aug 9, 2018
Add useful info about process.domain to error meesages in the
uncaughtException event listener and the beforeExit event listener.

Refactor code such as using template literals, and also make sure
uncaughtException listner is detached after firing once to avoid
endless loop in case of exception throw in the beforeExit event
listner.

PR-URL: #18541
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Aug 16, 2018
MylesBorins pushed a commit that referenced this pull request Aug 16, 2018
Add useful info about process.domain to error meesages in the
uncaughtException event listener and the beforeExit event listener.

Refactor code such as using template literals, and also make sure
uncaughtException listner is detached after firing once to avoid
endless loop in case of exception throw in the beforeExit event
listner.

PR-URL: #18541
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants