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 TestDeviceListUpdates flakes, again #610

Merged
merged 1 commit into from
Feb 14, 2023

Conversation

squahtx
Copy link
Contributor

@squahtx squahtx commented Feb 14, 2023

When a homeserver (Synapse) performs a partial state join to a room, it
may emit device_lists.changed entries for users it thinks are in the
room, despite the room not appearing in the /sync response yet. This is
questionable behaviour, but not forbidden by the spec. Once the
homeserver is fully joined to the room, it may emit another round of
device_lists.changed entries for users it knows are definitely in the
room.

These extra entries were causing test cases to advance prematurely and
randomly fail. To fix the tests, we explicitly check for join events
alongside the device_lists.changed entries produced by joins. In
effect, we ensure that homeservers are fully joined to the test rooms
before continuing the rest of the tests.

For consistency, we also check for leave events alongside
device_lists.left entries.


Fixes matrix-org/synapse#14103.

When a homeserver (Synapse) performs a partial state join to a room, it
may emit `device_lists.changed` entries for users it thinks are in the
room, despite the room not appearing in the /sync response yet. This is
questionable behaviour, but not forbidden by the spec. Once the
homeserver is fully joined to the room, it may emit another round of
`device_lists.changed` entries for users it knows are definitely in the
room.

These extra entries were causing test cases to advance prematurely and
randomly fail. To fix the tests, we explicitly check for join events
alongside the `device_lists.changed` entries produced by joins. In
effect, we ensure that homeservers are fully joined to the test rooms
before continuing the rest of the tests.

For consistency, we also check for leave events alongside
`device_lists.left` enrties.
Copy link
Contributor

@DMRobertson DMRobertson left a comment

Choose a reason for hiding this comment

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

Huge thanks for investigating this!

@squahtx squahtx merged commit f528a63 into main Feb 14, 2023
@squahtx squahtx deleted the squah/fix_device_lists_tests_flakes_2 branch February 14, 2023 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Complement TestDeviceListUpdates/when_remote_user_joins_a_room flake(?)
2 participants