From 92d0290dc5d6c9c6f64a13fe28b71584bbd2dcf4 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Tue, 30 Jun 2020 19:29:42 -0700 Subject: [PATCH] tests: Remove slow tests detection. According to @showell: > All the slow decorators can die. That was a failed experiment of > mine from 2014 days. I have meaning to kill them for a couple years > now. I wrote this with the best of intentions, but I believe it's > now just cruft. We never made a "fast" mode, for one. And we kept > writing more and more slow tests, haha. Signed-off-by: Anders Kaseorg --- tools/test-backend | 11 --- zerver/lib/test_runner.py | 69 ------------------- zerver/tests/test_docs.py | 4 -- zerver/tests/test_events.py | 8 --- zerver/tests/test_home.py | 4 -- zerver/tests/test_import_export.py | 2 - zerver/tests/test_management_commands.py | 2 - zerver/tests/test_markdown.py | 2 - zerver/tests/test_messages.py | 4 -- zerver/tests/test_realm.py | 2 - zerver/tests/test_slack_message_conversion.py | 2 - zerver/tests/test_subs.py | 4 -- zerver/tests/test_urls.py | 2 - 13 files changed, 116 deletions(-) diff --git a/tools/test-backend b/tools/test-backend index 004629c8e2..6ffc836144 100755 --- a/tools/test-backend +++ b/tools/test-backend @@ -241,10 +241,6 @@ def main() -> None: action="store_true", default=False, help="Show detailed output") - parser.add_argument('--report-slow-tests', dest='report_slow_tests', - action="store_true", - default=False, - help="Show which tests are slowest.") parser.add_argument('--reverse', dest='reverse', action="store_true", default=False, @@ -458,13 +454,6 @@ def main() -> None: print("1.) `vagrant ssh -- -L 8080:127.0.0.1:8080`") print(f"2.) `snakeviz -s {shlex.quote(stats_file.name)}`") - if options.report_slow_tests: - from zerver.lib.test_runner import report_slow_tests - - # We do this even with failures, since slowness can be - # an important clue as to why tests fail. - report_slow_tests() - # Ideally, we'd check for any leaked test databases here; # but that needs some hackery with database names. # diff --git a/zerver/lib/test_runner.py b/zerver/lib/test_runner.py index d33fd21848..073fba0797 100644 --- a/zerver/lib/test_runner.py +++ b/zerver/lib/test_runner.py @@ -2,7 +2,6 @@ import os import random import shutil import sys -import time import unittest from functools import partial from multiprocessing.sharedctypes import Synchronized @@ -49,75 +48,14 @@ _worker_id = 0 # Used to identify the worker process. ReturnT = TypeVar('ReturnT') # Constrain return type to match -def slow(slowness_reason: str) -> Callable[[Callable[..., ReturnT]], Callable[..., ReturnT]]: - ''' - This is a decorate that annotates a test as being "known - to be slow." The decorator will set expected_run_time and slowness_reason - as attributes of the function. Other code can use this annotation - as needed, e.g. to exclude these tests in "fast" mode. - ''' - def decorator(f: Callable[..., ReturnT]) -> Callable[..., ReturnT]: - setattr(f, 'slowness_reason', slowness_reason) - return f - - return decorator - -def is_known_slow_test(test_method: Callable[..., ReturnT]) -> bool: - return hasattr(test_method, 'slowness_reason') - def full_test_name(test: TestCase) -> str: test_module = test.__module__ test_class = test.__class__.__name__ test_method = test._testMethodName return f'{test_module}.{test_class}.{test_method}' -def get_test_method(test: TestCase) -> Callable[[], None]: - return getattr(test, test._testMethodName) - -# Each tuple is delay, test_name, slowness_reason -TEST_TIMINGS: List[Tuple[float, str, str]] = [] - - -def report_slow_tests() -> None: - timings = sorted(TEST_TIMINGS, reverse=True) - print('SLOWNESS REPORT') - print(' delay test') - print(' ---- ----') - for delay, test_name, slowness_reason in timings[:15]: - if not slowness_reason: - slowness_reason = 'UNKNOWN WHY SLOW, please investigate' - print(f' {delay:0.3f} {test_name}\n {slowness_reason}\n') - - print('...') - for delay, test_name, slowness_reason in timings[100:]: - if slowness_reason: - print(f' {delay:.3f} {test_name} is not that slow') - print(' consider removing @slow decorator') - print(f' This may no longer be true: {slowness_reason}') - -def enforce_timely_test_completion(test_method: Callable[..., ReturnT], test_name: str, - delay: float, result: unittest.TestResult) -> None: - if hasattr(test_method, 'slowness_reason'): - max_delay = 2.0 # seconds - else: - max_delay = 0.4 # seconds - - assert isinstance(result, (TextTestResult, RemoteTestResult)) - - if delay > max_delay: - msg = f'** Test is TOO slow: {test_name} ({delay:.3f} s)\n' - result.addInfo(test_method, msg) - -def fast_tests_only() -> bool: - return "FAST_TESTS_ONLY" in os.environ - def run_test(test: TestCase, result: TestResult) -> bool: failed = False - test_method = get_test_method(test) - - if fast_tests_only() and is_known_slow_test(test_method): - return failed - test_name = full_test_name(test) bounce_key_prefix_for_testing(test_name) @@ -129,15 +67,8 @@ def run_test(test: TestCase, result: TestResult) -> bool: result.addError(test, sys.exc_info()) return True - start_time = time.time() - test(result) # unittest will handle skipping, error, failure and success. - delay = time.time() - start_time - enforce_timely_test_completion(test_method, test_name, delay, result) - slowness_reason = getattr(test_method, 'slowness_reason', '') - TEST_TIMINGS.append((delay, test_name, slowness_reason)) - test._post_teardown() return failed diff --git a/zerver/tests/test_docs.py b/zerver/tests/test_docs.py index c588033545..54ff26505f 100644 --- a/zerver/tests/test_docs.py +++ b/zerver/tests/test_docs.py @@ -11,7 +11,6 @@ from django.test import TestCase, override_settings from zerver.lib.integrations import INTEGRATIONS from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import HostRequestMock -from zerver.lib.test_runner import slow from zerver.lib.utils import split_by from zerver.models import Realm, get_realm from zerver.views.documentation import add_api_uri_context @@ -89,7 +88,6 @@ class DocPageTest(ZulipTestCase): if not doc_html_str: self.assert_in_success_response([''], result) - @slow("Tests dozens of endpoints") def test_api_doc_endpoints(self) -> None: current_dir = os.path.dirname(os.path.abspath(__file__)) api_docs_dir = os.path.join(current_dir, '..', '..', 'templates/zerver/api/') @@ -105,7 +103,6 @@ class DocPageTest(ZulipTestCase): endpoint = f'/api/{os.path.splitext(f)[0]}' self._test(endpoint, '', doc_html_str=True) - @slow("Tests dozens of endpoints, including generating lots of emails") def test_doc_endpoints(self) -> None: self._test('/api/', 'The Zulip API') self._test('/api/api-keys', 'be careful with it') @@ -167,7 +164,6 @@ class DocPageTest(ZulipTestCase): self._test(url, title, doc_html_str=True) self._test(url, description, doc_html_str=True) - @slow("Tests dozens of endpoints, including all our integrations docs") def test_integration_doc_endpoints(self) -> None: self._test('/integrations/', 'native integrations.', diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index b6ef99aa60..a60d72df53 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -102,7 +102,6 @@ from zerver.lib.test_helpers import ( reset_emails_in_zulip_realm, stdout_suppressed, ) -from zerver.lib.test_runner import slow from zerver.lib.topic import ORIG_TOPIC, TOPIC_LINKS, TOPIC_NAME from zerver.lib.validator import ( Validator, @@ -1330,7 +1329,6 @@ class NormalActionsTest(BaseAction): schema_checker('events[0]', events[0]) avatar_schema_checker('events[1]', events[1]) - @slow("Runs a large matrix of tests") def test_change_realm_authentication_methods(self) -> None: schema_checker = check_events_dict([ ('type', equals('realm')), @@ -1432,7 +1430,6 @@ class NormalActionsTest(BaseAction): value)) schema_checker('events[0]', events[0]) - @slow("Runs a matrix of 6 queries to the /home view") def test_change_realm_message_edit_settings(self) -> None: schema_checker = check_events_dict([ ('type', equals('realm')), @@ -1558,7 +1555,6 @@ class NormalActionsTest(BaseAction): lambda: do_change_user_role(self.user_profile, role)) schema_checker('events[0]', events[0]) - @slow("Actually runs several full-stack fetching tests") def test_change_notification_settings(self) -> None: for notification_setting, v in self.user_profile.notification_setting_types.items(): if notification_setting in ["notification_sound", "desktop_icon_count_display"]: @@ -2504,7 +2500,6 @@ class RealmPropertyActionTest(BaseAction): state_change_expected=state_change_expected) schema_checker('events[0]', events[0]) - @slow("Actually runs several full-stack fetching tests") def test_change_realm_property(self) -> None: for prop in Realm.property_types: with self.settings(SEND_DIGEST_EMAILS=True): @@ -2582,17 +2577,14 @@ class UserDisplayActionTest(BaseAction): if setting_name == "timezone": timezone_schema_checker('events[1]', events[1]) - @slow("Actually runs several full-stack fetching tests") def test_set_user_display_settings(self) -> None: for prop in UserProfile.property_types: self.do_set_user_display_settings_test(prop) class SubscribeActionTest(BaseAction): - @slow("Actually several tests combined together") def test_subscribe_events(self) -> None: self.do_test_subscribe_events(include_subscribers=True) - @slow("Actually several tests combined together") def test_subscribe_events_no_include_subscribers(self) -> None: self.do_test_subscribe_events(include_subscribers=False) diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index 00ae60fee6..70a8fad464 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -16,7 +16,6 @@ from zerver.lib.events import add_realm_logo_fields from zerver.lib.soft_deactivation import do_soft_deactivate_users from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import get_user_messages, queries_captured -from zerver.lib.test_runner import slow from zerver.lib.users import compute_show_invites_and_add_streams from zerver.models import ( DefaultStream, @@ -320,7 +319,6 @@ class HomeTest(ZulipTestCase): self.assert_length(cache_mock.call_args_list, 6) self.assert_length(queries, 40) - @slow("Creates and subscribes 10 users in a loop. Should use bulk queries.") def test_num_queries_with_streams(self) -> None: main_user = self.example_user('hamlet') other_user = self.example_user('cordelia') @@ -510,7 +508,6 @@ class HomeTest(ZulipTestCase): page_params = self._get_page_params(result) self.assertEqual(page_params['realm_signup_notifications_stream_id'], get_stream('Denmark', realm).id) - @slow('creating users and loading home page') def test_people(self) -> None: hamlet = self.example_user('hamlet') realm = get_realm('zulip') @@ -891,7 +888,6 @@ class HomeTest(ZulipTestCase): idle_user_msg_list = get_user_messages(long_term_idle_user) self.assertEqual(idle_user_msg_list[-1].content, message) - @slow("Loads home page data several times testing different cases") def test_multiple_user_soft_deactivations(self) -> None: long_term_idle_user = self.example_user('hamlet') # We are sending this message to ensure that long_term_idle_user has diff --git a/zerver/tests/test_import_export.py b/zerver/tests/test_import_export.py index d510fdf3b0..66c41e2793 100644 --- a/zerver/tests/test_import_export.py +++ b/zerver/tests/test_import_export.py @@ -24,7 +24,6 @@ from zerver.lib.import_realm import do_import_realm, get_incoming_message_ids from zerver.lib.streams import create_stream_if_needed from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import create_s3_buckets, get_test_image_file, use_s3_backend -from zerver.lib.test_runner import slow from zerver.lib.topic_mutes import add_topic_mute from zerver.lib.upload import ( claim_attachment, @@ -70,7 +69,6 @@ class QueryUtilTest(ZulipTestCase): for _ in range(5): self.send_personal_message(user, self.example_user('othello')) - @slow('creates lots of data') def test_query_chunker(self) -> None: self._create_messages() diff --git a/zerver/tests/test_management_commands.py b/zerver/tests/test_management_commands.py index e48b5dd7a0..49846dee75 100644 --- a/zerver/tests/test_management_commands.py +++ b/zerver/tests/test_management_commands.py @@ -16,7 +16,6 @@ from zerver.lib.actions import do_add_reaction, do_create_user from zerver.lib.management import CommandError, ZulipBaseCommand, check_config from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import most_recent_message, stdout_suppressed -from zerver.lib.test_runner import slow from zerver.models import ( Message, Reaction, @@ -188,7 +187,6 @@ class TestCommandsCanStart(TestCase): if command != '__init__' ] - @slow("Aggregate of runs dozens of individual --help tests") def test_management_commands_show_help(self) -> None: with stdout_suppressed(): for command in self.commands: diff --git a/zerver/tests/test_markdown.py b/zerver/tests/test_markdown.py index 0ba9a9608b..9c7ca59738 100644 --- a/zerver/tests/test_markdown.py +++ b/zerver/tests/test_markdown.py @@ -42,7 +42,6 @@ from zerver.lib.mention import possible_mentions, possible_user_group_mentions from zerver.lib.message import render_markdown from zerver.lib.request import JsonableError from zerver.lib.test_classes import ZulipTestCase -from zerver.lib.test_runner import slow from zerver.lib.tex import render_tex from zerver.lib.user_groups import create_user_group from zerver.models import ( @@ -389,7 +388,6 @@ class MarkdownTest(ZulipTestCase): is_ignored = test.get('ignore', False) self.assertFalse(is_ignored, message) - @slow("Aggregate of runs dozens of individual markdown tests") def test_markdown_fixtures(self) -> None: format_tests, linkify_tests = self.load_markdown_tests() valid_keys = {"name", "input", "expected_output", diff --git a/zerver/tests/test_messages.py b/zerver/tests/test_messages.py index 303796fde5..f3063e5255 100644 --- a/zerver/tests/test_messages.py +++ b/zerver/tests/test_messages.py @@ -81,7 +81,6 @@ from zerver.lib.test_helpers import ( queries_captured, reset_emails_in_zulip_realm, ) -from zerver.lib.test_runner import slow from zerver.lib.timestamp import convert_to_UTC, datetime_to_timestamp from zerver.lib.timezone import get_timezone from zerver.lib.topic import DB_TOPIC_NAME, LEGACY_PREV_TOPIC, TOPIC_LINKS, TOPIC_NAME @@ -364,7 +363,6 @@ class TestCrossRealmPMs(ZulipTestCase): self.register(email, 'test', subdomain=subdomain) return get_user(email, get_realm(subdomain)) - @slow("Sends a large number of messages") @override_settings(CROSS_REALM_BOT_EMAILS=['notification-bot@zulip.com', 'welcome-bot@zulip.com', 'support@3.example.com']) @@ -806,7 +804,6 @@ class PersonalMessagesTest(ZulipTestCase): f'', ) - @slow("checks several profiles") def test_personal_to_self(self) -> None: """ If you send a personal to yourself, only you see it. @@ -1366,7 +1363,6 @@ class MessageDictTest(ZulipTestCase): self.assertEqual(send_message_payload, fetch_payload) - @slow('builds lots of messages') def test_bulk_message_fetching(self) -> None: sender = self.example_user('othello') receiver = self.example_user('hamlet') diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index 3cac2ad8fb..021cbbb5c7 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -22,7 +22,6 @@ from zerver.lib.send_email import send_future_email from zerver.lib.streams import create_stream_if_needed from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import reset_emails_in_zulip_realm, tornado_redirected_to_list -from zerver.lib.test_runner import slow from zerver.models import ( Attachment, CustomProfileField, @@ -738,7 +737,6 @@ class RealmAPITest(ZulipTestCase): realm = self.update_with_api(name, vals[0]) self.assertEqual(getattr(realm, name), vals[0]) - @slow("Tests a dozen properties in a loop") def test_update_realm_properties(self) -> None: for prop in Realm.property_types: with self.subTest(property=prop): diff --git a/zerver/tests/test_slack_message_conversion.py b/zerver/tests/test_slack_message_conversion.py index fe358aeef8..870723eecd 100644 --- a/zerver/tests/test_slack_message_conversion.py +++ b/zerver/tests/test_slack_message_conversion.py @@ -9,7 +9,6 @@ from zerver.data_import.slack_message_conversion import ( ) from zerver.lib import mdiff from zerver.lib.test_classes import ZulipTestCase -from zerver.lib.test_runner import slow class SlackMessageConversion(ZulipTestCase): @@ -31,7 +30,6 @@ class SlackMessageConversion(ZulipTestCase): return test_fixtures - @slow("Aggregate of runs of individual slack message conversion tests") def test_message_conversion_fixtures(self) -> None: format_tests = self.load_slack_message_conversion_tests() valid_keys = {'name', "input", "conversion_output"} diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 0bcc3e0eaf..5b58fa185f 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -64,7 +64,6 @@ from zerver.lib.test_helpers import ( reset_emails_in_zulip_realm, tornado_redirected_to_list, ) -from zerver.lib.test_runner import slow from zerver.models import ( DefaultStream, DefaultStreamGroup, @@ -3688,7 +3687,6 @@ class InviteOnlyStreamTest(ZulipTestCase): if sub['name'] == "Saxony": self.assertEqual(sub['invite_only'], True, "Saxony was not properly marked private") - @slow("lots of queries") def test_inviteonly(self) -> None: # Creating an invite-only stream is allowed hamlet = self.example_user('hamlet') @@ -3786,7 +3784,6 @@ class GetSubscribersTest(ZulipTestCase): stream_name = gather_subscriptions(self.user_profile)[0][0]['name'] self.make_successful_subscriber_request(stream_name) - @slow("common_subscribe_to_streams is slow") def test_gather_subscriptions(self) -> None: """ gather_subscriptions returns correct results with only 3 queries @@ -3855,7 +3852,6 @@ class GetSubscribersTest(ZulipTestCase): self.assertTrue(len(sub["subscribers"]) == len(users_to_subscribe)) self.assert_length(queries, 6) - @slow("common_subscribe_to_streams is slow") def test_never_subscribed_streams(self) -> None: """ Check never_subscribed streams are fetched correctly and not include invite_only streams. diff --git a/zerver/tests/test_urls.py b/zerver/tests/test_urls.py index 9b4747a4dc..8e459c46cf 100644 --- a/zerver/tests/test_urls.py +++ b/zerver/tests/test_urls.py @@ -7,7 +7,6 @@ import ujson from django.test import Client, TestCase from zerver.lib.test_classes import ZulipTestCase -from zerver.lib.test_runner import slow from zerver.models import Stream from zproject import urls @@ -25,7 +24,6 @@ class PublicURLTest(ZulipTestCase): self.assertEqual(response.status_code, expected_status, msg=f"Expected {expected_status}, received {response.status_code} for {method} to {url}") - @slow("Tests dozens of endpoints, including all of our /help/ documents") def test_public_urls(self) -> None: """ Test which views are accessible when not logged in.