From 481a890ec58bee5b412ed8221822241a21c308ed Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Bodas Date: Thu, 27 May 2021 19:23:22 +0530 Subject: [PATCH] tests: Assert num_events in tornado_redirected_to_list. --- zerver/lib/test_classes.py | 6 ++- zerver/tests/test_bots.py | 12 +++--- zerver/tests/test_message_flags.py | 2 - zerver/tests/test_reactions.py | 11 ----- zerver/tests/test_subs.py | 64 +++++++++++------------------- zerver/tests/test_users.py | 15 +++---- 6 files changed, 40 insertions(+), 70 deletions(-) diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index 0bb46b1129..b5df226a71 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -1258,7 +1258,9 @@ Output: self.assertFalse(validation_func(guest_user)) @contextmanager - def tornado_redirected_to_list(self, lst: List[Mapping[str, Any]]) -> Iterator[None]: + def tornado_redirected_to_list( + self, lst: List[Mapping[str, Any]], expected_num_events: int = 1 + ) -> Iterator[None]: real_event_queue_process_notification = django_tornado_api.process_notification django_tornado_api.process_notification = lambda notice: lst.append(notice) # process_notification takes a single parameter called 'notice'. @@ -1269,6 +1271,8 @@ Output: yield django_tornado_api.process_notification = real_event_queue_process_notification + self.assert_length(lst, expected_num_events) + class WebhookTestCase(ZulipTestCase): """ diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py index e270f3eee7..5341e2ff5d 100644 --- a/zerver/tests/test_bots.py +++ b/zerver/tests/test_bots.py @@ -166,7 +166,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): self.login("hamlet") self.assert_num_bots_equal(0) events: List[Mapping[str, Any]] = [] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=2): result = self.create_bot() self.assert_num_bots_equal(1) @@ -332,7 +332,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): self.login_user(user) self.assert_num_bots_equal(0) events: List[Mapping[str, Any]] = [] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=2): result = self.create_bot() self.assert_num_bots_equal(1) @@ -386,7 +386,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): "principals": '["' + iago.email + '"]', } events: List[Mapping[str, Any]] = [] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=3): result = self.common_subscribe_to_streams(hamlet, ["Rome"], request_data) self.assert_json_success(result) @@ -403,7 +403,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): "principals": '["hambot-bot@zulip.testserver"]', } events_bot: List[Mapping[str, Any]] = [] - with self.tornado_redirected_to_list(events_bot): + with self.tornado_redirected_to_list(events_bot, expected_num_events=2): result = self.common_subscribe_to_streams(hamlet, ["Rome"], bot_request_data) self.assert_json_success(result) @@ -424,7 +424,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): self.assert_num_bots_equal(0) events: List[Mapping[str, Any]] = [] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=2): result = self.create_bot(default_sending_stream="Denmark") self.assert_num_bots_equal(1) self.assertEqual(result["default_sending_stream"], "Denmark") @@ -496,7 +496,7 @@ class BotTest(ZulipTestCase, UploadSerializeMixin): self.assert_num_bots_equal(0) events: List[Mapping[str, Any]] = [] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=2): result = self.create_bot(default_events_register_stream="Denmark") self.assert_num_bots_equal(1) self.assertEqual(result["default_events_register_stream"], "Denmark") diff --git a/zerver/tests/test_message_flags.py b/zerver/tests/test_message_flags.py index 02d17a9393..cf56e283bf 100644 --- a/zerver/tests/test_message_flags.py +++ b/zerver/tests/test_message_flags.py @@ -234,7 +234,6 @@ class UnreadCountTests(ZulipTestCase): ) self.assert_json_success(result) - self.assertTrue(len(events) == 1) event = events[0]["event"] expected = dict( @@ -306,7 +305,6 @@ class UnreadCountTests(ZulipTestCase): ) self.assert_json_success(result) - self.assertTrue(len(events) == 1) event = events[0]["event"] expected = dict( diff --git a/zerver/tests/test_reactions.py b/zerver/tests/test_reactions.py index f31e17f1fd..b997e362a3 100644 --- a/zerver/tests/test_reactions.py +++ b/zerver/tests/test_reactions.py @@ -423,7 +423,6 @@ class ReactionEventTest(ZulipTestCase): reaction_sender, f"/api/v1/messages/{pm_id}/reactions", reaction_info ) self.assert_json_success(result) - self.assert_length(events, 1) event = events[0]["event"] event_user_ids = set(events[0]["users"]) @@ -468,7 +467,6 @@ class ReactionEventTest(ZulipTestCase): reaction_sender, f"/api/v1/messages/{pm_id}/reactions", reaction_info ) self.assert_json_success(result) - self.assert_length(events, 1) event = events[0]["event"] event_user_ids = set(events[0]["users"]) @@ -507,7 +505,6 @@ class ReactionEventTest(ZulipTestCase): iago, f"/api/v1/messages/{message_before_id}/reactions", reaction_info ) self.assert_json_success(result) - self.assert_length(events, 1) event = events[0]["event"] self.assertEqual(event["type"], "reaction") event_user_ids = set(events[0]["users"]) @@ -528,7 +525,6 @@ class ReactionEventTest(ZulipTestCase): iago, f"/api/v1/messages/{message_after_id}/reactions", reaction_info ) self.assert_json_success(result) - self.assert_length(events, 1) event = events[0]["event"] self.assertEqual(event["type"], "reaction") event_user_ids = set(events[0]["users"]) @@ -549,7 +545,6 @@ class ReactionEventTest(ZulipTestCase): iago, f"/api/v1/messages/{message_before_id}/reactions", reaction_info ) self.assert_json_success(result) - self.assert_length(events, 1) event = events[0]["event"] self.assertEqual(event["type"], "reaction") event_user_ids = set(events[0]["users"]) @@ -569,7 +564,6 @@ class ReactionEventTest(ZulipTestCase): iago, f"/api/v1/messages/{message_before_id}/reactions", reaction_info ) self.assert_json_success(result) - self.assert_length(events, 1) event = events[0]["event"] self.assertEqual(event["type"], "reaction") event_user_ids = set(events[0]["users"]) @@ -591,7 +585,6 @@ class ReactionEventTest(ZulipTestCase): hamlet, f"/api/v1/messages/{private_message_id}/reactions", reaction_info ) self.assert_json_success(result) - self.assert_length(events, 1) event = events[0]["event"] self.assertEqual(event["type"], "reaction") event_user_ids = set(events[0]["users"]) @@ -609,7 +602,6 @@ class ReactionEventTest(ZulipTestCase): polonius, f"/api/v1/messages/{huddle_message_id}/reactions", reaction_info ) self.assert_json_success(result) - self.assert_length(events, 1) event = events[0]["event"] self.assertEqual(event["type"], "reaction") event_user_ids = set(events[0]["users"]) @@ -1038,8 +1030,6 @@ class ReactionAPIEventTest(EmojiReactionBase): with self.tornado_redirected_to_list(events): self.api_post(reaction_sender, f"/api/v1/messages/{pm_id}/reactions", reaction_info) - self.assert_length(events, 1) - event = events[0]["event"] event_user_ids = set(events[0]["users"]) @@ -1085,7 +1075,6 @@ class ReactionAPIEventTest(EmojiReactionBase): ) self.assert_json_success(result) - self.assert_length(events, 1) event = events[0]["event"] event_user_ids = set(events[0]["users"]) diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 50cac47ec2..608e284cbd 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -124,7 +124,6 @@ class TestCreateStreams(ZulipTestCase): events: List[Mapping[str, Any]] = [] with self.tornado_redirected_to_list(events): ensure_stream(realm, "Public stream", invite_only=False, acting_user=None) - self.assert_length(events, 1) self.assertEqual(events[0]["event"]["type"], "stream") self.assertEqual(events[0]["event"]["op"], "create") @@ -135,7 +134,6 @@ class TestCreateStreams(ZulipTestCase): events = [] with self.tornado_redirected_to_list(events): ensure_stream(realm, "Private stream", invite_only=True, acting_user=None) - self.assert_length(events, 1) self.assertEqual(events[0]["event"]["type"], "stream") self.assertEqual(events[0]["event"]["op"], "create") @@ -717,8 +715,6 @@ class StreamAdminTest(ZulipTestCase): {"description": "Test description"}, ) self.assert_json_success(result) - # Should be just a description change event - self.assert_length(events, 1) cordelia = self.example_user("cordelia") prospero = self.example_user("prospero") @@ -729,12 +725,11 @@ class StreamAdminTest(ZulipTestCase): self.assertNotIn(prospero.id, notified_user_ids) events = [] - with self.tornado_redirected_to_list(events): + # Three events should be sent: a name event, an email address event and a notification event + with self.tornado_redirected_to_list(events, expected_num_events=3): stream_id = get_stream("private_stream", user_profile.realm).id result = self.client_patch(f"/json/streams/{stream_id}", {"new_name": "whatever"}) self.assert_json_success(result) - # Should be a name event, an email address event and a notification event - self.assert_length(events, 3) notified_user_ids = set(events[0]["users"]) self.assertIn(user_profile.id, notified_user_ids) @@ -768,7 +763,7 @@ class StreamAdminTest(ZulipTestCase): self.assert_json_success(result) events: List[Mapping[str, Any]] = [] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=3): stream_id = get_stream("stream_name1", user_profile.realm).id result = self.client_patch(f"/json/streams/{stream_id}", {"new_name": "stream_name2"}) self.assert_json_success(result) @@ -798,7 +793,7 @@ class StreamAdminTest(ZulipTestCase): # Test case to handle Unicode stream name change # *NOTE: Here encoding is needed when Unicode string is passed as an argument* - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=6): stream_id = stream_name2_exists.id result = self.client_patch(f"/json/streams/{stream_id}", {"new_name": "नया नाम"}) self.assert_json_success(result) @@ -809,7 +804,7 @@ class StreamAdminTest(ZulipTestCase): # Test case to handle changing of Unicode stream name to newer name # NOTE: Unicode string being part of URL is handled cleanly # by client_patch call, encoding of URL is not needed. - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=9): stream_id = stream_name_uni_exists.id result = self.client_patch( f"/json/streams/{stream_id}", @@ -823,7 +818,7 @@ class StreamAdminTest(ZulipTestCase): self.assertTrue(stream_name_new_uni_exists) # Test case to change name from one language to other. - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=12): stream_id = stream_name_new_uni_exists.id result = self.client_patch(f"/json/streams/{stream_id}", {"new_name": "français"}) self.assert_json_success(result) @@ -831,7 +826,7 @@ class StreamAdminTest(ZulipTestCase): self.assertTrue(stream_name_fr_exists) # Test case to change name to mixed language name. - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=15): stream_id = stream_name_fr_exists.id result = self.client_patch(f"/json/streams/{stream_id}", {"new_name": "français name"}) self.assert_json_success(result) @@ -844,7 +839,7 @@ class StreamAdminTest(ZulipTestCase): ) self.subscribe(self.example_user("cordelia"), "stream_private_name1") del events[:] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=3): stream_id = get_stream("stream_private_name1", realm).id result = self.client_patch( f"/json/streams/{stream_id}", @@ -874,13 +869,12 @@ class StreamAdminTest(ZulipTestCase): acting_user=None, ) del events[:] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=3): result = self.client_patch( f"/json/streams/{new_stream.id}", {"new_name": "stream_rename"}, ) self.assert_json_success(result) - self.assert_length(events, 3) stream_rename_exists = get_stream("stream_rename", realm) self.assertTrue(stream_rename_exists) @@ -1065,7 +1059,7 @@ class StreamAdminTest(ZulipTestCase): acting_user=None, ) - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=2): stream_id = get_stream("stream_name1", realm).id result = self.client_patch( f"/json/streams/{stream_id}", @@ -2555,7 +2549,6 @@ class SubscriptionPropertiesTest(ZulipTestCase): }, ) self.assert_json_success(result) - self.assert_length(events, 1) self.assertEqual(events[0]["event"]["property"], "in_home_view") self.assertEqual(events[0]["event"]["value"], False) sub = Subscription.objects.get( @@ -2584,7 +2577,6 @@ class SubscriptionPropertiesTest(ZulipTestCase): }, ) self.assert_json_success(result) - self.assert_length(events, 1) self.assertEqual(events[0]["event"]["property"], "in_home_view") self.assertEqual(events[0]["event"]["value"], True) self.assert_json_success(result) @@ -2613,7 +2605,6 @@ class SubscriptionPropertiesTest(ZulipTestCase): }, ) self.assert_json_success(result) - self.assert_length(events, 1) self.assertEqual(events[0]["event"]["property"], "in_home_view") self.assertEqual(events[0]["event"]["value"], False) @@ -3055,7 +3046,7 @@ class SubscriptionAPITest(ZulipTestCase): add_streams = ["Verona2", "Denmark5"] self.assertNotEqual(len(add_streams), 0) # necessary for full test coverage events: List[Mapping[str, Any]] = [] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=6): self.helper_check_subs_before_and_after_add( self.streams + add_streams, {}, @@ -3065,7 +3056,6 @@ class SubscriptionAPITest(ZulipTestCase): self.streams + add_streams, self.test_realm, ) - self.assert_length(events, 6) def test_successful_subscriptions_add_with_announce(self) -> None: """ @@ -3086,7 +3076,7 @@ class SubscriptionAPITest(ZulipTestCase): self.test_realm.notifications_stream_id = notifications_stream.id self.test_realm.save() - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=7): self.helper_check_subs_before_and_after_add( self.streams + add_streams, other_params, @@ -3096,7 +3086,6 @@ class SubscriptionAPITest(ZulipTestCase): self.streams + add_streams, self.test_realm, ) - self.assert_length(events, 7) expected_stream_ids = {get_stream(stream, self.test_realm).id for stream in add_streams} @@ -3505,7 +3494,7 @@ class SubscriptionAPITest(ZulipTestCase): streams_to_sub = ["multi_user_stream"] events: List[Mapping[str, Any]] = [] flush_per_request_caches() - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=5): with queries_captured() as queries: self.common_subscribe_to_streams( self.test_user, @@ -3514,7 +3503,6 @@ class SubscriptionAPITest(ZulipTestCase): ) self.assert_length(queries, 35) - self.assert_length(events, 5) for ev in [x for x in events if x["event"]["type"] not in ("message", "stream")]: if ev["event"]["op"] == "add": self.assertEqual( @@ -3532,7 +3520,7 @@ class SubscriptionAPITest(ZulipTestCase): # Now add ourselves events = [] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=2): with queries_captured() as queries: self.common_subscribe_to_streams( self.test_user, @@ -3541,7 +3529,6 @@ class SubscriptionAPITest(ZulipTestCase): ) self.assert_length(queries, 11) - self.assert_length(events, 2) add_event, add_peer_event = events self.assertEqual(add_event["event"]["type"], "subscription") self.assertEqual(add_event["event"]["op"], "add") @@ -3567,10 +3554,9 @@ class SubscriptionAPITest(ZulipTestCase): user3 = user_profile realm3 = user_profile.realm stream = get_stream("multi_user_stream", realm) - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=2): bulk_add_subscriptions(realm, [stream], [user_profile], acting_user=None) - self.assert_length(events, 2) add_event, add_peer_event = events self.assertEqual(add_event["event"]["type"], "subscription") @@ -3603,10 +3589,9 @@ class SubscriptionAPITest(ZulipTestCase): user_profile = self.example_user("cordelia") events: List[Mapping[str, Any]] = [] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=3): bulk_add_subscriptions(realm, [stream], [user_profile], acting_user=None) - self.assert_length(events, 3) create_event, add_event, add_peer_event = events self.assertEqual(create_event["event"]["type"], "stream") @@ -3635,7 +3620,7 @@ class SubscriptionAPITest(ZulipTestCase): # private stream creation event on stream creation. new_stream = ensure_stream(realm, "private stream", invite_only=True, acting_user=None) events = [] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=2): bulk_add_subscriptions( realm, [new_stream], [self.example_user("iago")], acting_user=None ) @@ -3767,7 +3752,7 @@ class SubscriptionAPITest(ZulipTestCase): new_user_ids_to_subscribe = [iago.id, cordelia.id] events: List[Mapping[str, Any]] = [] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=5): self.common_subscribe_to_streams( self.test_user, streams_to_sub, @@ -3816,7 +3801,7 @@ class SubscriptionAPITest(ZulipTestCase): self.subscribe(user3, "private_stream") events: List[Mapping[str, Any]] = [] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=13): with queries_captured() as query_count: with cache_tries_captured() as cache_count: bulk_remove_subscriptions( @@ -3872,8 +3857,10 @@ class SubscriptionAPITest(ZulipTestCase): stream.is_in_zephyr_realm = True stream.save() + # Make sure Zephyr mirroring realms such as MIT do not get + # any tornado subscription events events: List[Mapping[str, Any]] = [] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=0): with queries_captured() as queries: self.common_subscribe_to_streams( mit_user, @@ -3882,13 +3869,10 @@ class SubscriptionAPITest(ZulipTestCase): subdomain="zephyr", allow_fail=True, ) - # Make sure Zephyr mirroring realms such as MIT do not get - # any tornado subscription events - self.assert_length(events, 0) self.assert_length(queries, 4) events = [] - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=0): bulk_remove_subscriptions( users=[mit_user], streams=streams, @@ -3896,8 +3880,6 @@ class SubscriptionAPITest(ZulipTestCase): acting_user=None, ) - self.assert_length(events, 0) - def test_bulk_subscribe_many(self) -> None: # Create a whole bunch of streams diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index 023dff0fd9..81ae02d489 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -216,7 +216,7 @@ class PermissionTest(ZulipTestCase): person = events[0]["event"]["person"] self.assertEqual(person["user_id"], iago.id) self.assertEqual(person["role"], UserProfile.ROLE_MEMBER) - with self.tornado_redirected_to_list([]): + with self.tornado_redirected_to_list([], expected_num_events=0): result = self.client_patch(f"/json/users/{desdemona.id}", req) self.assert_json_error( result, "The owner permission cannot be removed from the only organization owner." @@ -224,7 +224,7 @@ class PermissionTest(ZulipTestCase): do_change_user_role(iago, UserProfile.ROLE_REALM_ADMINISTRATOR, acting_user=None) self.login("iago") - with self.tornado_redirected_to_list([]): + with self.tornado_redirected_to_list([], expected_num_events=0): result = self.client_patch(f"/json/users/{desdemona.id}", req) self.assert_json_error(result, "Must be an organization owner") @@ -763,7 +763,7 @@ class QueryCountTest(ZulipTestCase): with queries_captured() as queries: with cache_tries_captured() as cache_tries: - with self.tornado_redirected_to_list(events): + with self.tornado_redirected_to_list(events, expected_num_events=7): fred = do_create_user( email="fred@zulip.com", password="password", @@ -775,7 +775,6 @@ class QueryCountTest(ZulipTestCase): self.assert_length(queries, 70) self.assert_length(cache_tries, 22) - self.assert_length(events, 7) peer_add_events = [event for event in events if event["event"].get("op") == "peer_add"] @@ -1164,15 +1163,13 @@ class UserProfileTest(ZulipTestCase): for hotspot in hotspots_completed: UserHotspot.objects.create(user=cordelia, hotspot=hotspot) - events: List[Mapping[str, Any]] = [] - with self.tornado_redirected_to_list(events): - copy_user_settings(cordelia, iago) - # Check that we didn't send an realm_user update events to # users; this work is happening before the user account is # created, so any changes will be reflected in the "add" event # introducing the user to clients. - self.assert_length(events, 0) + events: List[Mapping[str, Any]] = [] + with self.tornado_redirected_to_list(events, expected_num_events=0): + copy_user_settings(cordelia, iago) # We verify that cordelia and iago match, but hamlet has the defaults. self.assertEqual(iago.full_name, "Cordelia, Lear's daughter")