-
Notifications
You must be signed in to change notification settings - Fork 155
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
Make it idempotent to capturePromise #400
Conversation
Add uncapturePromise function in order to cancel patch in tests. fix aws#398
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.
Thanks for the contribution! Generally lgtm, just one comment/question.
@@ -70,3 +70,6 @@ function sendRequest(address, cb) { | |||
.on('error', cb) | |||
.end(); | |||
} | |||
|
|||
// Cancel the patch because it doesn't affect other tests | |||
require('../../lib/patchers/promise_p').uncapturePromise(); |
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.
So this is effectively cleanup for the integration test? If so, can we add it after the assertion so we're confident it's executed after the test will execute? Or perhaps wrap things in a try..finally
block? Please inform me if I'm misunderstanding the purpose here.
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.
@willarmiros Thank you for your review. I moved cleanup for integration test after assertion, and change setup and cleanup process in promise_p.test.js
too.
To ensure it's executed after the test will execute.
To assure variables reference to original `then` and `catch` function.
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.
Thanks!
Issue #, if available:
#398
Description of changes:
Make it idempotent to capturePromise.
Add uncapturePromise function in order to cancel patch in tests.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.