Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

tests: Make soft-reactivation tests readable. 

The `expected` flag was incredibly confusing, as you
couldn't tell from the calling code what you were
actually expecting to happen.

I avoid the context manager idiom in order to force
the callers to create simple helper functions, and
I de-duplicate some code in some places.

I also force the caller to explicitly soft-deactivate
the user with one simple line of code, so that the
person reading the test doesn't have to research
the side effects of the helper. (And I make it
very easy for new authors to follow the practice
going forward.)

This is also somewhat of a prep commit to avoid
the obfuscated use of refresh_from_db.
This commit is contained in:
Steve Howell 2023-08-19 13:49:00 +00:00 committed by Tim Abbott
parent 31d2660ee2
commit a8f5836ee6
4 changed files with 90 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
zerver/lib/test_classes.py
View File

@ -1887,23 +1887,32 @@ Output:
UserGroupMembership.objects.filter(user_profile=user, user_group=user_group).exists()
)
@contextmanager
def soft_deactivate_and_check_long_term_idle(
self, user: UserProfile, expected: bool
) -> Iterator[None]:
"""
Ensure that the user is soft deactivated (long term idle), and check if the user
has been reactivated when exiting the context with an assertion
"""
def _assert_long_term_idle(self, user: UserProfile) -> None:
if not user.long_term_idle:
do_soft_deactivate_users([user])
self.assertTrue(user.long_term_idle)
try:
yield
finally:
# Prevent from using the old user object
user.refresh_from_db()
self.assertEqual(user.long_term_idle, expected)
raise AssertionError(
"""
We expect you to explicitly call self.soft_deactivate_user
if your user is not already soft-deactivated.
"""
)
def expect_soft_reactivation(self, user: UserProfile, action: Callable[[], None]) -> None:
self._assert_long_term_idle(user)
action()
# Prevent from using the old user object
user.refresh_from_db()
self.assertEqual(user.long_term_idle, False)
def expect_to_stay_long_term_idle(self, user: UserProfile, action: Callable[[], None]) -> None:
self._assert_long_term_idle(user)
action()
# Prevent from using the old user object
user.refresh_from_db()
self.assertEqual(user.long_term_idle, True)
def soft_deactivate_user(self, user: UserProfile) -> None:
do_soft_deactivate_users([user])
assert user.long_term_idle
class ZulipTestCase(ZulipTestCaseMixin, TestCase):

57
zerver/tests/test_message_notification_emails.py
View File

@ -1635,12 +1635,17 @@ class TestMessageNotificationEmails(ZulipTestCase):
get_realm("zulip"), "large_user_group", [hamlet, othello, cordelia], acting_user=None
)
def reset_hamlet_as_soft_deactivated_user() -> None:
nonlocal hamlet
hamlet = self.example_user("hamlet")
self.soft_deactivate_user(hamlet)
# Do note that the event dicts for the missed messages are constructed by hand
# The part of testing the consumption of missed messages by the worker is left to
# test_queue_worker.test_missed_message_worker
# Personal mention in a stream message should soft reactivate the user
with self.soft_deactivate_and_check_long_term_idle(hamlet, expected=False):
def send_personal_mention() -> None:
mention = f"@**{hamlet.full_name}**"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_missedmessage_emails(
@ -1652,8 +1657,11 @@ class TestMessageNotificationEmails(ZulipTestCase):
},
)
reset_hamlet_as_soft_deactivated_user()
self.expect_soft_reactivation(hamlet, send_personal_mention)
# Direct message should soft reactivate the user
with self.soft_deactivate_and_check_long_term_idle(hamlet, expected=False):
def send_direct_message() -> None:
# Soft reactivate the user by sending a personal message
personal_message_id = self.send_personal_message(othello, hamlet, "Message")
handle_missedmessage_emails(
@ -1665,6 +1673,9 @@ class TestMessageNotificationEmails(ZulipTestCase):
},
)
reset_hamlet_as_soft_deactivated_user()
self.expect_soft_reactivation(hamlet, send_direct_message)
# Hamlet FOLLOWS the topic.
# 'wildcard_mentions_notify' is disabled to verify the corner case when only
# 'enable_followed_topic_wildcard_mentions_notify' is enabled (True by default).
@ -1679,7 +1690,8 @@ class TestMessageNotificationEmails(ZulipTestCase):
# Topic wildcard mention in followed topic should soft reactivate the user
# hamlet should be a topic participant
self.send_stream_message(hamlet, "Denmark", "test message")
with self.soft_deactivate_and_check_long_term_idle(hamlet, expected=False):
def send_topic_wildcard_mention() -> None:
mention = "@**topic**"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_missedmessage_emails(
@ -1691,8 +1703,11 @@ class TestMessageNotificationEmails(ZulipTestCase):
},
)
reset_hamlet_as_soft_deactivated_user()
self.expect_soft_reactivation(hamlet, send_topic_wildcard_mention)
# Stream wildcard mention in followed topic should NOT soft reactivate the user
with self.soft_deactivate_and_check_long_term_idle(hamlet, expected=True):
def send_stream_wildcard_mention() -> None:
mention = "@**all**"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_missedmessage_emails(
@ -1704,6 +1719,9 @@ class TestMessageNotificationEmails(ZulipTestCase):
},
)
reset_hamlet_as_soft_deactivated_user()
self.expect_to_stay_long_term_idle(hamlet, send_stream_wildcard_mention)
# Reset
do_set_user_topic_visibility_policy(
hamlet,
@ -1714,33 +1732,15 @@ class TestMessageNotificationEmails(ZulipTestCase):
do_change_user_setting(hamlet, "wildcard_mentions_notify", True, acting_user=None)
# Topic Wildcard mention should soft reactivate the user
with self.soft_deactivate_and_check_long_term_idle(hamlet, expected=False):
mention = "@**topic**"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_missedmessage_emails(
hamlet.id,
{
stream_mentioned_message_id: MissedMessageData(
trigger=NotificationTriggers.TOPIC_WILDCARD_MENTION
),
},
)
reset_hamlet_as_soft_deactivated_user()
self.expect_soft_reactivation(hamlet, send_topic_wildcard_mention)
# Stream Wildcard mention should NOT soft reactivate the user
with self.soft_deactivate_and_check_long_term_idle(hamlet, expected=True):
mention = "@**all**"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_missedmessage_emails(
hamlet.id,
{
stream_mentioned_message_id: MissedMessageData(
trigger=NotificationTriggers.STREAM_WILDCARD_MENTION
),
},
)
reset_hamlet_as_soft_deactivated_user()
self.expect_to_stay_long_term_idle(hamlet, send_stream_wildcard_mention)
# Group mention should NOT soft reactivate the user
with self.soft_deactivate_and_check_long_term_idle(hamlet, expected=True):
def send_group_mention() -> None:
mention = "@*large_user_group*"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_missedmessage_emails(
@ -1753,6 +1753,9 @@ class TestMessageNotificationEmails(ZulipTestCase):
},
)
reset_hamlet_as_soft_deactivated_user()
self.expect_to_stay_long_term_idle(hamlet, send_group_mention)
def test_followed_topic_missed_message(self) -> None:
hamlet = self.example_user("hamlet")
othello = self.example_user("othello")

57
zerver/tests/test_push_notifications.py
View File

@ -58,7 +58,6 @@ from zerver.lib.remote_server import (
send_to_push_bouncer,
)
from zerver.lib.response import json_response_from_error
from zerver.lib.soft_deactivation import do_soft_deactivate_users
from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.test_helpers import mock_queue_publish
from zerver.lib.timestamp import datetime_to_timestamp
@ -1068,6 +1067,10 @@ class PushNotificationTest(BouncerTestCase):
class HandlePushNotificationTest(PushNotificationTest):
DEFAULT_SUBDOMAIN = ""
def soft_deactivate_main_user(self) -> None:
self.user_profile = self.example_user("hamlet")
self.soft_deactivate_user(self.user_profile)
def request_callback(self, request: PreparedRequest) -> Tuple[int, ResponseHeaders, bytes]:
assert request.url is not None # allow mypy to infer url is present.
assert settings.PUSH_NOTIFICATION_BOUNCER_URL is not None
@ -1592,7 +1595,8 @@ class HandlePushNotificationTest(PushNotificationTest):
self.subscribe(sender, "public_stream")
logger_string = "zulip.soft_deactivation"
with self.assertLogs(logger_string, level="INFO") as info_logs:
do_soft_deactivate_users([self.user_profile])
self.soft_deactivate_main_user()
self.assertEqual(
info_logs.output,
[
@ -1649,7 +1653,7 @@ class HandlePushNotificationTest(PushNotificationTest):
)
# Personal mention in a stream message should soft reactivate the user
with self.soft_deactivate_and_check_long_term_idle(self.user_profile, expected=False):
def mention_in_stream() -> None:
mention = f"@**{self.user_profile.full_name}**"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_push_notification(
@ -1660,8 +1664,11 @@ class HandlePushNotificationTest(PushNotificationTest):
},
)
self.soft_deactivate_main_user()
self.expect_soft_reactivation(self.user_profile, mention_in_stream)
# Direct message should soft reactivate the user
with self.soft_deactivate_and_check_long_term_idle(self.user_profile, expected=False):
def direct_message() -> None:
# Soft reactivate the user by sending a personal message
personal_message_id = self.send_personal_message(othello, self.user_profile, "Message")
handle_push_notification(
@ -1672,6 +1679,9 @@ class HandlePushNotificationTest(PushNotificationTest):
},
)
self.soft_deactivate_main_user()
self.expect_soft_reactivation(self.user_profile, direct_message)
# User FOLLOWS the topic.
# 'wildcard_mentions_notify' is disabled to verify the corner case when only
# 'enable_followed_topic_wildcard_mentions_notify' is enabled (True by default).
@ -1688,7 +1698,8 @@ class HandlePushNotificationTest(PushNotificationTest):
# Topic wildcard mention in followed topic should soft reactivate the user
# user should be a topic participant
self.send_stream_message(self.user_profile, "Denmark", "topic paticipant")
with self.soft_deactivate_and_check_long_term_idle(self.user_profile, expected=False):
def send_topic_wildcard_mention() -> None:
mention = "@**topic**"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_push_notification(
@ -1699,8 +1710,11 @@ class HandlePushNotificationTest(PushNotificationTest):
},
)
self.soft_deactivate_main_user()
self.expect_soft_reactivation(self.user_profile, send_topic_wildcard_mention)
# Stream wildcard mention in followed topic should NOT soft reactivate the user
with self.soft_deactivate_and_check_long_term_idle(self.user_profile, expected=True):
def send_stream_wildcard_mention() -> None:
mention = "@**all**"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_push_notification(
@ -1711,6 +1725,9 @@ class HandlePushNotificationTest(PushNotificationTest):
},
)
self.soft_deactivate_main_user()
self.expect_to_stay_long_term_idle(self.user_profile, send_stream_wildcard_mention)
# Reset
do_set_user_topic_visibility_policy(
self.user_profile,
@ -1723,31 +1740,14 @@ class HandlePushNotificationTest(PushNotificationTest):
)
# Topic Wildcard mention should soft reactivate the user
with self.soft_deactivate_and_check_long_term_idle(self.user_profile, expected=False):
mention = "@**topic**"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_push_notification(
self.user_profile.id,
{
"message_id": stream_mentioned_message_id,
"trigger": NotificationTriggers.TOPIC_WILDCARD_MENTION,
},
)
self.expect_soft_reactivation(self.user_profile, send_topic_wildcard_mention)
# Stream Wildcard mention should NOT soft reactivate the user
with self.soft_deactivate_and_check_long_term_idle(self.user_profile, expected=True):
mention = "@**all**"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_push_notification(
self.user_profile.id,
{
"message_id": stream_mentioned_message_id,
"trigger": NotificationTriggers.STREAM_WILDCARD_MENTION,
},
)
self.soft_deactivate_main_user()
self.expect_to_stay_long_term_idle(self.user_profile, send_stream_wildcard_mention)
# Group mention should NOT soft reactivate the user
with self.soft_deactivate_and_check_long_term_idle(self.user_profile, expected=True):
def send_group_mention() -> None:
mention = "@*large_user_group*"
stream_mentioned_message_id = self.send_stream_message(othello, "Denmark", mention)
handle_push_notification(
@ -1759,6 +1759,9 @@ class HandlePushNotificationTest(PushNotificationTest):
},
)
self.soft_deactivate_main_user()
self.expect_to_stay_long_term_idle(self.user_profile, send_group_mention)
@mock.patch("zerver.lib.push_notifications.logger.info")
@mock.patch("zerver.lib.push_notifications.push_notifications_enabled", return_value=True)
def test_user_push_notification_already_active(

6
zerver/tests/test_signup.py
View File

@ -754,7 +754,8 @@ class PasswordResetTest(ZulipTestCase):
def test_password_reset_for_soft_deactivated_user(self) -> None:
user_profile = self.example_user("hamlet")
email = user_profile.delivery_email
with self.soft_deactivate_and_check_long_term_idle(user_profile, False):
def reset_password() -> None:
# start the password reset process by supplying an email address
result = self.client_post("/accounts/password/reset/", {"email": email})
@ -762,6 +763,9 @@ class PasswordResetTest(ZulipTestCase):
self.assertEqual(result.status_code, 302)
self.assertTrue(result["Location"].endswith("/accounts/password/reset/done/"))
self.soft_deactivate_user(user_profile)
self.expect_soft_reactivation(user_profile, reset_password)
class LoginTest(ZulipTestCase):
"""