Skip to content
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

Allow multiple subscribers to a MockSubscriptionLink #6081

Conversation

dfrankland
Copy link
Contributor

@dfrankland dfrankland commented Mar 24, 2020

The behavior of MockSubscriptionLink differs from that of WebSocketLink. Specifically, MockSubscriptionLink doesn't allow multi subscriptions, which can make testing super confusing if you have a couple of components using useSubscription.

This PR adds an array of observables to MockSubscriptionLink for it to publish to when a simulateResult or simulateComplete method is called. In its current form it can have a memory leak because the array of observables is never cleaned up. This was the case prior as well, but moving forward should we remove observables that are complete or encounter an error?

@apollo-cla
Copy link

@dfrankland: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@codecov
Copy link

codecov bot commented Mar 24, 2020

Codecov Report

Merging #6081 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6081   +/-   ##
=======================================
  Coverage   95.36%   95.36%           
=======================================
  Files          89       89           
  Lines        3664     3667    +3     
  Branches      903      871   -32     
=======================================
+ Hits         3494     3497    +3     
  Misses        146      146           
  Partials       24       24           
Impacted Files Coverage Δ
.../utilities/testing/mocking/mockSubscriptionLink.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b9fdfb...6d09e57. Read the comment docs.

@hwillson hwillson added this to the Release 3.0 milestone Apr 23, 2020
@dfrankland dfrankland force-pushed the fix-mocksubscriptionlink-multiple-subscriptions branch from 4617da1 to f1786a5 Compare May 8, 2020 01:38
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dfrankland!

@hwillson hwillson merged commit 190a7f6 into apollographql:master Jul 10, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants