test_push_notifications: Fix leak that can leak to test flakes.

While reviewing #11012, I discovered a nondeterministic result for
test_signup, which I tracked down to specifically this triple of tests
failing when run in this order:

test-backend GCMSuccessTest \
  zerver.tests.test_push_notifications.TestAPNs.test_get_apns_client \
  zerver.tests.test_signup.LoginTest.test_register

with a query count mismatch like this:

expected length: 73
actual length: 79

Comparing the list of queries, it's clear that test_register was
seeing `push_notifications_enabled()` returning True in this test order.

It's not clear why GCMSuccessTest was required here (it was!), but
further debugging determined the problem was that
`test_get_apns_client` left the _apns_client initialization system in
a state where get_apns_client would return a non-None value, resulting
in push_notifications_enabled() returning True for future tests.

The immediate fix is to just reset the `_apns_client` and
`_apns_client_initializedstate` state properly after the test runs;
but arguably we should do a larger refactor to make this less
fragile.
This commit is contained in:
Tim Abbott
2018-12-15 11:05:43 -08:00
parent 111eda604b
commit a63eae48cc

View File

@@ -707,12 +707,20 @@ class TestAPNs(PushNotificationTest):
self.user_profile.id, devices, payload_data)
def test_get_apns_client(self) -> None:
"""This test is pretty hacky, and needs to carefully reset the state
it modifies in order to avoid leaking state that can lead to
nondeterministic results for other tests.
"""
import zerver.lib.push_notifications
zerver.lib.push_notifications._apns_client_initialized = False
with self.settings(APNS_CERT_FILE='/foo.pem'), \
mock.patch('apns2.client.APNsClient') as mock_client:
client = get_apns_client()
self.assertEqual(mock_client.return_value, client)
# Reset the values set by `get_apns_client` so that we don't
# leak changes to the rest of the world.
zerver.lib.push_notifications._apns_client_initialized = False
zerver.lib.push_notifications._apns_client = None
def test_not_configured(self) -> None:
with mock.patch('zerver.lib.push_notifications.get_apns_client') as mock_get, \