-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add more Tree concurrency tests #195
Conversation
WalkthroughThe recent updates primarily focus on versioning and concurrency testing. The Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- gradle.properties (1 hunks)
- yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeConcurrencyTest.kt (4 hunks)
Files skipped from review due to trivial changes (1)
- gradle.properties
Additional comments not posted (6)
yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeConcurrencyTest.kt (6)
7-7
: Added import forStyleOpCode
is appropriate for the new test methods that use style operations.
15-15
: Added import forIgnore
annotation. Ensure it's used appropriately to skip tests that are not ready or are waiting for dependencies.
215-296
: New test methodtest_concurrent_style_and_style
added. This method tests concurrent style operations on text nodes. The test setup and assertions appear to be correctly implemented.
387-394
: Intest_concurrent_edit_and_style
, the references toStyleOpCode
have been updated correctly fromOpCode.StyleOpCode
toStyleOpCode.StyleRemove
andStyleOpCode.StyleSet
. This change aligns with the new import and simplifies the code.
412-515
: Thetest_concurrent_split_and_split
method is correctly marked with@Ignore
due to dependencies on the JS SDK. This is a good practice to prevent the test from failing builds while the dependent features are not implemented. The test logic itself, which involves complex nested elements and split operations, seems well-structured and should effectively test the intended concurrency scenarios once enabled.
517-652
: Thetest_concurrent_split_and_edit
method is also marked with@Ignore
for the same reasons as the previous test. The test setup involves complex operations combining splits and edits, which are crucial for testing the robustness of the concurrency model in the SDK. The method is well-prepared for future activation.
[APROVED]Verification successful
The recent commits and pull requests in the JS SDK repository indicate that significant work has been done on concurrency and edit operations, including the implementation of tree concurrency tests and handling concurrent edits. This suggests that the necessary features for the
test_concurrent_split_and_edit
test might now be in place.
- Commit
dc60203367b5bc19173023d7c0307e8becbe44d1
implementstree_concurrency_test.ts
.- PR
#792
adds tree concurrency tests.- PR
#805
handlesTree.toXML
to return proper XML string.- PR
#611
handles concurrent cases forYorkie.tree
.- PR
#709
supports concurrent insertion and splitting in Tree.- PR
#712
generates correctTreeChange
in concurrent edits.These updates suggest that the concurrency model and edit operations have been enhanced and tested in the JS SDK.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the JS SDK has implemented the necessary features for this test. # Test: Check for updates or issues related to this test in the JS SDK repository. gh issue list -R yorkie-team/yorkie-js-sdk --search "edit operations in concurrency tests"Length of output: 92
Script:
#!/bin/bash # Description: Check for recent commits in the JS SDK repository related to concurrency or edit operations. gh repo clone yorkie-team/yorkie-js-sdk cd yorkie-js-sdk git log --since="6 months ago" --grep="concurrency" --grep="edit operations" # Description: Check for recent pull requests in the JS SDK repository related to concurrency or edit operations. gh pr list -R yorkie-team/yorkie-js-sdk --search "concurrency" --state all gh pr list -R yorkie-team/yorkie-js-sdk --search "edit operations" --state allLength of output: 2856
What this PR does / why we need it?
I added more tests which were previously skipped on the JS SDK.
Any background context you want to provide?
What are the relevant tickets?
Fixes #
Checklist
Summary by CodeRabbit
New Features
Chores
0.4.21-rc
.