Skip to content

Commit

Permalink
Attempt to deflake a sytest (#816)
Browse files Browse the repository at this point in the history
This test was a bit flaky. The trouble was that there were three things that
could triger a deice update notification:
 * Joining the room
 * Updating the device keys
 * Changing display name

... but we only checked for two update notifications, and didn't sanity-check
that they were matching up. By adding another poll for update notifications, we
can avoid getting out of sync.

I also don't think we need the display_name update here.

We can also tighten up the final check by making sure that the departed user
doesn't appear in *any* syncs between the departure and the undeparted user's
update arriving.
  • Loading branch information
richvdh authored Feb 21, 2020
1 parent 676972a commit feef3e1
Showing 1 changed file with 57 additions and 15 deletions.
72 changes: 57 additions & 15 deletions tests/41end-to-end-keys/06-device-lists.pl
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,8 @@ sub sync_until_user_in_device_list
matrix_create_room( $creator )->then( sub {
( $room_id ) = @_;

matrix_sync( $creator );
})->then( sub {
matrix_invite_user_to_room( $creator, $remote_leaver, $room_id )
})->then( sub {
matrix_join_room( $remote_leaver, $room_id );
Expand All @@ -355,38 +357,78 @@ sub sync_until_user_in_device_list
})->then( sub {
matrix_join_room( $remote2, $room_id );
})->then( sub {
matrix_sync( $creator );
log_if_fail "Created and joined room";

# make sure we've received the device list update for remote_leaver's
# join to the room, otherwise we could get out of sync.
sync_until_user_in_device_list( $creator, $remote_leaver );
})->then( sub {
matrix_put_e2e_keys( $remote_leaver )

# there must be e2e keys for the devices, otherwise they don't appear in /query.
matrix_put_e2e_keys( $remote2, device_keys => { x => "x" } );
})->then( sub {
matrix_put_e2e_keys( $remote2 )
matrix_put_e2e_keys( $remote_leaver, device_keys => { x => "y" } );
})->then( sub {
sync_until_user_in_device_list( $creator, $remote_leaver );

})->then( sub {
matrix_set_device_display_name( $remote_leaver, $remote_leaver->device_id, "test display name" ),
})->then( sub {
log_if_fail "Remote_leaver " . $remote_leaver->user_id . " set display name";
sync_until_user_in_device_list( $creator, $remote_leaver );
# sanity check: make sure that we've got the right update
do_request_json_for( $creator,
method => "POST",
uri => "/r0/keys/query",

content => {
device_keys => { $remote_leaver->user_id => [ $remote_leaver->device_id ] },
},
)->then( sub {
my ( $body ) = @_;

log_if_fail "keys after remote_leaver uploaded keys", $body;
assert_json_keys( $body, qw( device_keys ));
my $update = $body->{device_keys}->{ $remote_leaver->user_id }->{ $remote_leaver->device_id };
assert_eq( $update->{x}, "y" );
Future->done;
});
})->then( sub {
matrix_leave_room_synced( $remote_leaver, $room_id )

# now one of the remote users leaves the room...
matrix_leave_room_synced( $remote_leaver, $room_id );
})->then( sub {
log_if_fail "Remote_leaver " . $remote_leaver->user_id . " left room";

# now /finally/ we can test what we came here for. Both remote users update their
# device keys, and we check that we only get an update for one of them.
matrix_put_e2e_keys( $remote_leaver, device_keys => { updated => "keys" } )
})->then( sub {
log_if_fail "Remote_leaver " . $remote_leaver->user_id . " updated keys";
matrix_put_e2e_keys( $remote2, device_keys => { updated => "keys" } )
})->then( sub {
log_if_fail "Remote user 2 " . $remote2->user_id . " updated keys";
sync_until_user_in_device_list( $creator, $remote2 );
})->then( sub {
my ( $body ) = @_;

log_if_fail "Final body", $body;
# we wait for a sync in which remote2 appears in the changed list, and make
# sure that remote_leaver *doesn't* appear in the meantime.

any { $_ eq $remote_leaver->user_id } @{ $body->{device_lists}{changed} }
and die "user2 in changed list after leaving";
my $wait_for_id = $remote2->user_id;
repeat_until_true {
matrix_sync_again( $creator, timeout => 1000 )
->then( sub {
my ( $body ) = @_;

Future->done(1);
log_if_fail "waiting for $wait_for_id in 'changed'", $body;

return Future->done(0) unless
$body->{device_lists} &&
$body->{device_lists}{changed};

my @changed_list = @{ $body->{device_lists}{changed} };
any { $_ eq $remote_leaver->user_id } @changed_list
and die "remote_leaver " . $remote_leaver->user_id . " in changed list after leaving";

return Future->done(
any { $_ eq $wait_for_id } @changed_list
);
});
};
});
};

Expand Down

0 comments on commit feef3e1

Please sign in to comment.