Skip to content

test: use common.crashOnUnhandledRejection to test-promise.js #17229

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from
Closed

test: use common.crashOnUnhandledRejection to test-promise.js #17229

wants to merge 1 commit into from

Conversation

kurthsu
Copy link

@kurthsu kurthsu commented Nov 22, 2017

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

@nodejs-github-bot nodejs-github-bot added async_hooks Issues and PRs related to the async hooks subsystem. test Issues and PRs related to the tests. labels Nov 22, 2017
@@ -1,6 +1,10 @@
'use strict';

const common = require('../common');
common.crashOnUnhandledRejection();

// This test ensures that the promise handling resolution correctly
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this comment is necessary, can you please remove it

@gireeshpunathil gireeshpunathil added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Nov 22, 2017
@@ -1,6 +1,10 @@
'use strict';

const common = require('../common');
common.crashOnUnhandledRejection();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please move this after all the require statements

@jasnell
Copy link
Member

jasnell commented Nov 22, 2017

@jasnell
Copy link
Member

jasnell commented Nov 22, 2017

Fixing the nits on landing

@jasnell jasnell dismissed MylesBorins’s stale review November 22, 2017 23:57

Fixing the nits on landing

jasnell pushed a commit that referenced this pull request Nov 23, 2017
PR-URL: #17229
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Nov 23, 2017

Landed in 01dc6d2! Thank you and congrats on the PR to core!

@jasnell jasnell closed this Nov 23, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
PR-URL: #17229
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
PR-URL: #17229
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
PR-URL: #17229
Reviewed-By: James M Snell <jasnell@gmail.com>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
PR-URL: #17229
Reviewed-By: James M Snell <jasnell@gmail.com>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants