Raise default room version to "12" - #20130
Draft
jason-famedly wants to merge 34 commits into
Draft
Conversation
…ederationTestCase to be room version arbitrary Should be able to handle room version "12" and at least some version prior. Made the default room version a class attribute. Correct for depth miscalculation(rooms start at 1 not 0). Use make_event_from_dict() to handle which FrozenEvent class is used. Only add in auth_events to source event dicts if appropriate, same for room_id when it is generated by the event hash(msc4921). Drive-by: Don't accidentally shadow `create_event()` which is an imported function.
…estCase to gracefully handle room creator power levels during room creation
…gy power levels to handle upgrading a v9 room to a room with msc4289 by using the other user instead of the room's creator
…el handling when upgrading room to accommodate for the tombstone event power being higher than before room v12
…ult room version of server The MSC for this does not appear to be ready for FCP, and hence has not been merged. It does not seem to be explicit about what room versions it entails.
…that auth_events no longer contains the creation event ID for room version 12
… room creator when ban/kick is applied and redaction powerlevel is too low
…oving the 'users' object in the power level event properly
…ls correctly in msc4289 compliant rooms by removing the room creator from the 'users' object
Use separate rooms for the two tests. The main issue was test_send_dummy_event(), as a room creator present in the room will always have the power level to send dummy events to tie up extremities. This needed to be changed to another user, so the room creator could leave the room to prove the test was accurate
…ser instead of the room's creator to puppet giving admin rights
…r room version 12
…room version 11 and 12 Also recall that depth for rooms starts at 1
…msRequiredStateTestCase for room version 12
…on event from the fake remote event auth_events
…s having creation event in auth_events
…ault room version
…ion to not use hard coded prev_events and auth_events The `create_event()` function can look these up properly so assumptions do not have to be made about how the previous events are *probably* also the auth_events. Just calculate the correct values as they are not part of the test itself
…_join_room_preset_invalid_permissions() to room version 11 A new default of room version 12 means that this test case can not happen as is. The room creator's power level can not be moved from infinite so will always have the permission to invite a new member
…s.test_process_pulled_event_with_rejected_missing_state() by introducing a member that can match power levels with the remote member sending rejected power level events I believe this is caused by the room creator being the member to send the final power levels event. It failed the reverse topological sort in inverted order(the two events were swapped places). Since the room creator has an infinitely higher power level than the remote user, the reverse topological sort should have fallen to the origin_server_ts as the last fallback and it was not
…(moved the 'redacts' key in v11)
…ding a KeyError during logging about an unknown/missing room version This occurs because `m.room.create` events do not have a `room_id` in their JSON form, so that data is missing from the dictionary and a `KeyError` is raised.
… event/room V12 compliant
…c/test_rooms_invites.SlidingSyncRoomsInvitesTestCase to use 'stripped state' to compare against responses instead of fleshing out a full PDU
…ally make rooms on each `events` worker
… by removing creation events from auth_events
Contributor
Author
|
I suspect the complement tests will fail until gomatrixserverlib is bumped in its dependencies. The unit tests though, these errors are some I was getting locally(but somehow stopped in the last week) and could not figure out how it is happening. They are all related to sliding sync and appear to be the sorting on the stream ID being backwards of expected values? Do not quote me on that though. https://github.com/element-hq/synapse/actions/runs/32282679358/job/96165453636 were the tests that flaked during testing(there may have been more such as the fallback versions that are not in my notes, I didn't start writing them down until some time after), no noticeable consistency or pattern other than system load https://github.com/element-hq/synapse/actions/runs/32282679358/job/96165405724 EDIT 2: |
The test harness caches parsed HomeServerConfig objects and hands each test a copy. deepcopy_config recursed into Config sub-objects but assigned every other attribute by reference, so mutable containers such as rooms_to_exclude_from_sync were shared between the cached template and every homeserver built from it. A test that mutates such a value in place (test_exclude_rooms_from_sync appends a room ID) leaked it into all later tests in the process. This went unnoticed because the leaked room IDs were random and never matched a room in a later test. With room version 12 as the default, room IDs are derived from the create event, which is identical from test to test under the fake clock, so the leaked ID matched the first room of every subsequent test and broke the sliding sync list tests.
Contributor
Author
|
I had a conversation with the author of the more recent pull request to address the above test issue. With @barodeur 's permission, I will cherry-pick commit 64ea986 from that branch which should solve the set of test flakes above I did not know what to do with. I'm pleased that it was only a test infrastructure oversight and not something incorrect about sliding sync's behavior itself |
3 tasks
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
2 participants
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.
Requires
check_event_allowedcallback to function for room version "12" creation events #19768To meet the requirements for bumping Synapse to support Matrix spec to 1.16 the default room version should be incremented to "12".
Other than the two separate Synapse PRs above(which are included here but marked as "[diverted]" and should be removed prior to review) there is only one other real change to the code base itself to fix a
KeyErrorduring logging for a/synctest against unknown room versions. Everything else should be on the unit tests themselves.Standard unit test running applies, should be nothing special to test this outright.
poetry run trial -jN testsand similar for Postgresql.Probably ok to review commit-by-commit
I took the liberty of writing a room creating helper for the two test series that try and test the sharding of the
event_persisterworkers. I'm not certain it stands up to scrutiny, but at least does not do any funny mocking when producing room v12 appropriate room IDs.I also took the liberty of writing an assertion helper for comparing lists of dicts for a select subset of keys/values. This is used to compare stripped state selections while waiting on #19723 to be completed.
Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.