From c3c7b3335167da2f1050670d8c6ea51add4e4ff1 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Fri, 10 Aug 2018 13:43:58 -0700 Subject: [PATCH] tests: Move clear_client_event_queues_for_testing to ZulipTestCase. Following recent testing flakes that were traced down to this not having been called causing `receiver_is_off_zulip` to depend on test ordering, it makes sense to centralize this. I think it should always have been in ZulipTestCase; it appears the reason it wasn't from the beginning was that originally only test_events.py interacted with it, and do_test there still needs to call this directly (because it can be called multiple times within a single test). And then we did the wrong thing as expanded use of Tornado event_queue code in tests to more of the codebase. --- zerver/lib/test_classes.py | 8 ++++++-- zerver/tests/test_event_queue.py | 4 +--- zerver/tests/test_events.py | 22 +--------------------- zerver/tests/test_tornado.py | 5 ----- 4 files changed, 8 insertions(+), 31 deletions(-) diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index 474f3ebba7..222360fc83 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -51,10 +51,9 @@ from zerver.models import ( Subscription, UserProfile, ) - from zilencer.models import get_remote_server_by_uuid from zerver.decorator import do_two_factor_login - +from zerver.tornado.event_queue import clear_client_event_queues_for_testing import base64 import mock @@ -91,6 +90,11 @@ class ZulipTestCase(TestCase): # Ensure that the test system just shows us diffs maxDiff = None # type: Optional[int] + def tearDown(self) -> None: + super().tearDown() + # Important: we need to clear event queues to avoid leaking data to future tests. + clear_client_event_queues_for_testing() + ''' WRAPPER_COMMENT: diff --git a/zerver/tests/test_event_queue.py b/zerver/tests/test_event_queue.py index 25c35de1d0..ac27f32b72 100644 --- a/zerver/tests/test_event_queue.py +++ b/zerver/tests/test_event_queue.py @@ -10,7 +10,7 @@ from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import POSTRequestMock from zerver.models import Recipient, Stream, Subscription, UserProfile, get_stream from zerver.tornado.event_queue import maybe_enqueue_notifications, \ - allocate_client_descriptor, process_message_event, clear_client_event_queues_for_testing, \ + allocate_client_descriptor, process_message_event, \ get_client_descriptor, missedmessage_hook from zerver.tornado.views import get_events @@ -139,8 +139,6 @@ class MissedMessageNotificationsTest(ZulipTestCase): self.unsubscribe(hamlet, stream_name) - clear_client_event_queues_for_testing() - queue_data = dict( all_public_streams=True, apply_markdown=True, diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index c58067eafd..b48e6d1664 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -160,11 +160,6 @@ class LogEventsTest(ZulipTestCase): class EventsEndpointTest(ZulipTestCase): - def tearDown(self) -> None: - super().tearDown() - # Important: we need to clear event queues to avoid leaking data to future tests. - clear_client_event_queues_for_testing() - def test_events_register_endpoint(self) -> None: # This test is intended to get minimal coverage on the @@ -270,11 +265,6 @@ class EventsEndpointTest(ZulipTestCase): self.assert_json_success(result) class GetEventsTest(ZulipTestCase): - def tearDown(self) -> None: - super().tearDown() - # Important: we need to clear event queues to avoid leaking data to future tests. - clear_client_event_queues_for_testing() - def tornado_call(self, view_func: Callable[[HttpRequest, UserProfile], HttpResponse], user_profile: UserProfile, post_data: Dict[str, Any]) -> HttpResponse: @@ -458,11 +448,6 @@ class GetEventsTest(ZulipTestCase): self.assertEqual(message["avatar_url"], None) class EventsRegisterTest(ZulipTestCase): - def tearDown(self) -> None: - super().tearDown() - # Important: we need to clear event queues to avoid leaking data to future tests. - clear_client_event_queues_for_testing() - def setUp(self) -> None: super().setUp() self.user_profile = self.example_user('hamlet') @@ -487,7 +472,7 @@ class EventsRegisterTest(ZulipTestCase): ''' Make sure we have a clean slate of client descriptors for these tests. If we don't do this, then certain failures will only manifest when you - run multiple tests. + run multiple tests within a single test function. ''' clear_client_event_queues_for_testing() @@ -2755,11 +2740,6 @@ class EventQueueTest(TestCase): "timestamp": "1"}]) class ClientDescriptorsTest(ZulipTestCase): - def tearDown(self) -> None: - super().tearDown() - # Important: we need to clear event queues to avoid leaking data to future tests. - clear_client_event_queues_for_testing() - def test_get_client_info_for_all_public_streams(self) -> None: hamlet = self.example_user('hamlet') realm = hamlet.realm diff --git a/zerver/tests/test_tornado.py b/zerver/tests/test_tornado.py index db8fdc4d3f..30c35c14c1 100644 --- a/zerver/tests/test_tornado.py +++ b/zerver/tests/test_tornado.py @@ -50,8 +50,6 @@ class TornadoWebTestCase(AsyncHTTPTestCase, ZulipTestCase): def tearDown(self) -> None: super().tearDown() self.session_cookie = None # type: Optional[Dict[str, str]] - # Important: we need to clear event queues to avoid leaking data to future tests. - event_queue.clear_client_event_queues_for_testing() @override_settings(DEBUG=False) def get_app(self) -> Application: @@ -143,7 +141,6 @@ class EventsTestCase(TornadoWebTestCase): self.assertEqual(data['result'], 'success') class WebSocketBaseTestCase(AsyncHTTPTestCase, ZulipTestCase): - def setUp(self) -> None: settings.RUNNING_INSIDE_TORNADO = True super().setUp() @@ -151,8 +148,6 @@ class WebSocketBaseTestCase(AsyncHTTPTestCase, ZulipTestCase): def tearDown(self) -> None: super().tearDown() settings.RUNNING_INSIDE_TORNADO = False - # Important: we need to clear event queues to avoid leaking data to future tests. - event_queue.clear_client_event_queues_for_testing() @gen.coroutine def ws_connect(self, path: str, cookie_header: str,