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

Additional tests for collections in mixed #6648

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

Conversation

papafe
Copy link
Contributor

@papafe papafe commented May 2, 2024

The new tests are all passing if run one their own, but there is a crash when running multiple tests. This is reported in realm/realm-core#7657

This PR should be picked up only when the core issue has been solved.

elle-j and others added 30 commits April 26, 2024 14:38
* Differentiate use of 'mixedToBinding()' for query arg validation.

* Refactor 'mixedFromBinding()' to own function and account for List and Dictionary.

* Implement setting a flat list and dictionary in Mixed.

* Implement accessing a flat list and dictionary in Mixed.

* Add tests for storing and accessing flat lists and dictionaries in Mixed.

* Refactor helper in test to not rely on collection position.

* Add tests for Set in Mixed throwing.

* Add tests for updating lists and dictionaries.

* Add tests for removing items in lists and dictionaries.

* Add tests for filtering lists and dictionaries by path.

* Throw if adding a set via property accessors.

* Group tests into separate sub-suites.

* Guard for embedded objects being set as Mixed value.

* Add tests for embedded objects in Mixed throwing.

* Add more filtering tests.

* Add 'at_keys' query tests to uncomment after Core bug fix.

* Add tests for inserting into lists and dictionaries.

* Add tests for notifications on lists.

* Add tests for notifications on dictionaries.

* Add tests for notifications on object when changed prop is list in mixed.

* Add tests for invalidating old list and dictionary.

* Minor updates to names.

* Add tests for notifications on object when changed prop is dictionary in mixed.

* Add tests for creating dictionary via object without prototype.

* Add tests filtering by query path using IN.

* Access array of changes only 1 time in notifications tests.

* Remove unnecessary type assertion.

* Update schema object names in tests.

* Add link to Core bug.

* Add tests for default list and dictionary in schema.

* Add tests for setting lists and dictionaries outside transaction.

* Add tests for spreading Realm and non-Realm objects as Dictionary.

* Add unit tests for 'isPOJO()'.

* Point to updated Core commit and enable related tests.

* Wrap chai's 'instanceOf()' in custom helper to assert type.

* Update helper function name to be consistent with other helpers.

* Add internal docs for 'isQueryArg'.

* Rename unit test file.

* Refactor notification tests into 'observable.ts'.

* Refactor notification tests to use test context.

* Use named import of 'ObjectSchema'.

