-
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 transferring bans on a room upgrade #563
Conversation
f86f30a
to
956ea4a
Compare
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.
Looks good, apart from we can use helper functions and we should check in state
section
tests/30rooms/60version_upgrade.pl
Outdated
|
||
my $event = first { | ||
$_->{type} eq "m.room.member" && $_->{state_key} eq '@bob:matrix.org', | ||
} @{ $room->{timeline}->{events} }; |
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.
We should probably check if its in the state
section, as that's also a valid place.
On top of that, we can write this entire thing using await_sync_timeline_contains
:
await_sync_timeline_contains( $user, $room, check => sub {
return unless $event->{type} eq "m.room.member";
return unless $event->{state_key} eq "@bob:matrix.org";
assert_deeply_eq(
$event->{content},
$content,
"ban in replacement room",
);
return 1;
});
Which makes the test a lot shorter, and stops you from having to do the manual syncs.
We will have to create a new await_sync_timeline_or_state_contains
function, but that is fairly trivial to do.
tests/30rooms/60version_upgrade.pl
Outdated
return 1; | ||
}); | ||
|
||
Future->done(1); |
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.
Remove this, as otherwise you're not using the result of await_sync_timeline_or_state_contains
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.
Ah lol. Happy to see that wasn't the reason it passed.
…server_specific_tests * 'develop' of github.com:matrix-org/sytest: (44 commits) test endpoint for updating backup versions (#559) Add test for transferring bans on a room upgrade (#563) fix tests for matrix-org/synapse#2090 (#350) Fix typo Fixup Incorporate review Remove prev_state from federation API Make the userdir synced not rely on being able to search for yourself (#567) Add basic soft fail test Check that event_id is given over state fetching over federation (#566) Fix registration rate limiting settings Fix comments Make sytest support auth rate limiting Fixup to not send 100 messages Test history visibility works for backfill Check that Server ACLs are preserved on room upgrade Better diagnostics from synapse startup (#561) Don't set PYTHONPATH when running synapse (#560) Regression test for redactions in room v3 (#558) Add test for PDU limit on transactions API (#555) ...
Synapse PR: matrix-org/synapse#4642