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

RJS-2204: Re-enabled client reset tests #6738

Merged
merged 3 commits into from
Jun 25, 2024
Merged

Conversation

kraenhansen
Copy link
Member

@kraenhansen kraenhansen commented Jun 19, 2024

What, How & Why?

Fixes #5500 by calling the admin API to perform client reset.
This also brings a refactor of the app importer to take the client as an argument, since we want to instantiate and call this outside of the context of importing apps.

Note: This should be rebased once #6733 merge.

throw new Error(`Cannot trigger client reset in ${maxAttempts} attempts`);
await baasAdminClient.ensureLogIn();
const { _id } = await baasAdminClient.getAppByClientAppId(app.id);
syncSession.pause();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a connection listener to ensure it is safe to call forceSyncReset()?

Copy link
Member Author

Choose a reason for hiding this comment

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

Safe in what way? That we have access to BaaS from the test suite or perhaps that the Realm is ready to get the reset? (for the latter we are doing a await syncSession.uploadAllLocalChanges() before calling triggerClientReset).

Copy link
Contributor

Choose a reason for hiding this comment

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

pause() will not immediately pause the connection, and I wonder if we need to wait for it before calling forceSyncReset()?

Copy link
Member Author

@kraenhansen kraenhansen Jun 21, 2024

Choose a reason for hiding this comment

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

Ah 🤔 I honestly don't know .. I think it's safe to assume resume will internally block for anything async that pause started on the sync client thread. (I just copied this from the C++ SDK's test).

Base automatically changed from kh/sync-sessin-file-ident to main June 20, 2024 12:07
@kraenhansen kraenhansen marked this pull request as ready for review June 20, 2024 12:09
@kraenhansen kraenhansen requested a review from elle-j June 20, 2024 12:09
Copy link
Contributor

@elle-j elle-j left a comment

Choose a reason for hiding this comment

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

LGTM 💪

@kraenhansen kraenhansen merged commit b473415 into main Jun 25, 2024
36 checks passed
@kraenhansen kraenhansen deleted the kh/client-reset-tests branch June 25, 2024 14:51
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reenable "client reset" tests
3 participants