* Group CRUD tests into subsuites.
…ions (#6513)

* Move geospatial helper functions to related file.

* Implement setting nested lists in Mixed.

* Implement setting nested dictionaries in Mixed.

* Implement getting nested lists in Mixed.

* Implement getting nested dictionaries in Mixed.

* Test creating and accessing nested lists and dicts.

* Make previous flat collections tests use the new 'expect' function.

* Test that max nesting level throws.

* Delegate throwing when using a Set to 'mixedToBinding()'.

* Implement setting nested collections on a dictionary via setter.

* Test nested collections on dictionary via setter.

* Minor update to names of tests.

* Combine nested and flat collections tests into same suite.

* Implement setting nested collections on a list via setter.

* Test nested collections on list via setter.

* Refactor common test logic to helper functions.

* Optimize property setter for hot-path and use friendlier err msg.

* Refactor test helper function to build collections of any depth.

* Implement inserting nested collections on a list via 'push()'.

* Test nested collections on a list via 'push()'.

* Test updating dictionary entry to nested collections via setter.

* Test updating nested list/dictionary item via setter.

* Test removing items from collections via 'remove()'.

* Test object notifications when modifying nested collections.

* Group previous notification tests into one test.

* Group collection notifications tests into 'List' and 'Dictionary'.

* Test collection notifications when modifying nested collections.

* Remove collections from test context.

* Test filtering by query path on nested collections.

* Align object schema property names in tests.

* Test filtering with int at_type.

* Implement setting nested collections on a dictionary via 'set()' overloads.

* Test JS Array method 'values()'.

* Test JS Array method 'entries()'.

* Implement getting nested collections on dictionary 'values()' and 'entries()'.

* Test 'values()' and 'entries()' on dictionary with nested collections.

* Remove unnecessary 'fromBinding()' calls.

* Refactor collection helpers from 'PropertyHelpers' into the respective collection file.

* Introduce list/dict sentinels to circumvent extra Core access.

* Rename getter to default.

* Remove redundant 'snapshotGet'.

* Add abstract 'get' and 'set' to 'OrderedCollection'.

* Rename the collection helpers to 'accessor'.

* Move tests into subsuites.

* Fix 'Results.update()'.

* Support nested collections in 'pop()', 'shift()', 'unshift()', 'splice()'.

* Test list 'pop()'.

* Test list 'shift()'.

* Test list 'unshift()'.

* Test list 'splice()'.

* Return 'not found' for collections searched for in 'indexOf()'.

* Test ordered collection 'indexOf()'.

* Support list/dict sentinels in JSI.

* Test references per access.

* Enable skipped tests after Core bug fix.

* Point to updated Core.

* Fix accessor for non-Mixed top-level collection with Mixed items.

* Enable and fix previously skipped test.

* Update 'mixed{}'.

* Update 'mixed<>'.

* Remove now-invalidated test.

* Remove unused injectable from Node bindgen template.

* Replace if-statements with switch.

* Add explicit Results and Set accessors for Mixed.

* Adapt to change in Core treating missing keys as null in queries.

* Rename insertion function.

* Include tests of Dictionary property type with Mixed.

* Test reassigning to new collection and self-assignment.

* Test mixed

* Update 'mixed[]'.

* Test results accessor.

* Update error messages.

* Make accessor helpers an object field rather than spread.

* Suggestions for "nested collections in mixed" (#6566)

* Fix type bundling issue

* Inline functions into "create*Accessor*" functions

* Refactored typeHelpers out of accessors

* Remove leftover 'Symbol_for' in node-wrapper template.

* Test not invalidating new collection.

* Remove test for max nesting level.

The max nesting level in debug in Core has been updated to be the same as for release.

* Remove reliance on issue-fix in certain tests.

* Add key path test for object listener on mixed field.

* Use '.values()' and '.entries()' in iteration.

* Update comments.

* Add CHANGELOG entry.

---------

Co-authored-by: Kræn Hansen <kraen.hansen@mongodb.com>
Co-authored-by: LJ <81748770+elle-j@users.noreply.github.com>
…xed-sync-tests-additional

* fp/collections-mixed-sync-tests:
  Added comment
  Removed only
  Corrected testing
  trying to improve testing
  Removed realm use
  Remove unused

# Conflicts:
#	integration-tests/tests/src/tests/sync/mixed.ts
@cla-bot cla-bot bot added the cla: yes label May 2, 2024
Base automatically changed from fp/collections-mixed-sync-tests to lj/collections-in-mixed May 8, 2024 08:12
@papafe
Copy link
Contributor Author

papafe commented May 8, 2024

realm/realm-core#7657 has been fixed, and I verified that it fixes the crashes. It has been merged into master, so we should rebase this branch onto master when possible.

There's an additional issue though unfortunately, that was also present before but I think it was covered by the crashes.
When running the tests on their own, they all pass but when running them together:

  • The second test usually encounters a timeout. I have verified that this happens because we don't get correct notifications. In all the test methods we create a promise with getWaiter that gets resolved when we get a notification for the property called value. The timeout happens because we get two empty notifcations from core (the first one is expected);
  • Resolving the promise on the second notification (even if the notification is empty) results in Accessing object which has been invalidated or deleted;
  • Trying to use the sleep method, instead of the promise results in Accessing object which has been invalidated or deleted;

These timeouts are also flaky as they don't happen all the time. This makes the investigation quite annoying though.

@elle-j elle-j force-pushed the lj/collections-in-mixed branch 3 times, most recently from 6ddda98 to 233948d Compare May 23, 2024 09:54
Base automatically changed from lj/collections-in-mixed to main May 23, 2024 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants