This repository has been archived by the owner on Apr 4, 2022. It is now read-only.
Remove chai-as-promised and convert tests to async/await #690
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@leplatrem would love a quick spot check on this! For the most part it's exactly the same, so just skimming the changes and checking a few spots would be really appreciated! |
leplatrem
approved these changes
Feb 27, 2020
test/bucket_test.ts
Outdated
| return getBlogBucket() | ||
| .getData() | ||
| .should.become({ foo: "bar" }); | ||
| ((await getBlogBucket().getData()) as { foo: string }).should.deep.equal({ |
There was a problem hiding this comment.
Maybe in this case, an intermediary variable would make sense
There was a problem hiding this comment.
I've updated the tests to use an intermediary variable whenever we were casting an awaited value. However, we still have tests that look like this:
(await api.fetchServerInfo()).should.deep.equal(fakeServerInfo);Is this clear enough, or should we lift all awaited calls into variables?
There was a problem hiding this comment.
Yes, this was like one of the few complex ones. Thanks for the changes!
leplatrem
approved these changes
Feb 27, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In preparation for running tests in an actual browser environment, I've refactored all the tests that relied on
chai-as-promisedto async/await. In the process I discovered several tests that weren't waiting on promises correctly, and were throwing unhandled rejections.Removing
chai-as-promisedwas necessary to get the tests passing in Intern. I'll send a future PR actually adding Intern, but I figured this would be a good PR to merge even without Intern.