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

Fix duplicate leave events #297

Merged
merged 6 commits into from
Sep 13, 2023
Merged

Fix duplicate leave events #297

merged 6 commits into from
Sep 13, 2023

Conversation

S7evinK
Copy link
Collaborator

@S7evinK S7evinK commented Sep 8, 2023

Still investigating, but doesn't even seem to require multiple pollers as per #250

@S7evinK S7evinK added the bug Something isn't working label Sep 8, 2023
@S7evinK S7evinK marked this pull request as ready for review September 13, 2023 10:56
@S7evinK S7evinK changed the title WIP: Fix duplicate leave events Fix duplicate leave events Sep 13, 2023
res = bob.SlidingSyncUntilMembership(t, res.Pos, inviteRoomID, bob, "leave")

if room, ok := res.Rooms[inviteRoomID]; ok {
// If alice is NOT syncing, we run into this failure mode
Copy link
Member

Choose a reason for hiding this comment

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

Can you confirm what part of this test was failing without the diff to user.go?

Copy link
Collaborator Author

@S7evinK S7evinK Sep 13, 2023

Choose a reason for hiding this comment

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

If Alice is NOT syncing, we receive the leave event twice, once through Accumulate and once through OnLeftRoom.

If Alice IS syncing however, the test fails for exactly the same reason - we receive the same event twice, but at a different place in the test. We're syncing as Bob until we left the room, after that we do another SlidingSync call, which again receives the event.

https://github.com/matrix-org/sliding-sync/actions/runs/6121433636/job/16615233253#step:7:41 was one of the failing tests.

tests-e2e/membership_transitions_test.go Outdated Show resolved Hide resolved
tests-e2e/membership_transitions_test.go Outdated Show resolved Hide resolved
@S7evinK S7evinK merged commit 2a8e48c into main Sep 13, 2023
7 checks passed
@S7evinK S7evinK deleted the s7evink/dupeleave branch September 13, 2023 13:41
@S7evinK S7evinK mentioned this pull request Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants