From 751b8b5bb5bbbae3eb387918abf7abcbd95fc7cc Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Fri, 11 Aug 2023 14:40:06 +0000 Subject: [PATCH] tests: Flush per-request caches automatically for query counts. --- analytics/tests/test_activity_views.py | 5 +---- zerver/lib/test_helpers.py | 2 ++ zerver/tests/test_digest.py | 5 ----- zerver/tests/test_event_system.py | 3 --- zerver/tests/test_home.py | 5 ----- zerver/tests/test_message_dict.py | 1 - zerver/tests/test_message_edit.py | 21 +++++++++++---------- zerver/tests/test_signup.py | 4 +--- zerver/tests/test_subs.py | 2 -- 9 files changed, 15 insertions(+), 33 deletions(-) diff --git a/analytics/tests/test_activity_views.py b/analytics/tests/test_activity_views.py index 2a8261627d..d6e42c4d68 100644 --- a/analytics/tests/test_activity_views.py +++ b/analytics/tests/test_activity_views.py @@ -3,7 +3,7 @@ from unittest import mock from django.utils.timezone import now as timezone_now from zerver.lib.test_classes import ZulipTestCase -from zerver.models import Client, UserActivity, UserProfile, flush_per_request_caches +from zerver.models import Client, UserActivity, UserProfile class ActivityTest(ZulipTestCase): @@ -31,18 +31,15 @@ class ActivityTest(ZulipTestCase): user_profile.is_staff = True user_profile.save(update_fields=["is_staff"]) - flush_per_request_caches() with self.assert_database_query_count(18): result = self.client_get("/activity") self.assertEqual(result.status_code, 200) - flush_per_request_caches() with self.assert_database_query_count(8): result = self.client_get("/realm_activity/zulip/") self.assertEqual(result.status_code, 200) iago = self.example_user("iago") - flush_per_request_caches() with self.assert_database_query_count(5): result = self.client_get(f"/user_activity/{iago.id}/") self.assertEqual(result.status_code, 200) diff --git a/zerver/lib/test_helpers.py b/zerver/lib/test_helpers.py index d4216422e1..16029ca68e 100644 --- a/zerver/lib/test_helpers.py +++ b/zerver/lib/test_helpers.py @@ -56,6 +56,7 @@ from zerver.models import ( Subscription, UserMessage, UserProfile, + flush_per_request_caches, get_client, get_realm, get_stream, @@ -181,6 +182,7 @@ def queries_captured( if not keep_cache_warm: cache = get_cache_backend(None) cache.clear() + flush_per_request_caches() with mock.patch.multiple( TimeTrackingCursor, execute=cursor_execute, executemany=cursor_executemany ): diff --git a/zerver/tests/test_digest.py b/zerver/tests/test_digest.py index c3d9a94887..d195a1eaf9 100644 --- a/zerver/tests/test_digest.py +++ b/zerver/tests/test_digest.py @@ -32,7 +32,6 @@ from zerver.models import ( Stream, UserActivityInterval, UserProfile, - flush_per_request_caches, get_client, get_realm, get_stream, @@ -60,7 +59,6 @@ class TestDigestEmailMessages(ZulipTestCase): # Remove RealmAuditLog rows, so we don't exclude polonius. RealmAuditLog.objects.all().delete() - flush_per_request_caches() # When this test is run in isolation, one additional query is run which # is equivalent to # ContentType.objects.get(app_label='zerver', model='userprofile') @@ -137,7 +135,6 @@ class TestDigestEmailMessages(ZulipTestCase): # Remove RealmAuditoLog rows, so we don't exclude polonius. RealmAuditLog.objects.all().delete() - flush_per_request_caches() # When this test is run in isolation, one additional query is run which # is equivalent to # ContentType.objects.get(app_label='zerver', model='userprofile') @@ -195,8 +192,6 @@ class TestDigestEmailMessages(ZulipTestCase): one_hour_ago = timezone_now() - datetime.timedelta(seconds=3600) cutoff = time.mktime(one_hour_ago.timetuple()) - flush_per_request_caches() - # When this test is run in isolation, one additional query is run which # is equivalent to # ContentType.objects.get(app_label='zerver', model='userprofile') diff --git a/zerver/tests/test_event_system.py b/zerver/tests/test_event_system.py index b328a41ced..1e38591636 100644 --- a/zerver/tests/test_event_system.py +++ b/zerver/tests/test_event_system.py @@ -32,7 +32,6 @@ from zerver.models import ( UserMessage, UserPresence, UserProfile, - flush_per_request_caches, get_client, get_realm, get_stream, @@ -1248,7 +1247,6 @@ class FetchQueriesTest(ZulipTestCase): self.login_user(user) - flush_per_request_caches() with self.assert_database_query_count(39): with mock.patch("zerver.lib.events.always_want") as want_mock: fetch_initial_state_data(user) @@ -1298,7 +1296,6 @@ class FetchQueriesTest(ZulipTestCase): for event_type in sorted(wanted_event_types): count = expected_counts[event_type] - flush_per_request_caches() with self.assert_database_query_count(count): if event_type == "update_message_flags": event_types = ["update_message_flags", "message"] diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index b32ba69f3d..98dcea61d7 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -29,7 +29,6 @@ from zerver.models import ( Realm, UserActivity, UserProfile, - flush_per_request_caches, get_realm, get_stream, get_system_bot, @@ -246,7 +245,6 @@ class HomeTest(ZulipTestCase): self.client_post("/json/bots", bot_info) # Verify succeeds once logged-in - flush_per_request_caches() with self.assert_database_query_count(49): with patch("zerver.lib.cache.cache_set") as cache_mock: result = self._get_home_page(stream="Denmark") @@ -302,7 +300,6 @@ class HomeTest(ZulipTestCase): self.login("hamlet") # Verify succeeds once logged-in - flush_per_request_caches() with queries_captured(): with patch("zerver.lib.cache.cache_set"): result = self._get_home_page(stream="Denmark") @@ -436,7 +433,6 @@ class HomeTest(ZulipTestCase): def test_num_queries_for_realm_admin(self) -> None: # Verify number of queries for Realm admin isn't much higher than for normal users. self.login("iago") - flush_per_request_caches() with self.assert_database_query_count(50): with patch("zerver.lib.cache.cache_set") as cache_mock: result = self._get_home_page() @@ -468,7 +464,6 @@ class HomeTest(ZulipTestCase): self._get_home_page() # Then for the second page load, measure the number of queries. - flush_per_request_caches() with self.assert_database_query_count(44): result = self._get_home_page() diff --git a/zerver/tests/test_message_dict.py b/zerver/tests/test_message_dict.py index ad816c0889..3ffe3554ea 100644 --- a/zerver/tests/test_message_dict.py +++ b/zerver/tests/test_message_dict.py @@ -176,7 +176,6 @@ class MessageDictTest(ZulipTestCase): num_ids = len(ids) self.assertTrue(num_ids >= 600) - flush_per_request_caches() with self.assert_database_query_count(7): rows = list(MessageDict.get_raw_db_rows(ids)) diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py index 4c26e93258..9d3d1e1498 100644 --- a/zerver/tests/test_message_edit.py +++ b/zerver/tests/test_message_edit.py @@ -289,13 +289,14 @@ class EditMessageTest(EditMessageTestCase): # Check number of queries performed # 1 query for realm_id per message = 3 # 1 query each for reactions & submessage for all messages = 2 - with self.assert_database_query_count(5): + # 1 query for linkifiers + # 1 query for display recipients + with self.assert_database_query_count(7): MessageDict.to_dict_uncached(messages) realm_id = 2 # Fetched from stream object # Check number of queries performed with realm_id - # 1 query each for reactions & submessage for all messages = 2 - with self.assert_database_query_count(2): + with self.assert_database_query_count(3): MessageDict.to_dict_uncached(messages, realm_id) def test_save_message(self) -> None: @@ -1374,7 +1375,7 @@ class EditMessageTest(EditMessageTestCase): # state + 1/user with a UserTopic row for the events data) # beyond what is typical were there not UserTopic records to # update. Ideally, we'd eliminate the per-user component. - with self.assert_database_query_count(21): + with self.assert_database_query_count(22): check_update_message( user_profile=hamlet, message_id=message_id, @@ -1471,7 +1472,7 @@ class EditMessageTest(EditMessageTestCase): set_topic_visibility_policy(desdemona, muted_topics, UserTopic.VisibilityPolicy.MUTED) set_topic_visibility_policy(cordelia, muted_topics, UserTopic.VisibilityPolicy.MUTED) - with self.assert_database_query_count(30): + with self.assert_database_query_count(31): check_update_message( user_profile=desdemona, message_id=message_id, @@ -1502,7 +1503,7 @@ class EditMessageTest(EditMessageTestCase): set_topic_visibility_policy(desdemona, muted_topics, UserTopic.VisibilityPolicy.MUTED) set_topic_visibility_policy(cordelia, muted_topics, UserTopic.VisibilityPolicy.MUTED) - with self.assert_database_query_count(35): + with self.assert_database_query_count(36): check_update_message( user_profile=desdemona, message_id=message_id, @@ -1535,7 +1536,7 @@ class EditMessageTest(EditMessageTestCase): set_topic_visibility_policy(desdemona, muted_topics, UserTopic.VisibilityPolicy.MUTED) set_topic_visibility_policy(cordelia, muted_topics, UserTopic.VisibilityPolicy.MUTED) - with self.assert_database_query_count(30): + with self.assert_database_query_count(31): check_update_message( user_profile=desdemona, message_id=message_id, @@ -1558,7 +1559,7 @@ class EditMessageTest(EditMessageTestCase): second_message_id = self.send_stream_message( hamlet, stream_name, topic_name="changed topic name", content="Second message" ) - with self.assert_database_query_count(26): + with self.assert_database_query_count(27): check_update_message( user_profile=desdemona, message_id=second_message_id, @@ -1657,7 +1658,7 @@ class EditMessageTest(EditMessageTestCase): users_to_be_notified_via_muted_topics_event.append(user_topic.user_profile_id) change_all_topic_name = "Topic 1 edited" - with self.assert_database_query_count(26): + with self.assert_database_query_count(27): check_update_message( user_profile=hamlet, message_id=message_id, @@ -3691,7 +3692,7 @@ class EditMessageTest(EditMessageTestCase): "iago", "test move stream", "new stream", "test" ) - with self.assert_database_query_count(56), self.assert_memcached_count(13): + with self.assert_database_query_count(57), self.assert_memcached_count(14): result = self.client_patch( f"/json/messages/{msg_id}", { diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index f057ff5fe9..f1d5a84329 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -77,7 +77,6 @@ from zerver.models import ( Subscription, UserMessage, UserProfile, - flush_per_request_caches, get_realm, get_stream, get_system_bot, @@ -923,8 +922,7 @@ class LoginTest(ZulipTestCase): content="test message", ) - # Clear all the caches. - flush_per_request_caches() + # Clear the ContentType cache. ContentType.objects.clear_cache() # Ensure the number of queries we make is not O(streams) diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 8a5cb9e6c3..3b24402f1b 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -102,7 +102,6 @@ from zerver.models import ( UserMessage, UserProfile, active_non_guest_user_ids, - flush_per_request_caches, get_default_stream_groups, get_realm, get_stream, @@ -4625,7 +4624,6 @@ class SubscriptionAPITest(ZulipTestCase): user2 = self.example_user("iago") realm = get_realm("zulip") streams_to_sub = ["multi_user_stream"] - flush_per_request_caches() with self.capture_send_event_calls(expected_num_events=5) as events: with self.assert_database_query_count(37): self.common_subscribe_to_streams(