From df43f86cbc1e40623f045dfdb1b60b3ca15ee323 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Sat, 19 Aug 2023 17:12:01 +0000 Subject: [PATCH] tests: Clean up check_has_permission_policies. I add a bunch of cute helper methods to make the test a bit more readable. And then I make sure to get clean objects, which precludes the need for our callback functions to refresh the user objects. And finally I make sure that our validation functions don't cause any round trips (assuming we have fetched objects using a standard Zulip helper, which example_user ensures.) --- zerver/lib/test_classes.py | 140 ++++++++++++++++-------------- zerver/tests/test_invite.py | 1 - zerver/tests/test_message_edit.py | 1 - zerver/tests/test_realm_emoji.py | 1 - zerver/tests/test_subs.py | 4 - zerver/tests/test_user_groups.py | 1 - 6 files changed, 77 insertions(+), 71 deletions(-) diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index f496ca16b0..6a265c7769 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -1662,79 +1662,93 @@ Output: self, policy: str, validation_func: Callable[[UserProfile], bool] ) -> None: realm = get_realm("zulip") - owner_user = self.example_user("desdemona") - admin_user = self.example_user("iago") - moderator_user = self.example_user("shiva") - member_user = self.example_user("hamlet") - new_member_user = self.example_user("othello") - guest_user = self.example_user("polonius") + + owner = "desdemona" + admin = "iago" + moderator = "shiva" + member = "hamlet" + new_member = "othello" + guest = "polonius" + + def set_age(user_name: str, age: int) -> None: + user = self.example_user(user_name) + user.date_joined = timezone_now() - timedelta(age) + user.save() do_set_realm_property(realm, "waiting_period_threshold", 1000, acting_user=None) - new_member_user.date_joined = timezone_now() - timedelta( - days=realm.waiting_period_threshold - 1 - ) - new_member_user.save() + set_age(member, age=realm.waiting_period_threshold + 1) + set_age(new_member, age=realm.waiting_period_threshold - 1) - member_user.date_joined = timezone_now() - timedelta( - days=realm.waiting_period_threshold + 1 - ) - member_user.save() + def allow(user_name: str) -> None: + # Fetch a clean object for the user. + user = self.example_user(user_name) + with self.assert_database_query_count(0): + self.assertTrue(validation_func(user)) - do_set_realm_property(realm, policy, Realm.POLICY_NOBODY, acting_user=None) - self.assertFalse(validation_func(owner_user)) - self.assertFalse(validation_func(admin_user)) - self.assertFalse(validation_func(moderator_user)) - self.assertFalse(validation_func(member_user)) - self.assertFalse(validation_func(new_member_user)) - self.assertFalse(validation_func(guest_user)) + def prevent(user_name: str) -> None: + # Fetch a clean object for the user. + user = self.example_user(user_name) + with self.assert_database_query_count(0): + self.assertFalse(validation_func(user)) - do_set_realm_property(realm, policy, Realm.POLICY_OWNERS_ONLY, acting_user=None) - self.assertTrue(validation_func(owner_user)) - self.assertFalse(validation_func(admin_user)) - self.assertFalse(validation_func(moderator_user)) - self.assertFalse(validation_func(member_user)) - self.assertFalse(validation_func(new_member_user)) - self.assertFalse(validation_func(guest_user)) + def set_policy(level: int) -> None: + do_set_realm_property(realm, policy, level, acting_user=None) - do_set_realm_property(realm, policy, Realm.POLICY_ADMINS_ONLY, acting_user=None) - self.assertTrue(validation_func(owner_user)) - self.assertTrue(validation_func(admin_user)) - self.assertFalse(validation_func(moderator_user)) - self.assertFalse(validation_func(member_user)) - self.assertFalse(validation_func(new_member_user)) - self.assertFalse(validation_func(guest_user)) + set_policy(Realm.POLICY_NOBODY) + prevent(owner) + prevent(admin) + prevent(moderator) + prevent(member) + prevent(new_member) + prevent(guest) - do_set_realm_property(realm, policy, Realm.POLICY_MODERATORS_ONLY, acting_user=None) - self.assertTrue(validation_func(owner_user)) - self.assertTrue(validation_func(admin_user)) - self.assertTrue(validation_func(moderator_user)) - self.assertFalse(validation_func(member_user)) - self.assertFalse(validation_func(new_member_user)) - self.assertFalse(validation_func(guest_user)) + set_policy(Realm.POLICY_OWNERS_ONLY) + allow(owner) + prevent(admin) + prevent(moderator) + prevent(member) + prevent(new_member) + prevent(guest) - do_set_realm_property(realm, policy, Realm.POLICY_FULL_MEMBERS_ONLY, acting_user=None) - self.assertTrue(validation_func(owner_user)) - self.assertTrue(validation_func(admin_user)) - self.assertTrue(validation_func(moderator_user)) - self.assertTrue(validation_func(member_user)) - self.assertFalse(validation_func(new_member_user)) - self.assertFalse(validation_func(guest_user)) + set_policy(Realm.POLICY_ADMINS_ONLY) + allow(owner) + allow(admin) + prevent(moderator) + prevent(member) + prevent(new_member) + prevent(guest) - do_set_realm_property(realm, policy, Realm.POLICY_MEMBERS_ONLY, acting_user=None) - self.assertTrue(validation_func(owner_user)) - self.assertTrue(validation_func(admin_user)) - self.assertTrue(validation_func(moderator_user)) - self.assertTrue(validation_func(member_user)) - self.assertTrue(validation_func(new_member_user)) - self.assertFalse(validation_func(guest_user)) + set_policy(Realm.POLICY_MODERATORS_ONLY) + allow(owner) + allow(admin) + allow(moderator) + prevent(member) + prevent(new_member) + prevent(guest) - do_set_realm_property(realm, policy, Realm.POLICY_EVERYONE, acting_user=None) - self.assertTrue(validation_func(owner_user)) - self.assertTrue(validation_func(admin_user)) - self.assertTrue(validation_func(moderator_user)) - self.assertTrue(validation_func(member_user)) - self.assertTrue(validation_func(new_member_user)) - self.assertTrue(validation_func(guest_user)) + set_policy(Realm.POLICY_FULL_MEMBERS_ONLY) + allow(owner) + allow(admin) + allow(moderator) + allow(member) + prevent(new_member) + prevent(guest) + + set_policy(Realm.POLICY_MEMBERS_ONLY) + allow(owner) + allow(admin) + allow(moderator) + allow(member) + allow(new_member) + prevent(guest) + + set_policy(Realm.POLICY_EVERYONE) + allow(owner) + allow(admin) + allow(moderator) + allow(member) + allow(new_member) + allow(guest) def subscribe_realm_to_manual_license_management_plan( self, realm: Realm, licenses: int, licenses_at_next_renewal: int, billing_schedule: int diff --git a/zerver/tests/test_invite.py b/zerver/tests/test_invite.py index f5dadc3361..bf7ddad00d 100644 --- a/zerver/tests/test_invite.py +++ b/zerver/tests/test_invite.py @@ -801,7 +801,6 @@ class InviteUserTest(InviteUserBase): def test_can_invite_others_to_realm(self) -> None: def validation_func(user_profile: UserProfile) -> bool: - user_profile.refresh_from_db() return user_profile.can_invite_users_by_email() realm = get_realm("zulip") diff --git a/zerver/tests/test_message_edit.py b/zerver/tests/test_message_edit.py index 327d6e9d74..15b00f8152 100644 --- a/zerver/tests/test_message_edit.py +++ b/zerver/tests/test_message_edit.py @@ -4566,7 +4566,6 @@ class EditMessageTest(EditMessageTestCase): def test_can_move_messages_between_streams(self) -> None: def validation_func(user_profile: UserProfile) -> bool: - user_profile.refresh_from_db() return user_profile.can_move_messages_between_streams() self.check_has_permission_policies("move_messages_between_streams_policy", validation_func) diff --git a/zerver/tests/test_realm_emoji.py b/zerver/tests/test_realm_emoji.py index 78fce0d44e..cfd63a473d 100644 --- a/zerver/tests/test_realm_emoji.py +++ b/zerver/tests/test_realm_emoji.py @@ -161,7 +161,6 @@ class RealmEmojiTest(ZulipTestCase): def test_can_add_custom_emoji(self) -> None: def validation_func(user_profile: UserProfile) -> bool: - user_profile.refresh_from_db() return user_profile.can_add_custom_emoji() self.check_has_permission_policies("add_custom_emoji_policy", validation_func) diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index f144c53280..59afb8ccc1 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -4401,13 +4401,11 @@ class SubscriptionAPITest(ZulipTestCase): if invite_only: def validation_func(user_profile: UserProfile) -> bool: - user_profile.refresh_from_db() return user_profile.can_create_private_streams() else: def validation_func(user_profile: UserProfile) -> bool: - user_profile.refresh_from_db() return user_profile.can_create_public_streams() self.check_has_permission_policies(stream_policy, validation_func) @@ -4420,7 +4418,6 @@ class SubscriptionAPITest(ZulipTestCase): def test_can_create_web_public_streams(self) -> None: def validation_func(user_profile: UserProfile) -> bool: - user_profile.refresh_from_db() return user_profile.can_create_web_public_streams() self.check_has_permission_policies("create_web_public_stream_policy", validation_func) @@ -4522,7 +4519,6 @@ class SubscriptionAPITest(ZulipTestCase): """ def validation_func(user_profile: UserProfile) -> bool: - user_profile.refresh_from_db() return user_profile.can_subscribe_other_users() self.check_has_permission_policies("invite_to_stream_policy", validation_func) diff --git a/zerver/tests/test_user_groups.py b/zerver/tests/test_user_groups.py index 2ad5cecd23..ad80b68650 100644 --- a/zerver/tests/test_user_groups.py +++ b/zerver/tests/test_user_groups.py @@ -427,7 +427,6 @@ class UserGroupAPITestCase(UserGroupTestCase): def test_can_edit_user_groups(self) -> None: def validation_func(user_profile: UserProfile) -> bool: - user_profile.refresh_from_db() return user_profile.can_edit_user_groups() self.check_has_permission_policies("user_group_edit_policy", validation_func)