add sinon-bluebird to make promises in tests a bit cleaner #3000
Conversation
There was a problem hiding this comment.
Great addition @dougmoscrop 👍
Makes the whole test code a lot easier to read.
Just saw one require (added a comment about it) which might be removed if not needed. Otherwise it's GTM from my side!
Edit: Just tested it w/o this require, which works. Pushed an update to this branch which removes it.
| const Package = require('../index'); | ||
| const Serverless = require('../../../../lib/Serverless'); | ||
|
|
||
| require('sinon-bluebird'); |
There was a problem hiding this comment.
Why is sinon-bluebird required here?
|
Thanks, yeah that was my bad. |
|
No worries! Thanks for the PR @dougmoscrop 💯 |
|
Fixed conflicts |
|
Fixed conflicts / rebased - cc @pmuens again :D |
|
Thanks @dougmoscrop 👍 /cc @eahefnawy Edit: We might want to do a follow-up PR because other tests have been written which might still use the old approach... |
pmuens
left a comment
There was a problem hiding this comment.
Just pushed a quick update so that all the recently added tests also use sinon-bluebird. Will merge after the tests pass.
Thanks for the addition @dougmoscrop 👍 💯
What did you implement:
Closes #2115
The promise rejections were causing fake 'unhandled rejection' messages to print during the tests. As the related issue says, there's nothing wrong, but this fixes it so that the messages do not get displayed.
How did you implement it:
Using
sinon-bluebird.How can we verify it:
npm test- only affects unit tests.Todos:
Is this ready for review?: YES
Is it a breaking change?: NO