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
Add sinon.promise() implementation #2369
Conversation
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.
This looks good 👍
Would you mind adding some documentation in the Jekyll site, so users can learn how to use it?
test/promise-test.js
Outdated
promise | ||
.then(function (val) { | ||
status = "fulfilled"; | ||
value = val; | ||
}) | ||
.catch(function (reason) { | ||
status = "rejected"; | ||
value = reason; | ||
}); |
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.
Is there any reason not to use await
wrapped in try/catch
instead of then/catch
on the promise?
promise | |
.then(function (val) { | |
status = "fulfilled"; | |
value = val; | |
}) | |
.catch(function (reason) { | |
status = "rejected"; | |
value = reason; | |
}); | |
try { | |
status = "fulfilled"; | |
value = await promise; | |
} catch (reason) { | |
status = "rejected"; | |
value = reason; | |
} |
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.
Yes, there is a good reason: We don't want to await
the promise, because we want to know if the promise is still pending
. I committed your suggestion, but then there's a test failure and I reverted.
Thanks for reviewing. Before spending time on writing documentation, I wanted to get some feedback. We can keep iterating on this PR for a bit until everyone is happy with the API and implementation. |
I have made some minor changes and added a documentation page. I cannot see the new documentation page locally for reasons I don't understand. |
@mantoni This is a problem with the docs setup: it does not have a way of showing |
Purpose (TL;DR)
This is an implementation of the feature request described in #2367 (
sinon.promise()
).Background
The state of a promise can only be controlled by the promise itself. Also, promises do not expose their state, which makes it difficult to work with them in tests.
Solution
The
sinon.promise([executor])
function returns a new promise with this additional API:status
: The internal status of the promise, one ofpending
,resolved
orrejected
.resolvedValue
: The promise resolved value.rejectedValue
: The promise rejected value.resolve(value)
: Resolves the promise with the given value. Throws if the promise is notpending
.reject(value)
: Rejects the promise with the given value. Throws if the promise is notpending
.A new promise can be created with a custom executor. This may be a
fake
orstub
instance, or a custom function. If the custom executor resolves or rejects the promise, the above fields are populated accordingly.Example:
Furthermore the
promise.resolve(value)
andpromise.reject(value)
functions return promises, so that the caller canawait
until the sinon promise has settled. These promises always resolve successfully.Example:
How to verify
npm install
npm test
Checklist for author
npm run lint
passes