Add tests for edge cases of multiple membership changes in sync period #787
Conversation
Addresses element-hq/synapse#14683 case seen in the real world.
tests/csapi/sync_test.go
Outdated
| // Alice re-invites Bob | ||
| alice.MustInviteRoom(t, roomID, bob.UserID) | ||
|
|
||
| // 4. Bob does a sync | ||
| jsonRes, _ := bob.MustSync(t, client.SyncReq{Since: bobSince}) |
There was a problem hiding this comment.
This doesn't guarantee that the leave/invite will be federated to Bob's homeserver by the time Bob syncs.
Perhaps we need another hs2 user in the room to observe first
If we just use bob.MustSyncUntil(t, client.SyncReq{Since: bobSince}, client.SyncInvitedTo(bob.UserID, roomID)) the leave and invite could be separated in different sync responses as we paginate.
There was a problem hiding this comment.
Very good point. Another user would work. I'll modify the test to include that.
tests/csapi/sync_test.go
Outdated
|
|
||
| // Wait until Bob is kicked. | ||
| aliceSince := alice.MustSyncUntil(t, client.SyncReq{}, client.SyncLeftFrom(bob.UserID, roomID)) | ||
| charlieSince = charlie.MustSyncUntil(t, client.SyncReq{Since: charlieSince}, client.SyncLeftFrom(bob.UserID, roomID)) |
There was a problem hiding this comment.
I don't think we need to worry about getting a since token in any of these places execpt for bob which we are actually testing incremental sync for.
The other sync's are just asserting pre-conditions.
| charlieSince = charlie.MustSyncUntil(t, client.SyncReq{Since: charlieSince}, client.SyncLeftFrom(bob.UserID, roomID)) | |
| charlie.MustSyncUntil(t, client.SyncReq{}, client.SyncLeftFrom(bob.UserID, roomID)) |
tests/csapi/sync_test.go
Outdated
|
|
||
| // Wait until Bob is kicked. | ||
| aliceSince := alice.MustSyncUntil(t, client.SyncReq{}, client.SyncLeftFrom(bob.UserID, roomID)) | ||
| charlieSince = charlie.MustSyncUntil(t, client.SyncReq{Since: charlieSince}, client.SyncLeftFrom(bob.UserID, roomID)) |
There was a problem hiding this comment.
Add a comment about why we wait here as well
(needs wrapping)
| charlieSince = charlie.MustSyncUntil(t, client.SyncReq{Since: charlieSince}, client.SyncLeftFrom(bob.UserID, roomID)) | |
| // Wait until hs2 sees that Bob has been kicked. If we don't wait, then we have no idea whether `hs2` received the kick event. | |
| charlieSince = charlie.MustSyncUntil(t, client.SyncReq{Since: charlieSince}, client.SyncLeftFrom(bob.UserID, roomID)) |
Addresses element-hq/synapse#14683 case seen in the real world.
Requires element-hq/synapse#18648 for these tests to pass in Synapse.
Pull Request Checklist