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: refactor test-handle-wrap-close-abort #10188

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Dec 9, 2016

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

test process

Description of change
  • use common.mustCall() to confirm number of uncaught exceptions
  • var -> const
  • specify duratino of 1ms for setTimeout() and setInterval()

@Trott Trott added process Issues and PRs related to the process subsystem. test Issues and PRs related to the tests. labels Dec 9, 2016
@Trott
Copy link
Member Author

Trott commented Dec 9, 2016

@mscdex
Copy link
Contributor

mscdex commented Dec 9, 2016

Typo in commit message: s/duratino/duration/

* use common.mustCall() to confirm number of uncaught exceptions
* var -> const
* specify duration of 1ms for setTimeout() and setInterval()
@Trott
Copy link
Member Author

Trott commented Dec 9, 2016

Typo in commit message: s/duratino/duration/

Fixed; thanks!

@Trott
Copy link
Member Author

Trott commented Dec 9, 2016


process.on('uncaughtException', function() { });
function noop() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious why you decided to define noop and uncaughtExceptionHandler, instead of just doing common.mustCall(function() {}, 2) inline.

Copy link
Member Author

Choose a reason for hiding this comment

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

No good reason. Aesthetics. Or indecisiveness. Or something. If you prefer it all inline, I'm happy to do that instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

No strong preference. I was just curious.

Copy link
Contributor

@thefourtheye thefourtheye left a comment

Choose a reason for hiding this comment

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

LGTM with @cjihrig's suggestion.

Trott added a commit to Trott/io.js that referenced this pull request Dec 12, 2016
* use common.mustCall() to confirm number of uncaught exceptions
* var -> const
* specify duration of 1ms for setTimeout() and setInterval()

PR-URL: nodejs#10188
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@Trott
Copy link
Member Author

Trott commented Dec 12, 2016

Landed in 499fc7a.

@Trott Trott closed this Dec 12, 2016
Fishrock123 pushed a commit that referenced this pull request Dec 13, 2016
* use common.mustCall() to confirm number of uncaught exceptions
* var -> const
* specify duration of 1ms for setTimeout() and setInterval()

PR-URL: #10188
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@italoacasas italoacasas mentioned this pull request Dec 15, 2016
targos pushed a commit that referenced this pull request Dec 28, 2016
* use common.mustCall() to confirm number of uncaught exceptions
* var -> const
* specify duration of 1ms for setTimeout() and setInterval()

PR-URL: #10188
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 16, 2017
* use common.mustCall() to confirm number of uncaught exceptions
* var -> const
* specify duration of 1ms for setTimeout() and setInterval()

PR-URL: #10188
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 16, 2017
* use common.mustCall() to confirm number of uncaught exceptions
* var -> const
* specify duration of 1ms for setTimeout() and setInterval()

PR-URL: #10188
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
* use common.mustCall() to confirm number of uncaught exceptions
* var -> const
* specify duration of 1ms for setTimeout() and setInterval()

PR-URL: #10188
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
* use common.mustCall() to confirm number of uncaught exceptions
* var -> const
* specify duration of 1ms for setTimeout() and setInterval()

PR-URL: #10188
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
This was referenced Jan 24, 2017
MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
* use common.mustCall() to confirm number of uncaught exceptions
* var -> const
* specify duration of 1ms for setTimeout() and setInterval()

PR-URL: #10188
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 1, 2017
* use common.mustCall() to confirm number of uncaught exceptions
* var -> const
* specify duration of 1ms for setTimeout() and setInterval()

PR-URL: #10188
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@Trott Trott deleted the refactor-handle branch January 13, 2022 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Issues and PRs related to the process subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants