-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Rewrite tests that use subscribeAndCount
with ObservableStream
#12163
Conversation
|
✅ Docs Preview ReadyNo new or changed pages found. |
commit: |
size-limit report 📦
|
5d319bd
to
a0c0ba5
Compare
a0c0ba5
to
83a35d8
Compare
✅ Deploy Preview for apollo-client-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for apollo-client-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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 to me!
While I was reviewing this, I did refactor a few more tests and did some tweaks here and there, so please give 67a6a42 a quick review before merging this :)
for (const result of results) { | ||
await expect(observableStream).toEmitValue(result); | ||
} |
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.
Love it!
This PR updates all tests that use the
subscribeAndCount
test utility in favor of the much more useableObservableStream
helper. Tests are much more serial as a result and easier to follow since its easy to understand which order the assertions run relative to the checked behavior.As a part of this PR, I've added several new matchers that allow us to check the emitted value directly, rather than using the block
{ }
trick to reuse the same variable name.Since we plan to migrate to RxJS in 4.0, this change should make it much easier to determine what breaks and what doesn't.
NOTE: I'm opening in draft as this will be one of the first PRs that I'd like to merge in to
main
after 3.12 is released. We should deprecate and remove this function for 4.0 so that we no longer have to maintain it.