Prevent zombie presence events #208
Conversation
WalkthroughThis update includes several changes to the CI workflow and test functions within the Yorkie project. The main update relocates 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- .github/workflows/ci.yml (1 hunks)
- yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt (1 hunks)
- yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeSplitMergeTest.kt (16 hunks)
- yorkie/src/main/kotlin/dev/yorkie/document/Document.kt (4 hunks)
Additional comments not posted (5)
.github/workflows/ci.yml (1)
122-123: MovingCODECOV_TOKENfromwithtoenvis a good practice as it enhances the clarity and maintainability of the environment variables.yorkie/src/androidTest/kotlin/dev/yorkie/document/json/JsonTreeSplitMergeTest.kt (1)
8-8: Using a direct import forupdateAndSyncimproves readability by reducing verbosity, aligning with the DRY principle.yorkie/src/main/kotlin/dev/yorkie/document/Document.kt (2)
103-104: MarkingpresenceEventQueueas@VisibleForTestingand changing its visibility to internal is a prudent change for encapsulation and allows for better unit testing.
Line range hint
364-382: The logic to clear thepresenceEventQueuebased on certain conditions helps prevent zombie presence events. Ensure comprehensive tests cover these new conditions to avoid potential edge cases.Verification successful
The search results indicate that there are references to
presenceEventQueuein thePresenceTest.ktfile located in theyorkie/src/androidTest/kotlin/dev/yorkie/core/directory. This suggests that there are tests related to thepresenceEventQueue.
yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt: assertTrue(d1.presenceEventQueue.isEmpty())yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt: assertTrue(d2.presenceEventQueue.isEmpty())These references imply that the
presenceEventQueueis being tested to ensure it is empty under certain conditions. This aligns with the need to verify the clearing logic of thepresenceEventQueue.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the comprehensive coverage of tests related to presence event queue clearing. # Test: Search for tests related to presence event queue. Expect: Tests covering new conditions. rg --type kotlin $'presenceEventQueue' yorkie/src/androidTest/kotlin/dev/yorkie/document/Length of output: 88
Script:
#!/bin/bash # Description: Verify the presence of comprehensive tests related to presence event queue clearing. # Step 1: List all Kotlin test files in the repository echo "Listing all Kotlin test files in the repository..." fd --type f --extension kt --search-path yorkie/src/test/ > kotlin_test_files.txt fd --type f --extension kt --search-path yorkie/src/androidTest/ >> kotlin_test_files.txt # Step 2: Search for references to presenceEventQueue in all Kotlin test files echo "Searching for references to presenceEventQueue in Kotlin test files..." rg --type kotlin 'presenceEventQueue' $(cat kotlin_test_files.txt)Length of output: 3303
yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt (1)
830-874: The test functiontest_whether_presence_event_queue_is_empty_after_consecutive_presence_changeseffectively validates the clearing of the presence event queue after multiple presence updates. This is crucial for ensuring that zombie presence events do not persist in the system.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- yorkie/src/androidTest/kotlin/dev/yorkie/core/PresenceTest.kt
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/ci.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/ci.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/ci.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/ci.yml (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
What this PR does / why we need it?
Presence events are managed using the
presenceEventQueuein the Yorkie Android SDK. When_presencescollects a new value, it iterates through thepresenceEventQueue, filters for related presence events, and emits them into theeventStream. However, because_presencesis aStateFlow, when new values are emitted to_presencesrapidly, only the most recent value is collected, and only its accompanying presence event is emitted into theeventStream. This can leave other unprocessed presence events as zombies in thepresenceEventQueue, causing unintended behaviors.This PR addresses the issue by ensuring all presence events are either properly emitted or dropped.
Any background context you want to provide?
What are the relevant tickets?
Fixes #
Checklist
Summary by CodeRabbit
New Features
Chores
CODECOV_TOKENconfiguration to improve environment management.