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

Migrate and reenable sync realm integration test #6939

Merged
merged 20 commits into from
Sep 15, 2020

Conversation

rorbech
Copy link
Contributor

@rorbech rorbech commented Jun 10, 2020

Fixed issues:

  • OS does not support custom headers. Implemented Java-rework until OS support is in place.

A number of tests requires sessions to be closed, but we loose grip of them as tests intentionally triggers exceptions on getInstance. Already tracked in #5416. Tried incorporating shutdown_and_wait from realm/realm-object-store#1055 but did not solve the issue. Maybe dependant on realm/realm-object-store#1054 too.

@RealmBot RealmBot mentioned this pull request Jun 23, 2020
14 tasks
@rorbech rorbech requested a review from cmelchior June 30, 2020 08:42
@rorbech rorbech marked this pull request as ready for review August 7, 2020 14:03
try {
assertTrue(Realm.deleteRealm(config))
} catch (e: IllegalStateException) {
// FIXME: We don't have a way to ensure that the Realm instance on client thread has been
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I think I saw something about this being fixed. Try talking to @RedBeard0531

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The old issue #5416 is still open. I have tried incorporating the shutdown_and_wait from realm/realm-object-store#1055 but had various issue around it. In some of the other tests referring to #5416 I was not able to get hold of the session and other places like here it did not work out. Reapplied a workaround from another test by retried to close if it fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe related to some internal notification triggering to early due to realm/realm-object-store#1054

Copy link
Collaborator

@clementetb clementetb left a comment

Choose a reason for hiding this comment

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

Looks good 🧙🏻‍♂️! Have some questions/points

@rorbech rorbech requested a review from clementetb September 8, 2020 07:22
@rorbech rorbech merged commit 26685d1 into v10 Sep 15, 2020
@rorbech rorbech deleted the cr/sync-integration-test-realm branch September 15, 2020 10:58
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
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.

4 participants