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

Mock device fetcher #1681

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft

Mock device fetcher #1681

wants to merge 16 commits into from

Conversation

lawrence-forooghian
Copy link
Collaborator

No description provided.

@github-actions github-actions bot temporarily deployed to staging/pull/1681/features April 18, 2023 15:34 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1681/jazzydoc April 18, 2023 15:42 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1681/features April 18, 2023 17:20 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1681/jazzydoc April 18, 2023 17:26 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1681/features April 18, 2023 17:49 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1681/jazzydoc April 18, 2023 17:58 Inactive
The current logic assumes that the deviceRegistrations.save calls will
complete in the order that they are started (and hence, that when the
final device in the list has been registered, all of the devices have
been registered). This is not correct, and means that sometimes we end
up trying to create a channel subscription for a device that has not
been registered yet, causing the channelSubscriptions.save call to fail.
It doesn’t provide any functionality.
There aren’t any places where we use setupOptions in a way whose
behaviour differs from that of commonAppSetup.
Wasn’t adding anything useful.
It’s already covered in the clientOptions(…) method.
It’s already covered in the clientOptions(…) method. I’ve chosen to go
with .verbose as this feels more useful for someone wanting to debug a
misbehaving test.
The SDK doesn’t use this property any more.
This is just a refactor, but the intention is that it form part of #1601
(avoiding using global SDK state for controlling test behaviour) — we’ll
now be able to inject a mock device fetcher instead of having to use the
shared one.
nil is being passed in the tests, and then the Swift mock is seeing it
as an empty string
… Realtime

TODO describe better

i.e. just use the bits of TestEnvironment that are actually relevant to
the test
@github-actions github-actions bot temporarily deployed to staging/pull/1681/features April 18, 2023 18:40 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1681/jazzydoc April 18, 2023 18:47 Inactive
TODO are there tests that rely on using the same fetcher in both cases?

Confirmed (by adding logging) that DefaultLocalDeviceFetcher is not
being used in the test suite now, except for in its own tests.
Not quite sure why we’ve started needing this now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant