test(talk): fetch rooms once before checking modifiedSince - #459
Merged
Conversation
Talk captures the timestamp it returns in X-Nextcloud-Talk-Modified-Before at the start of getRooms, and only then dispatches BeforeRoomsFetchEvent, whose listeners create the "Note to self", sample and changelog conversations for a user seeing Talk for the first time. Those rooms end up with a lastActivity newer than the timestamp already reported, so the test's window of "nothing was modified since T + 2 seconds" only held while that creation finished within two seconds. On a loaded CI runner it does not, and the returned sample rooms fail the assertion, which is what made the matrix jobs flaky. Fetch the rooms once before the timed sequence, so the lazy creation happens outside the window that is checked. Signed-off-by: Oleksandr Piskun <oleksandr2088@icloud.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe synchronous and asynchronous modified-since tests now fetch the conversation list before creating test conversations. This prevents existing Talk-created default rooms from affecting modification-window assertions. ChangesConversation test stability
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
test_get_conversations_modified_sincefails intermittently across the matrix (seen on #458 and earlier onmain), always asassert not [<Conversation name=Note to self>, <Conversation name=Let's get started!>].Talk captures
X-Nextcloud-Talk-Modified-Before(T) at the start ofgetRoomsand only then dispatchesBeforeRoomsFetchEvent(RoomController.php:240-243), whose listeners create the note-to-self, sample and changelog conversations the first time a user fetches rooms. Those rooms get alastActivitynewer than the T already reported, and the server filter islastActivity >= modifiedSince. The test asks forT + 2, so it only passes while that creation finishes inside 2 seconds. Measured on an idle local server it already takes ~1.15 s.Fetch the rooms once before the timed sequence so the lazy creation happens outside the checked window.
Reproduced end to end by making sample creation exceed the margin (Talk's
samples_directory), then running the real test as a fresh admin user: current test fails, patched test passes, same server and conditions.Summary by CodeRabbit