From db4180724d7371c77b22f75c01bc43857e5fcffa Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sat, 25 Aug 2018 01:43:23 +0200 Subject: [PATCH 1/5] test to ensure spurious state doesn't leak into incr sync responses when LLing tests https://github.com/matrix-org/synapse/pull/3760 regression test for https://github.com/vector-im/riot-web/issues/7223 --- tests/10apidoc/09synced.pl | 30 +++++++++++++++++++++++++++++- tests/31sync/15lazy-members.pl | 14 +++++++++++--- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/tests/10apidoc/09synced.pl b/tests/10apidoc/09synced.pl index d20a85b37..a6099ad9a 100644 --- a/tests/10apidoc/09synced.pl +++ b/tests/10apidoc/09synced.pl @@ -205,7 +205,35 @@ sub await_sync_presence_contains { } -push @EXPORT, qw( assert_room_members assert_state_room_members_matches ); +push @EXPORT, qw( assert_room_members assert_state_room_members_matches assert_room_state); + +# assert that the state body of a sync response is made up of the given state types. +sub assert_room_state { + my ( $body, $room_id, $state_types ) = @_; + + my $room = $body->{rooms}{join}{$room_id}; + my $state = $room->{state}{events}; + + log_if_fail "Room", $room; + + my $found_types = []; + foreach (@$state) { + push @$found_types, [ $_->{type}, $_->{state_key} ]; + } + + my $comp = sub { + my ($a, $b) = @_; + return ($a->[0] cmp $b->[0]) || ($a->[1] cmp $b->[1]); + }; + + $found_types = [ sort { &$comp($a, $b) } @$found_types ]; + $state_types = [ sort { &$comp($a, $b) } @$state_types ]; + + log_if_fail "Found state types", $found_types; + log_if_fail "Desired state types", $state_types; + + assert_deeply_eq($found_types, $state_types); +} # assert that the given members are in the body of a sync response sub assert_room_members { diff --git a/tests/31sync/15lazy-members.pl b/tests/31sync/15lazy-members.pl index 9e1ffebbd..e0282e767 100644 --- a/tests/31sync/15lazy-members.pl +++ b/tests/31sync/15lazy-members.pl @@ -161,7 +161,14 @@ matrix_sync( $alice, filter => $filter_id ); })->then( sub { my ( $body ) = @_; - assert_room_members( $body, $room_id, [ $bob->user_id ]); + assert_room_state( $body, $room_id, [ + [ 'm.room.create', '' ], + [ 'm.room.join_rules', '' ], + [ 'm.room.power_levels', '' ], + [ 'm.room.name', '' ], + [ 'm.room.history_visibility', '' ], + [ 'm.room.member', $bob->user_id ], + ]); matrix_send_room_text_message_synced( $charlie, $room_id, body => "Message from charlie", @@ -170,7 +177,9 @@ matrix_sync_again( $alice, filter => $filter_id ); })->then( sub { my ( $body ) = @_; - assert_room_members( $body, $room_id, [ $charlie->user_id ]); + assert_room_state( $body, $room_id, [ + [ 'm.room.member', $charlie->user_id ], + ]); Future->done(1); }); }; @@ -258,7 +267,6 @@ }); }; - test "We don't send redundant membership state across incremental syncs by default", requires => [ local_user_fixtures( 3 ), qw( can_sync ) ], From 5b9bb710614a85269325eaa6d39183975efb1c4c Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sat, 25 Aug 2018 02:02:36 +0200 Subject: [PATCH 2/5] make comparator prettier --- tests/10apidoc/09synced.pl | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/10apidoc/09synced.pl b/tests/10apidoc/09synced.pl index a6099ad9a..851566c95 100644 --- a/tests/10apidoc/09synced.pl +++ b/tests/10apidoc/09synced.pl @@ -222,12 +222,11 @@ sub assert_room_state { } my $comp = sub { - my ($a, $b) = @_; return ($a->[0] cmp $b->[0]) || ($a->[1] cmp $b->[1]); }; - $found_types = [ sort { &$comp($a, $b) } @$found_types ]; - $state_types = [ sort { &$comp($a, $b) } @$state_types ]; + $found_types = [ sort $comp @$found_types ]; + $state_types = [ sort $comp @$state_types ]; log_if_fail "Found state types", $found_types; log_if_fail "Desired state types", $state_types; From 1f0ddd043e6650636261b85d6d5b62b7f6e7ee76 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 28 Aug 2018 23:21:55 +0100 Subject: [PATCH 3/5] incorporate review feedback --- tests/10apidoc/09synced.pl | 47 +++++++++++++++++++++++++--------- tests/30rooms/50context.pl | 2 +- tests/30rooms/52members.pl | 10 ++++---- tests/31sync/15lazy-members.pl | 8 ++++-- 4 files changed, 47 insertions(+), 20 deletions(-) diff --git a/tests/10apidoc/09synced.pl b/tests/10apidoc/09synced.pl index 851566c95..55257afd2 100644 --- a/tests/10apidoc/09synced.pl +++ b/tests/10apidoc/09synced.pl @@ -205,14 +205,20 @@ sub await_sync_presence_contains { } -push @EXPORT, qw( assert_room_members assert_state_room_members_matches assert_room_state); +=head2 assert_state_types_match -# assert that the state body of a sync response is made up of the given state types. -sub assert_room_state { - my ( $body, $room_id, $state_types ) = @_; +Assert that the state body of a sync response is made up of the given state types. - my $room = $body->{rooms}{join}{$room_id}; - my $state = $room->{state}{events}; +$state is an arrayref of state events. + +$state_types is an arrayref of arrayrefs, each a tuple of type & state_key. + +=cut + +push @EXPORT, qw( assert_state_types_match ); + +sub assert_state_types_match { + my ( $state, $room_id, $state_types ) = @_; log_if_fail "Room", $room; @@ -234,10 +240,19 @@ sub assert_room_state { assert_deeply_eq($found_types, $state_types); } -# assert that the given members are in the body of a sync response +=head2 assert_room_members + +Assert that the given members are in the body of a sync response + +$memberships is either an arrayref of user_ids or a hashref of user_id +to membership strings. + +=cut + +push @EXPORT, qw ( assert_room_members ); + sub assert_room_members { my ( $body, $room_id, $memberships ) = @_; - # Takes either an arrayref of user_ids or a hashref of user_id to membership strings my $room = $body->{rooms}{join}{$room_id}; my $timeline = $room->{timeline}{events}; @@ -246,14 +261,22 @@ sub assert_room_members { assert_json_keys( $room, qw( timeline state ephemeral )); - return assert_state_room_members_matches( $room->{state}{events}, $memberships ); + return assert_state_room_members_match( $room->{state}{events}, $memberships ); } +=head2 assert_state_room_members_match + +Assert that the given members are present in a block of state events + +$memberships is either an arrayref of user_ids or a hashref of user_id +to membership strings. + +=cut + +push @EXPORT, qw( assert_state_room_members_match ); -# assert that the given members are present in a block of state events -sub assert_state_room_members_matches { +sub assert_state_room_members_match { my ( $events, $memberships ) = @_; - # Takes either an arrayref of user_ids or a hashref of user_id to membership strings log_if_fail "expected members:", $memberships; log_if_fail "state:", $events; diff --git a/tests/30rooms/50context.pl b/tests/30rooms/50context.pl index 43b0aea8e..6a3df93dd 100644 --- a/tests/30rooms/50context.pl +++ b/tests/30rooms/50context.pl @@ -130,7 +130,7 @@ assert_json_keys( $body, qw( state event ) ); # only the user who sent 'hello world' should be present in the state - assert_state_room_members_matches( $body->{state}, [ $user->user_id ]); + assert_state_room_members_match( $body->{state}, [ $user->user_id ]); Future->done( 1 ) }); diff --git a/tests/30rooms/52members.pl b/tests/30rooms/52members.pl index a53a81c4a..0fbc8c843 100644 --- a/tests/30rooms/52members.pl +++ b/tests/30rooms/52members.pl @@ -18,7 +18,7 @@ assert_json_keys( $body, qw( chunk ) ); - assert_state_room_members_matches( $body->{chunk}, [ + assert_state_room_members_match( $body->{chunk}, [ $user1->user_id, $user2->user_id, ]); @@ -69,7 +69,7 @@ assert_json_keys( $body, qw( chunk ) ); # as of the first 'Hello world' the only member in the room should be user1 - assert_state_room_members_matches( $body->{chunk}, [ $user1->user_id ]); + assert_state_room_members_match( $body->{chunk}, [ $user1->user_id ]); Future->done(1); }) @@ -100,7 +100,7 @@ my ( $body ) = @_; assert_json_keys( $body, qw( chunk ) ); - assert_state_room_members_matches( $body->{chunk}, { $user1->user_id => 'join' } ); + assert_state_room_members_match( $body->{chunk}, { $user1->user_id => 'join' } ); do_request_json_for( $user1, method => "GET", @@ -113,7 +113,7 @@ my ( $body ) = @_; assert_json_keys( $body, qw( chunk ) ); - assert_state_room_members_matches( $body->{chunk}, { $user2->user_id => 'leave' }); + assert_state_room_members_match( $body->{chunk}, { $user2->user_id => 'leave' }); do_request_json_for( $user1, method => "GET", @@ -126,7 +126,7 @@ my ( $body ) = @_; assert_json_keys( $body, qw( chunk ) ); - assert_state_room_members_matches( $body->{chunk}, { $user1->user_id => 'join' }); + assert_state_room_members_match( $body->{chunk}, { $user1->user_id => 'join' }); Future->done(1); }) }; diff --git a/tests/31sync/15lazy-members.pl b/tests/31sync/15lazy-members.pl index e0282e767..1f8196c2e 100644 --- a/tests/31sync/15lazy-members.pl +++ b/tests/31sync/15lazy-members.pl @@ -161,7 +161,9 @@ matrix_sync( $alice, filter => $filter_id ); })->then( sub { my ( $body ) = @_; - assert_room_state( $body, $room_id, [ + my $state = $body->{rooms}{join}{$room_id}{state}{events}; + + assert_state_types_match( $state, $room_id, [ [ 'm.room.create', '' ], [ 'm.room.join_rules', '' ], [ 'm.room.power_levels', '' ], @@ -177,7 +179,9 @@ matrix_sync_again( $alice, filter => $filter_id ); })->then( sub { my ( $body ) = @_; - assert_room_state( $body, $room_id, [ + my $state = $body->{rooms}{join}{$room_id}{state}{events}; + + assert_state_types_match( $state, $room_id, [ [ 'm.room.member', $charlie->user_id ], ]); Future->done(1); From ef7e777ed45a3f04e294c3a1d15eb736557b3cd2 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 28 Aug 2018 23:49:19 +0100 Subject: [PATCH 4/5] oops --- tests/10apidoc/09synced.pl | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/10apidoc/09synced.pl b/tests/10apidoc/09synced.pl index 55257afd2..7e325b00a 100644 --- a/tests/10apidoc/09synced.pl +++ b/tests/10apidoc/09synced.pl @@ -220,8 +220,6 @@ =head2 assert_state_types_match sub assert_state_types_match { my ( $state, $room_id, $state_types ) = @_; - log_if_fail "Room", $room; - my $found_types = []; foreach (@$state) { push @$found_types, [ $_->{type}, $_->{state_key} ]; From 3b231026627abcee10de88bb6cb16c09ee90848f Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 18 Sep 2018 16:34:35 +0100 Subject: [PATCH 5/5] add comment --- tests/10apidoc/09synced.pl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/10apidoc/09synced.pl b/tests/10apidoc/09synced.pl index 7e325b00a..3df9fb6b3 100644 --- a/tests/10apidoc/09synced.pl +++ b/tests/10apidoc/09synced.pl @@ -211,7 +211,13 @@ =head2 assert_state_types_match $state is an arrayref of state events. -$state_types is an arrayref of arrayrefs, each a tuple of type & state_key. +$state_types is an arrayref of arrayrefs, each a tuple of type & state_key, e.g: + + [ + [ 'm.room.create', '' ], + [ 'm.room.name', '' ], + [ 'm.room.member', '@foo:bar.com' ], + ] =cut