From 4890cc5ba43ad73229ce4d2fe240acaf39194edb Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Tue, 14 Apr 2020 08:30:18 +0300 Subject: Created tests for `bot.cogs.logging` connected message. --- tests/bot/cogs/test_logging.py | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 tests/bot/cogs/test_logging.py (limited to 'tests') diff --git a/tests/bot/cogs/test_logging.py b/tests/bot/cogs/test_logging.py new file mode 100644 index 000000000..ba98a5a56 --- /dev/null +++ b/tests/bot/cogs/test_logging.py @@ -0,0 +1,42 @@ +import unittest +from unittest.mock import patch + +from bot import constants +from bot.cogs.logging import Logging +from tests.helpers import MockBot, MockTextChannel + + +class LoggingTests(unittest.IsolatedAsyncioTestCase): + """Test cases for connected login.""" + + def setUp(self): + self.bot = MockBot() + self.cog = Logging(self.bot) + self.dev_log = MockTextChannel(id=1234, name="dev-log") + + @patch("bot.cogs.logging.DEBUG_MODE", False) + async def test_debug_mode_false(self): + """Should send connected message to dev-log.""" + self.bot.get_channel.return_value = self.dev_log + + await self.cog.startup_greeting() + self.bot.wait_until_guild_available.assert_awaited_once_with() + self.bot.get_channel.assert_called_once_with(constants.Channels.dev_log) + + embed = self.dev_log.send.call_args[1]['embed'] + self.dev_log.send.assert_awaited_once_with(embed=embed) + + self.assertEqual(embed.description, "Connected!") + self.assertEqual(embed.author.name, "Python Bot") + self.assertEqual(embed.author.url, "https://github.com/python-discord/bot") + self.assertEqual( + embed.author.icon_url, + "https://raw.githubusercontent.com/python-discord/branding/master/logos/logo_circle/logo_circle_large.png" + ) + + @patch("bot.cogs.logging.DEBUG_MODE", True) + async def test_debug_mode_true(self): + """Should not send anything to dev-log.""" + await self.cog.startup_greeting() + self.bot.wait_until_guild_available.assert_awaited_once_with() + self.bot.get_channel.assert_not_called() -- cgit v1.2.3 From ededd1879cfb914445342b202d4c66aed23ee94b Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Fri, 22 May 2020 08:43:10 +0300 Subject: Logging Tests: Simplify `DEBUG_MODE` `False` test - Remove embed attributes checks - Replace `self.dev_log.assert_awaited_once_with` with `self.dev_log.assert_awaited_once`. --- tests/bot/cogs/test_logging.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/test_logging.py b/tests/bot/cogs/test_logging.py index ba98a5a56..8a18fdcd6 100644 --- a/tests/bot/cogs/test_logging.py +++ b/tests/bot/cogs/test_logging.py @@ -22,17 +22,7 @@ class LoggingTests(unittest.IsolatedAsyncioTestCase): await self.cog.startup_greeting() self.bot.wait_until_guild_available.assert_awaited_once_with() self.bot.get_channel.assert_called_once_with(constants.Channels.dev_log) - - embed = self.dev_log.send.call_args[1]['embed'] - self.dev_log.send.assert_awaited_once_with(embed=embed) - - self.assertEqual(embed.description, "Connected!") - self.assertEqual(embed.author.name, "Python Bot") - self.assertEqual(embed.author.url, "https://github.com/python-discord/bot") - self.assertEqual( - embed.author.icon_url, - "https://raw.githubusercontent.com/python-discord/branding/master/logos/logo_circle/logo_circle_large.png" - ) + self.dev_log.send.assert_awaited_once() @patch("bot.cogs.logging.DEBUG_MODE", True) async def test_debug_mode_true(self): -- cgit v1.2.3 From 876b4846f612fe0011cc2e0b498b4df9e54d74cb Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sun, 31 May 2020 19:17:07 +0200 Subject: Add support for bool values in RedisCache We're gonna need this for the help channel handling, and it seems like a reasonable type to support anyway. It requires a tiny bit of special handling, but nothing outrageous. --- bot/utils/redis_cache.py | 14 ++++++++++++-- tests/bot/utils/test_redis_cache.py | 4 +++- 2 files changed, 15 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/bot/utils/redis_cache.py b/bot/utils/redis_cache.py index de80cee84..2926e7a89 100644 --- a/bot/utils/redis_cache.py +++ b/bot/utils/redis_cache.py @@ -2,6 +2,7 @@ from __future__ import annotations import asyncio import logging +from distutils.util import strtobool from functools import partialmethod from typing import Any, Dict, ItemsView, Optional, Tuple, Union @@ -11,7 +12,7 @@ log = logging.getLogger(__name__) # Type aliases RedisKeyType = Union[str, int] -RedisValueType = Union[str, int, float] +RedisValueType = Union[str, int, float, bool] RedisKeyOrValue = Union[RedisKeyType, RedisValueType] # Prefix tuples @@ -20,6 +21,7 @@ _VALUE_PREFIXES = ( ("f|", float), ("i|", int), ("s|", str), + ("b|", bool), ) _KEY_PREFIXES = ( ("i|", int), @@ -117,7 +119,8 @@ class RedisCache: def _to_typestring(key_or_value: RedisKeyOrValue, prefixes: _PrefixTuple) -> str: """Turn a valid Redis type into a typestring.""" for prefix, _type in prefixes: - if isinstance(key_or_value, _type): + # isinstance is a bad idea here, because isintance(False, int) == True. + if type(key_or_value) is _type: return f"{prefix}{key_or_value}" raise TypeError(f"RedisCache._to_typestring only supports the following: {prefixes}.") @@ -131,6 +134,13 @@ class RedisCache: # Now we convert our unicode string back into the type it originally was. for prefix, _type in prefixes: if key_or_value.startswith(prefix): + + # For booleans, we need special handling because bool("False") is True. + if prefix == "b|": + value = key_or_value[len(prefix):] + return bool(strtobool(value)) + + # Otherwise we can just convert normally. return _type(key_or_value[len(prefix):]) raise TypeError(f"RedisCache._from_typestring only supports the following: {prefixes}.") diff --git a/tests/bot/utils/test_redis_cache.py b/tests/bot/utils/test_redis_cache.py index 8c1a40640..62c411681 100644 --- a/tests/bot/utils/test_redis_cache.py +++ b/tests/bot/utils/test_redis_cache.py @@ -59,7 +59,9 @@ class RedisCacheTests(unittest.IsolatedAsyncioTestCase): test_cases = ( ('favorite_fruit', 'melon'), ('favorite_number', 86), - ('favorite_fraction', 86.54) + ('favorite_fraction', 86.54), + ('favorite_boolean', False), + ('other_boolean', True), ) # Test that we can get and set different types. -- cgit v1.2.3 From 81e50cb2c970fc5c203e135434f897b6a3f7e52a Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 14 Jun 2020 23:34:28 -0700 Subject: Sync tests: test listeners ignore events from other guilds --- tests/bot/cogs/sync/test_cog.py | 64 ++++++++++++++++++++++++++++++++++------- 1 file changed, 54 insertions(+), 10 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/sync/test_cog.py b/tests/bot/cogs/sync/test_cog.py index 14fd909c4..d7d60e961 100644 --- a/tests/bot/cogs/sync/test_cog.py +++ b/tests/bot/cogs/sync/test_cog.py @@ -131,6 +131,12 @@ class SyncCogListenerTests(SyncCogTestCase): super().setUp() self.cog.patch_user = mock.AsyncMock(spec_set=self.cog.patch_user) + self.guild_id_patcher = mock.patch("bot.cogs.sync.cog.constants.Guild.id", 5) + self.guild_id = self.guild_id_patcher.start() + + def tearDown(self): + self.guild_id_patcher.stop() + async def test_sync_cog_on_guild_role_create(self): """A POST request should be sent with the new role's data.""" self.assertTrue(self.cog.on_guild_role_create.__cog_listener__) @@ -142,20 +148,32 @@ class SyncCogListenerTests(SyncCogTestCase): "permissions": 8, "position": 23, } - role = helpers.MockRole(**role_data) + role = helpers.MockRole(**role_data, guild=self.guild_id) await self.cog.on_guild_role_create(role) self.bot.api_client.post.assert_called_once_with("bot/roles", json=role_data) + async def test_sync_cog_on_guild_role_create_ignores_guilds(self): + """Events from other guilds should be ignored.""" + role = helpers.MockRole(guild=0) + await self.cog.on_guild_role_create(role) + self.bot.api_client.post.assert_not_awaited() + async def test_sync_cog_on_guild_role_delete(self): """A DELETE request should be sent.""" self.assertTrue(self.cog.on_guild_role_delete.__cog_listener__) - role = helpers.MockRole(id=99) + role = helpers.MockRole(id=99, guild=self.guild_id) await self.cog.on_guild_role_delete(role) self.bot.api_client.delete.assert_called_once_with("bot/roles/99") + async def test_sync_cog_on_guild_role_delete_ignores_guilds(self): + """Events from other guilds should be ignored.""" + role = helpers.MockRole(guild=0) + await self.cog.on_guild_role_delete(role) + self.bot.api_client.delete.assert_not_awaited() + async def test_sync_cog_on_guild_role_update(self): """A PUT request should be sent if the colour, name, permissions, or position changes.""" self.assertTrue(self.cog.on_guild_role_update.__cog_listener__) @@ -180,8 +198,8 @@ class SyncCogListenerTests(SyncCogTestCase): after_role_data = role_data.copy() after_role_data[attribute] = 876 - before_role = helpers.MockRole(**role_data) - after_role = helpers.MockRole(**after_role_data) + before_role = helpers.MockRole(**role_data, guild=self.guild_id) + after_role = helpers.MockRole(**after_role_data, guild=self.guild_id) await self.cog.on_guild_role_update(before_role, after_role) @@ -193,11 +211,17 @@ class SyncCogListenerTests(SyncCogTestCase): else: self.bot.api_client.put.assert_not_called() + async def test_sync_cog_on_guild_role_update_ignores_guilds(self): + """Events from other guilds should be ignored.""" + role = helpers.MockRole(guild=0) + await self.cog.on_guild_role_update(role, role) + self.bot.api_client.put.assert_not_awaited() + async def test_sync_cog_on_member_remove(self): - """Member should patched to set in_guild as False.""" + """Member should be patched to set in_guild as False.""" self.assertTrue(self.cog.on_member_remove.__cog_listener__) - member = helpers.MockMember() + member = helpers.MockMember(guild=self.guild_id) await self.cog.on_member_remove(member) self.cog.patch_user.assert_called_once_with( @@ -205,14 +229,20 @@ class SyncCogListenerTests(SyncCogTestCase): updated_information={"in_guild": False} ) + async def test_sync_cog_on_member_remove_ignores_guilds(self): + """Events from other guilds should be ignored.""" + member = helpers.MockMember(guild=0) + await self.cog.on_member_remove(member) + self.cog.patch_user.assert_not_awaited() + async def test_sync_cog_on_member_update_roles(self): """Members should be patched if their roles have changed.""" self.assertTrue(self.cog.on_member_update.__cog_listener__) # Roles are intentionally unsorted. before_roles = [helpers.MockRole(id=12), helpers.MockRole(id=30), helpers.MockRole(id=20)] - before_member = helpers.MockMember(roles=before_roles) - after_member = helpers.MockMember(roles=before_roles[1:]) + before_member = helpers.MockMember(roles=before_roles, guild=self.guild_id) + after_member = helpers.MockMember(roles=before_roles[1:], guild=self.guild_id) await self.cog.on_member_update(before_member, after_member) @@ -233,13 +263,19 @@ class SyncCogListenerTests(SyncCogTestCase): with self.subTest(attribute=attribute): self.cog.patch_user.reset_mock() - before_member = helpers.MockMember(**{attribute: old_value}) - after_member = helpers.MockMember(**{attribute: new_value}) + before_member = helpers.MockMember(**{attribute: old_value}, guild=self.guild_id) + after_member = helpers.MockMember(**{attribute: new_value}, guild=self.guild_id) await self.cog.on_member_update(before_member, after_member) self.cog.patch_user.assert_not_called() + async def test_sync_cog_on_member_update_ignores_guilds(self): + """Events from other guilds should be ignored.""" + member = helpers.MockMember(guild=0) + await self.cog.on_member_update(member, member) + self.cog.patch_user.assert_not_awaited() + async def test_sync_cog_on_user_update(self): """A user should be patched only if the name, discriminator, or avatar changes.""" self.assertTrue(self.cog.on_user_update.__cog_listener__) @@ -290,6 +326,7 @@ class SyncCogListenerTests(SyncCogTestCase): member = helpers.MockMember( discriminator="1234", roles=[helpers.MockRole(id=22), helpers.MockRole(id=12)], + guild=self.guild_id, ) data = { @@ -334,6 +371,13 @@ class SyncCogListenerTests(SyncCogTestCase): self.bot.api_client.post.assert_not_called() + async def test_sync_cog_on_member_join_ignores_guilds(self): + """Events from other guilds should be ignored.""" + member = helpers.MockMember(guild=0) + await self.cog.on_member_join(member) + self.bot.api_client.post.assert_not_awaited() + self.bot.api_client.put.assert_not_awaited() + class SyncCogCommandTests(SyncCogTestCase, CommandTestCase): """Tests for the commands in the Sync cog.""" -- cgit v1.2.3 From 4d6acdf32a323de8b88fed464358d70faf35c9d1 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 14 Jun 2020 23:47:40 -0700 Subject: Sync: ignore 404s in on_user_update 404s probably mean the user is from another guild. --- bot/cogs/sync/cog.py | 14 ++++++++------ tests/bot/cogs/sync/test_cog.py | 17 ++++++++++------- 2 files changed, 18 insertions(+), 13 deletions(-) (limited to 'tests') diff --git a/bot/cogs/sync/cog.py b/bot/cogs/sync/cog.py index 97ea31ba5..578cccfc9 100644 --- a/bot/cogs/sync/cog.py +++ b/bot/cogs/sync/cog.py @@ -34,14 +34,15 @@ class Sync(Cog): for syncer in (self.role_syncer, self.user_syncer): await syncer.sync(guild) - async def patch_user(self, user_id: int, updated_information: Dict[str, Any]) -> None: + async def patch_user(self, user_id: int, json: Dict[str, Any], ignore_404: bool = False) -> None: """Send a PATCH request to partially update a user in the database.""" try: - await self.bot.api_client.patch(f"bot/users/{user_id}", json=updated_information) + await self.bot.api_client.patch(f"bot/users/{user_id}", json=json) except ResponseCodeError as e: if e.response.status != 404: raise - log.warning("Unable to update user, got 404. Assuming race condition from join event.") + if not ignore_404: + log.warning("Unable to update user, got 404. Assuming race condition from join event.") @Cog.listener() async def on_guild_role_create(self, role: Role) -> None: @@ -137,7 +138,7 @@ class Sync(Cog): if member.guild != constants.Guild.id: return - await self.patch_user(member.id, updated_information={"in_guild": False}) + await self.patch_user(member.id, json={"in_guild": False}) @Cog.listener() async def on_member_update(self, before: Member, after: Member) -> None: @@ -147,7 +148,7 @@ class Sync(Cog): if before.roles != after.roles: updated_information = {"roles": sorted(role.id for role in after.roles)} - await self.patch_user(after.id, updated_information=updated_information) + await self.patch_user(after.id, json=updated_information) @Cog.listener() async def on_user_update(self, before: User, after: User) -> None: @@ -158,7 +159,8 @@ class Sync(Cog): "name": after.name, "discriminator": int(after.discriminator), } - await self.patch_user(after.id, updated_information=updated_information) + # A 404 likely means the user is in another guild. + await self.patch_user(after.id, json=updated_information, ignore_404=True) @commands.group(name='sync') @commands.has_permissions(administrator=True) diff --git a/tests/bot/cogs/sync/test_cog.py b/tests/bot/cogs/sync/test_cog.py index d7d60e961..e5be14391 100644 --- a/tests/bot/cogs/sync/test_cog.py +++ b/tests/bot/cogs/sync/test_cog.py @@ -226,7 +226,7 @@ class SyncCogListenerTests(SyncCogTestCase): self.cog.patch_user.assert_called_once_with( member.id, - updated_information={"in_guild": False} + json={"in_guild": False} ) async def test_sync_cog_on_member_remove_ignores_guilds(self): @@ -247,7 +247,7 @@ class SyncCogListenerTests(SyncCogTestCase): await self.cog.on_member_update(before_member, after_member) data = {"roles": sorted(role.id for role in after_member.roles)} - self.cog.patch_user.assert_called_once_with(after_member.id, updated_information=data) + self.cog.patch_user.assert_called_once_with(after_member.id, json=data) async def test_sync_cog_on_member_update_other(self): """Members should not be patched if other attributes have changed.""" @@ -308,12 +308,15 @@ class SyncCogListenerTests(SyncCogTestCase): # Don't care if *all* keys are present; only the changed one is required call_args = self.cog.patch_user.call_args - self.assertEqual(call_args[0][0], after_user.id) - self.assertIn("updated_information", call_args[1]) + self.assertEqual(call_args.args[0], after_user.id) + self.assertIn("json", call_args.kwargs) - updated_information = call_args[1]["updated_information"] - self.assertIn(api_field, updated_information) - self.assertEqual(updated_information[api_field], api_value) + self.assertIn("ignore_404", call_args.kwargs) + self.assertTrue(call_args.kwargs["ignore_404"]) + + json = call_args.kwargs["json"] + self.assertIn(api_field, json) + self.assertEqual(json[api_field], api_value) else: self.cog.patch_user.assert_not_called() -- cgit v1.2.3 From c7373fa1143a2d2f2d784a59d40bcb40ee765bfb Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 15 Jun 2020 10:26:23 -0700 Subject: Token remover: ignore DMs It's a private channel so there's no risk of a token "leaking". Furthermore, messages cannot be deleted in DMs. --- bot/cogs/token_remover.py | 3 +++ tests/bot/cogs/test_token_remover.py | 10 ++++++++++ 2 files changed, 13 insertions(+) (limited to 'tests') diff --git a/bot/cogs/token_remover.py b/bot/cogs/token_remover.py index d55e079e9..493479df9 100644 --- a/bot/cogs/token_remover.py +++ b/bot/cogs/token_remover.py @@ -63,6 +63,9 @@ class TokenRemover(Cog): See: https://discordapp.com/developers/docs/reference#snowflakes """ + if not msg.guild: + return # Ignore DMs; can't delete messages in there anyway. + found_token = self.find_token_in_message(msg) if found_token: await self.take_action(msg, found_token) diff --git a/tests/bot/cogs/test_token_remover.py b/tests/bot/cogs/test_token_remover.py index a10124d2d..22c31d7b1 100644 --- a/tests/bot/cogs/test_token_remover.py +++ b/tests/bot/cogs/test_token_remover.py @@ -121,6 +121,16 @@ class TokenRemoverTests(unittest.IsolatedAsyncioTestCase): find_token_in_message.assert_called_once_with(self.msg) take_action.assert_not_awaited() + @autospec(TokenRemover, "find_token_in_message") + async def test_on_message_ignores_dms(self, find_token_in_message): + """Shouldn't parse a message if it is a DM.""" + cog = TokenRemover(self.bot) + self.msg.guild = None + + await cog.on_message(self.msg) + + find_token_in_message.assert_not_called() + @autospec("bot.cogs.token_remover", "TOKEN_RE") def test_find_token_ignores_bot_messages(self, token_re): """The token finder should ignore messages authored by bots.""" -- cgit v1.2.3 From 2fa7429327e787a65803c16609da21463723bfeb Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 15 Jun 2020 10:38:46 -0700 Subject: Token remover: move bot check to on_message It just makes more sense to me to filter out messages at an earlier stage. --- bot/cogs/token_remover.py | 8 +++----- tests/bot/cogs/test_token_remover.py | 23 +++++++---------------- 2 files changed, 10 insertions(+), 21 deletions(-) (limited to 'tests') diff --git a/bot/cogs/token_remover.py b/bot/cogs/token_remover.py index 493479df9..1f7517501 100644 --- a/bot/cogs/token_remover.py +++ b/bot/cogs/token_remover.py @@ -63,8 +63,9 @@ class TokenRemover(Cog): See: https://discordapp.com/developers/docs/reference#snowflakes """ - if not msg.guild: - return # Ignore DMs; can't delete messages in there anyway. + # Ignore DMs; can't delete messages in there anyway. + if not msg.guild or msg.author.bot: + return found_token = self.find_token_in_message(msg) if found_token: @@ -115,9 +116,6 @@ class TokenRemover(Cog): @classmethod def find_token_in_message(cls, msg: Message) -> t.Optional[Token]: """Return a seemingly valid token found in `msg` or `None` if no token is found.""" - if msg.author.bot: - return - # Use finditer rather than search to guard against method calls prematurely returning the # token check (e.g. `message.channel.send` also matches our token pattern) for match in TOKEN_RE.finditer(msg.content): diff --git a/tests/bot/cogs/test_token_remover.py b/tests/bot/cogs/test_token_remover.py index 22c31d7b1..98ea9f823 100644 --- a/tests/bot/cogs/test_token_remover.py +++ b/tests/bot/cogs/test_token_remover.py @@ -122,24 +122,15 @@ class TokenRemoverTests(unittest.IsolatedAsyncioTestCase): take_action.assert_not_awaited() @autospec(TokenRemover, "find_token_in_message") - async def test_on_message_ignores_dms(self, find_token_in_message): - """Shouldn't parse a message if it is a DM.""" + async def test_on_message_ignores_dms_bots(self, find_token_in_message): + """Shouldn't parse a message if it is a DM or authored by a bot.""" cog = TokenRemover(self.bot) - self.msg.guild = None + dm_msg = MockMessage(guild=None) + bot_msg = MockMessage(author=MagicMock(bot=True)) - await cog.on_message(self.msg) - - find_token_in_message.assert_not_called() - - @autospec("bot.cogs.token_remover", "TOKEN_RE") - def test_find_token_ignores_bot_messages(self, token_re): - """The token finder should ignore messages authored by bots.""" - self.msg.author.bot = True - - return_value = TokenRemover.find_token_in_message(self.msg) - - self.assertIsNone(return_value) - token_re.finditer.assert_not_called() + for msg in (dm_msg, bot_msg): + await cog.on_message(msg) + find_token_in_message.assert_not_called() @autospec("bot.cogs.token_remover", "TOKEN_RE") def test_find_token_no_matches(self, token_re): -- cgit v1.2.3 From 3aecf14419c87e533d47fe082abeb54ca9edb73c Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 15 Jun 2020 10:49:18 -0700 Subject: Token remover: exit early if message already deleted --- bot/cogs/token_remover.py | 10 ++++++++-- tests/bot/cogs/test_token_remover.py | 15 ++++++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/bot/cogs/token_remover.py b/bot/cogs/token_remover.py index 1f7517501..ef979f222 100644 --- a/bot/cogs/token_remover.py +++ b/bot/cogs/token_remover.py @@ -4,7 +4,7 @@ import logging import re import typing as t -from discord import Colour, Message +from discord import Colour, Message, NotFound from discord.ext.commands import Cog from bot import utils @@ -83,7 +83,13 @@ class TokenRemover(Cog): async def take_action(self, msg: Message, found_token: Token) -> None: """Remove the `msg` containing the `found_token` and send a mod log message.""" self.mod_log.ignore(Event.message_delete, msg.id) - await msg.delete() + + try: + await msg.delete() + except NotFound: + log.debug(f"Failed to remove token in message {msg.id}: message already deleted.") + return + await msg.channel.send(DELETION_MESSAGE_TEMPLATE.format(mention=msg.author.mention)) log_message = self.format_log_message(msg, found_token) diff --git a/tests/bot/cogs/test_token_remover.py b/tests/bot/cogs/test_token_remover.py index 98ea9f823..3349caa73 100644 --- a/tests/bot/cogs/test_token_remover.py +++ b/tests/bot/cogs/test_token_remover.py @@ -3,7 +3,7 @@ from re import Match from unittest import mock from unittest.mock import MagicMock -from discord import Colour +from discord import Colour, NotFound from bot import constants from bot.cogs import token_remover @@ -282,6 +282,19 @@ class TokenRemoverTests(unittest.IsolatedAsyncioTestCase): channel_id=constants.Channels.mod_alerts ) + @mock.patch.object(TokenRemover, "mod_log", new_callable=mock.PropertyMock) + async def test_take_action_delete_failure(self, mod_log_property): + """Shouldn't send any messages if the token message can't be deleted.""" + cog = TokenRemover(self.bot) + mod_log_property.return_value = mock.create_autospec(ModLog, spec_set=True, instance=True) + self.msg.delete.side_effect = NotFound(MagicMock(), MagicMock()) + + token = mock.create_autospec(Token, spec_set=True, instance=True) + await cog.take_action(self.msg, token) + + self.msg.delete.assert_called_once_with() + self.msg.channel.send.assert_not_awaited() + class TokenRemoverExtensionTests(unittest.TestCase): """Tests for the token_remover extension.""" -- cgit v1.2.3 From ebc0eae42c1da67f61f040a67bc1b70e53a6f97e Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Wed, 17 Jun 2020 16:46:43 -0700 Subject: Sync: fix guild ID check Need to compare the IDs against each other rather than the Guild object against the ID. --- bot/cogs/sync/cog.py | 12 ++++++------ tests/bot/cogs/sync/test_cog.py | 35 +++++++++++++++++++---------------- 2 files changed, 25 insertions(+), 22 deletions(-) (limited to 'tests') diff --git a/bot/cogs/sync/cog.py b/bot/cogs/sync/cog.py index 578cccfc9..5ace957e7 100644 --- a/bot/cogs/sync/cog.py +++ b/bot/cogs/sync/cog.py @@ -47,7 +47,7 @@ class Sync(Cog): @Cog.listener() async def on_guild_role_create(self, role: Role) -> None: """Adds newly create role to the database table over the API.""" - if role.guild != constants.Guild.id: + if role.guild.id != constants.Guild.id: return await self.bot.api_client.post( @@ -64,7 +64,7 @@ class Sync(Cog): @Cog.listener() async def on_guild_role_delete(self, role: Role) -> None: """Deletes role from the database when it's deleted from the guild.""" - if role.guild != constants.Guild.id: + if role.guild.id != constants.Guild.id: return await self.bot.api_client.delete(f'bot/roles/{role.id}') @@ -72,7 +72,7 @@ class Sync(Cog): @Cog.listener() async def on_guild_role_update(self, before: Role, after: Role) -> None: """Syncs role with the database if any of the stored attributes were updated.""" - if after.guild != constants.Guild.id: + if after.guild.id != constants.Guild.id: return was_updated = ( @@ -103,7 +103,7 @@ class Sync(Cog): previously left), it will update the user's information. If the user is not yet known by the database, the user is added. """ - if member.guild != constants.Guild.id: + if member.guild.id != constants.Guild.id: return packed = { @@ -135,7 +135,7 @@ class Sync(Cog): @Cog.listener() async def on_member_remove(self, member: Member) -> None: """Set the in_guild field to False when a member leaves the guild.""" - if member.guild != constants.Guild.id: + if member.guild.id != constants.Guild.id: return await self.patch_user(member.id, json={"in_guild": False}) @@ -143,7 +143,7 @@ class Sync(Cog): @Cog.listener() async def on_member_update(self, before: Member, after: Member) -> None: """Update the roles of the member in the database if a change is detected.""" - if after.guild != constants.Guild.id: + if after.guild.id != constants.Guild.id: return if before.roles != after.roles: diff --git a/tests/bot/cogs/sync/test_cog.py b/tests/bot/cogs/sync/test_cog.py index e5be14391..120bc991d 100644 --- a/tests/bot/cogs/sync/test_cog.py +++ b/tests/bot/cogs/sync/test_cog.py @@ -134,6 +134,9 @@ class SyncCogListenerTests(SyncCogTestCase): self.guild_id_patcher = mock.patch("bot.cogs.sync.cog.constants.Guild.id", 5) self.guild_id = self.guild_id_patcher.start() + self.guild = helpers.MockGuild(id=self.guild_id) + self.other_guild = helpers.MockGuild(id=0) + def tearDown(self): self.guild_id_patcher.stop() @@ -148,14 +151,14 @@ class SyncCogListenerTests(SyncCogTestCase): "permissions": 8, "position": 23, } - role = helpers.MockRole(**role_data, guild=self.guild_id) + role = helpers.MockRole(**role_data, guild=self.guild) await self.cog.on_guild_role_create(role) self.bot.api_client.post.assert_called_once_with("bot/roles", json=role_data) async def test_sync_cog_on_guild_role_create_ignores_guilds(self): """Events from other guilds should be ignored.""" - role = helpers.MockRole(guild=0) + role = helpers.MockRole(guild=self.other_guild) await self.cog.on_guild_role_create(role) self.bot.api_client.post.assert_not_awaited() @@ -163,14 +166,14 @@ class SyncCogListenerTests(SyncCogTestCase): """A DELETE request should be sent.""" self.assertTrue(self.cog.on_guild_role_delete.__cog_listener__) - role = helpers.MockRole(id=99, guild=self.guild_id) + role = helpers.MockRole(id=99, guild=self.guild) await self.cog.on_guild_role_delete(role) self.bot.api_client.delete.assert_called_once_with("bot/roles/99") async def test_sync_cog_on_guild_role_delete_ignores_guilds(self): """Events from other guilds should be ignored.""" - role = helpers.MockRole(guild=0) + role = helpers.MockRole(guild=self.other_guild) await self.cog.on_guild_role_delete(role) self.bot.api_client.delete.assert_not_awaited() @@ -198,8 +201,8 @@ class SyncCogListenerTests(SyncCogTestCase): after_role_data = role_data.copy() after_role_data[attribute] = 876 - before_role = helpers.MockRole(**role_data, guild=self.guild_id) - after_role = helpers.MockRole(**after_role_data, guild=self.guild_id) + before_role = helpers.MockRole(**role_data, guild=self.guild) + after_role = helpers.MockRole(**after_role_data, guild=self.guild) await self.cog.on_guild_role_update(before_role, after_role) @@ -213,7 +216,7 @@ class SyncCogListenerTests(SyncCogTestCase): async def test_sync_cog_on_guild_role_update_ignores_guilds(self): """Events from other guilds should be ignored.""" - role = helpers.MockRole(guild=0) + role = helpers.MockRole(guild=self.other_guild) await self.cog.on_guild_role_update(role, role) self.bot.api_client.put.assert_not_awaited() @@ -221,7 +224,7 @@ class SyncCogListenerTests(SyncCogTestCase): """Member should be patched to set in_guild as False.""" self.assertTrue(self.cog.on_member_remove.__cog_listener__) - member = helpers.MockMember(guild=self.guild_id) + member = helpers.MockMember(guild=self.guild) await self.cog.on_member_remove(member) self.cog.patch_user.assert_called_once_with( @@ -231,7 +234,7 @@ class SyncCogListenerTests(SyncCogTestCase): async def test_sync_cog_on_member_remove_ignores_guilds(self): """Events from other guilds should be ignored.""" - member = helpers.MockMember(guild=0) + member = helpers.MockMember(guild=self.other_guild) await self.cog.on_member_remove(member) self.cog.patch_user.assert_not_awaited() @@ -241,8 +244,8 @@ class SyncCogListenerTests(SyncCogTestCase): # Roles are intentionally unsorted. before_roles = [helpers.MockRole(id=12), helpers.MockRole(id=30), helpers.MockRole(id=20)] - before_member = helpers.MockMember(roles=before_roles, guild=self.guild_id) - after_member = helpers.MockMember(roles=before_roles[1:], guild=self.guild_id) + before_member = helpers.MockMember(roles=before_roles, guild=self.guild) + after_member = helpers.MockMember(roles=before_roles[1:], guild=self.guild) await self.cog.on_member_update(before_member, after_member) @@ -263,8 +266,8 @@ class SyncCogListenerTests(SyncCogTestCase): with self.subTest(attribute=attribute): self.cog.patch_user.reset_mock() - before_member = helpers.MockMember(**{attribute: old_value}, guild=self.guild_id) - after_member = helpers.MockMember(**{attribute: new_value}, guild=self.guild_id) + before_member = helpers.MockMember(**{attribute: old_value}, guild=self.guild) + after_member = helpers.MockMember(**{attribute: new_value}, guild=self.guild) await self.cog.on_member_update(before_member, after_member) @@ -272,7 +275,7 @@ class SyncCogListenerTests(SyncCogTestCase): async def test_sync_cog_on_member_update_ignores_guilds(self): """Events from other guilds should be ignored.""" - member = helpers.MockMember(guild=0) + member = helpers.MockMember(guild=self.other_guild) await self.cog.on_member_update(member, member) self.cog.patch_user.assert_not_awaited() @@ -329,7 +332,7 @@ class SyncCogListenerTests(SyncCogTestCase): member = helpers.MockMember( discriminator="1234", roles=[helpers.MockRole(id=22), helpers.MockRole(id=12)], - guild=self.guild_id, + guild=self.guild, ) data = { @@ -376,7 +379,7 @@ class SyncCogListenerTests(SyncCogTestCase): async def test_sync_cog_on_member_join_ignores_guilds(self): """Events from other guilds should be ignored.""" - member = helpers.MockMember(guild=0) + member = helpers.MockMember(guild=self.other_guild) await self.cog.on_member_join(member) self.bot.api_client.post.assert_not_awaited() self.bot.api_client.put.assert_not_awaited() -- cgit v1.2.3 From 581573f2ece96a9ec666795431ff21068e949a63 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 20 Jun 2020 01:20:35 +0200 Subject: Write unit test for `sub_clyde` --- tests/bot/utils/test_messages.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 tests/bot/utils/test_messages.py (limited to 'tests') diff --git a/tests/bot/utils/test_messages.py b/tests/bot/utils/test_messages.py new file mode 100644 index 000000000..9c22c9751 --- /dev/null +++ b/tests/bot/utils/test_messages.py @@ -0,0 +1,27 @@ +import unittest + +from bot.utils import messages + + +class TestMessages(unittest.TestCase): + """Tests for functions in the `bot.utils.messages` module.""" + + def test_sub_clyde(self): + """Uppercase E's and lowercase e's are substituted with their cyrillic counterparts.""" + sub_e = "\u0435" + sub_E = "\u0415" # noqa: N806: Uppercase E in variable name + + test_cases = ( + (None, None), + ("", ""), + ("clyde", f"clyd{sub_e}"), + ("CLYDE", f"CLYD{sub_E}"), + ("cLyDe", f"cLyD{sub_e}"), + ("BIGclyde", f"BIGclyd{sub_e}"), + ("small clydeus the unholy", f"small clyd{sub_e}us the unholy"), + ("BIGCLYDE, babyclyde", f"BIGCLYD{sub_E}, babyclyd{sub_e}"), + ) + + for username_in, username_out in test_cases: + with self.subTest(input=username_in, expected_output=username_out): + self.assertEqual(messages.sub_clyde(username_in), username_out) -- cgit v1.2.3