-
Notifications
You must be signed in to change notification settings - Fork 55
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
Add test for ensuring power levels are correctly handled during upgrade #777
Conversation
tests/30rooms/60version_upgrade.pl
Outdated
# Check that the power levels in the old room have not changed | ||
# by making sure there is not a power levels event for the old room | ||
# in the sync response | ||
my $old_room = $sync_body->{rooms}{join}{$room_id}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have you checked this actually failed under the old code?
My suspicion is that it will not, since the bug is that we are modifying the existing PL event, rather than creating a new one. Maybe try fetching the state with /r0/room/{roomId}/state/m.room.power_levels
or sth?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does fail with:
| Got JSON::number(100), expected JSON::number(50) at {users}{@anon-20200114_100001-7:localhost:8800} for power levels in replacement room at tests/30rooms/60version_upgrade.pl line 435.
Edit: But this is only failing on the power levels being incorrect in the new room, without mention of the bug in the old room.
I'll try switching it around to use the state endpoints instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switched. Test still fails on old code, passes on new.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@@ -337,7 +338,7 @@ sub upgrade_room_synced { | |||
matrix_change_room_power_levels( | |||
$creator, $room_id, sub { | |||
( $pl_content ) = @_; | |||
$pl_content->{users}->{'@test:xyz'} = 40; | |||
$pl_content->{users}->{'@test:xyz'} = JSON::number(40); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh heh, that's what I fixed with #789. Arguably yours is a better fix.
…ease-v1.10.x * origin/release-v1.10.0: remove checks on m.room.aliases (#801) Remove test which uses unspecced /events?dir=b (#794) Add some tests to check for adding/removing aliases (#792) Add test for ensuring power levels are correctly handled during upgrade (#777) Test that fetching device correctly resyncs (#791) Add support for experimental MSC2260 room version (#790) Fix power levels being sent as strings (#789) Don't respond with 503 as server will be marked as down. (#788) Test Synapse with MAU limits enabled. (#782) Fix docker/push.sh and make it echo commands Change postgres in docker image to use C collation/ctype. (#783) Add a passing test for recovery from gappy device list updates (#761)
Synapse PR: matrix-org/synapse#6633