From 7f1d319a11de5e443307517ff1fd55fe87a69bb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leon=20Sand=C3=B8y?= Date: Sun, 27 Oct 2019 02:50:20 +0200 Subject: [PATCH 01/26] Add duck-pond constants. This adds the emojis, the channel, and the configuration needed for the duck-pond feature. This is added both to config-default.yml, and to the constants.py file. --- bot/constants.py | 57 +++++++++++++++++++++++++++------------------- config-default.yml | 15 ++++++++---- 2 files changed, 43 insertions(+), 29 deletions(-) diff --git a/bot/constants.py b/bot/constants.py index 838fe7a79e..ed1e65cca9 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -236,6 +236,14 @@ class Colours(metaclass=YAMLGetter): soft_orange: int +class DuckPond(metaclass=YAMLGetter): + section = "duck_pond" + + ducks_required: int + duck_custom_emojis: List[int] + duck_pond_channel: int + + class Emojis(metaclass=YAMLGetter): section = "style" subsection = "emojis" @@ -370,6 +378,7 @@ class Webhooks(metaclass=YAMLGetter): talent_pool: int big_brother: int reddit: int + duck_pond: int class Roles(metaclass=YAMLGetter): @@ -501,6 +510,30 @@ class RedirectOutput(metaclass=YAMLGetter): delete_delay: int +class Event(Enum): + """ + Event names. This does not include every event (for example, raw + events aren't here), but only events used in ModLog for now. + """ + + guild_channel_create = "guild_channel_create" + guild_channel_delete = "guild_channel_delete" + guild_channel_update = "guild_channel_update" + guild_role_create = "guild_role_create" + guild_role_delete = "guild_role_delete" + guild_role_update = "guild_role_update" + guild_update = "guild_update" + + member_join = "member_join" + member_remove = "member_remove" + member_ban = "member_ban" + member_unban = "member_unban" + member_update = "member_update" + + message_delete = "message_delete" + message_edit = "message_edit" + + # Debug mode DEBUG_MODE = True if 'local' in os.environ.get("SITE_URL", "local") else False @@ -572,27 +605,3 @@ class RedirectOutput(metaclass=YAMLGetter): "Noooooo!!", "I can't believe you've done this", ] - - -class Event(Enum): - """ - Event names. This does not include every event (for example, raw - events aren't here), but only events used in ModLog for now. - """ - - guild_channel_create = "guild_channel_create" - guild_channel_delete = "guild_channel_delete" - guild_channel_update = "guild_channel_update" - guild_role_create = "guild_role_create" - guild_role_delete = "guild_role_delete" - guild_role_update = "guild_role_update" - guild_update = "guild_update" - - member_join = "member_join" - member_remove = "member_remove" - member_ban = "member_ban" - member_unban = "member_unban" - member_update = "member_update" - - message_delete = "message_delete" - message_edit = "message_edit" diff --git a/config-default.yml b/config-default.yml index 4638a89eeb..bad9c72db3 100644 --- a/config-default.yml +++ b/config-default.yml @@ -22,11 +22,6 @@ style: defcon_enabled: "<:defconenabled:470326274213150730>" defcon_updated: "<:defconsettingsupdated:470326274082996224>" - green_chevron: "<:greenchevron:418104310329769993>" - red_chevron: "<:redchevron:418112778184818698>" - white_chevron: "<:whitechevron:418110396973711363>" - bb_message: "<:bbmessage:476273120999636992>" - status_online: "<:status_online:470326272351010816>" status_idle: "<:status_idle:470326266625785866>" status_dnd: "<:status_dnd:470326272082313216>" @@ -37,6 +32,9 @@ style: new: "\U0001F195" cross_mark: "\u274C" + ducky: &DUCKY_EMOJI 574951975574175744 + ducky_blurple: &DUCKY_BLURPLE_EMOJI 574951975310065675 + icons: crown_blurple: "https://cdn.discordapp.com/emojis/469964153289965568.png" crown_green: "https://cdn.discordapp.com/emojis/469964154719961088.png" @@ -98,6 +96,7 @@ guild: defcon: &DEFCON 464469101889454091 devlog: &DEVLOG 622895325144940554 devtest: &DEVTEST 414574275865870337 + duck_pond: &DUCK_POND 000000000000000000 help_0: 303906576991780866 help_1: 303906556754395136 help_2: 303906514266226689 @@ -148,6 +147,7 @@ guild: talent_pool: 569145364800602132 big_brother: 569133704568373283 reddit: 635408384794951680 + duck_pond: 637779355304722435 filter: @@ -382,5 +382,10 @@ redirect_output: delete_invocation: true delete_delay: 15 +duck_pond: + ducks_required: 5 + duck_custom_emojis: [*DUCKY_EMOJI, *DUCKY_BLURPLE_EMOJI] + duck_pond_channel: *DUCK_POND + config: required_keys: ['bot.token'] From 957f46226a6c9cbc9e86bab8a4365665d479885f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leon=20Sand=C3=B8y?= Date: Sun, 27 Oct 2019 02:56:20 +0200 Subject: [PATCH 02/26] Add duck_pond cog. This cog will listen for duck reactions on any message, and then: - If the reaction was added by a staff member - and the reaction was a duck - and the message has not already been added to the #duck-pond It will add the message to the #duck-pond and then add a green checkbox to the original message to indicate that the message has been ponded. Messages are added to the #duck-pond via webhook, so that they can retain the appearance of having their original authors. Once this checkmark has been added, the message will not be processed in the future. If the checkmark is removed and there are more than ducks_required ducks on the message, the bot will automatically add the checkmark back. However, if all reactions are removed, the bot does not have a countermeasure for this. In order to implement a countermeasure, it would be necessary to involve the API and the database. --- bot/__main__.py | 1 + bot/cogs/duck_pond.py | 206 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 207 insertions(+) create mode 100644 bot/cogs/duck_pond.py diff --git a/bot/__main__.py b/bot/__main__.py index f352cd60e9..ea7c43a12a 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -55,6 +55,7 @@ bot.load_extension("bot.cogs.alias") bot.load_extension("bot.cogs.defcon") bot.load_extension("bot.cogs.eval") +bot.load_extension("bot.cogs.duck_pond") bot.load_extension("bot.cogs.free") bot.load_extension("bot.cogs.information") bot.load_extension("bot.cogs.jams") diff --git a/bot/cogs/duck_pond.py b/bot/cogs/duck_pond.py new file mode 100644 index 0000000000..d5d528458c --- /dev/null +++ b/bot/cogs/duck_pond.py @@ -0,0 +1,206 @@ +import logging +from typing import List, Optional, Union + +import discord +from discord import Color, Embed, Member, Message, PartialEmoji, RawReactionActionEvent, Reaction, User, errors +from discord.ext.commands import Bot, Cog + +import bot.constants as constants +from bot.utils.messages import send_attachments + +log = logging.getLogger(__name__) + + +class DuckPond(Cog): + """Relays messages to #duck-pond whenever a certain number of duck reactions have been achieved.""" + + def __init__(self, bot: Bot): + self.bot = bot + self.log = log + self.webhook_id = constants.Webhooks.duck_pond + self.bot.loop.create_task(self.fetch_webhook()) + + async def fetch_webhook(self): + """Fetches the webhook object, so we can post to it.""" + await self.bot.wait_until_ready() + + try: + self.webhook = await self.bot.fetch_webhook(self.webhook_id) + except discord.HTTPException: + self.log.exception(f"Failed to fetch webhook with id `{self.webhook_id}`") + + @staticmethod + def is_staff(member: Union[User, Member]) -> bool: + """Check if a specific member or user is staff""" + if hasattr(member, "roles"): + for role in member.roles: + if role.id in constants.STAFF_ROLES: + return True + return False + + @staticmethod + def has_green_checkmark(message: Optional[Message] = None, reaction_list: Optional[List[Reaction]] = None) -> bool: + """Check if the message has a green checkmark reaction.""" + assert message or reaction_list, "You can either pass message or reactions, but not both, or neither." + + if message: + reactions = message.reactions + else: + reactions = reaction_list + + for reaction in reactions: + if isinstance(reaction.emoji, str): + if reaction.emoji == "✅": + return True + elif isinstance(reaction.emoji, PartialEmoji): + if reaction.emoji.name == "✅": + return True + return False + + async def send_webhook( + self, + content: Optional[str] = None, + username: Optional[str] = None, + avatar_url: Optional[str] = None, + embed: Optional[Embed] = None, + ) -> None: + try: + await self.webhook.send( + content=content, + username=username, + avatar_url=avatar_url, + embed=embed + ) + except discord.HTTPException as exc: + self.log.exception( + f"Failed to send a message to the Duck Pool webhook", + exc_info=exc + ) + + async def count_ducks(self, message: Optional[Message] = None, reaction_list: Optional[List[Reaction]] = None) -> int: + """Count the number of ducks in the reactions of a specific message. + + Only counts ducks added by staff members. + """ + assert message or reaction_list, "You can either pass message or reactions, but not both, or neither." + + duck_count = 0 + duck_reactors = [] + + if message: + reactions = message.reactions + else: + reactions = reaction_list + + for reaction in reactions: + async for user in reaction.users(): + + # Is the user or member a staff member? + if self.is_staff(user) and user.id not in duck_reactors: + + # Is the emoji a duck? + if hasattr(reaction.emoji, "id"): + if reaction.emoji.id in constants.DuckPond.duck_custom_emojis: + duck_count += 1 + duck_reactors.append(user.id) + else: + if isinstance(reaction.emoji, str): + if reaction.emoji == "🦆": + duck_count += 1 + duck_reactors.append(user.id) + elif isinstance(reaction.emoji, PartialEmoji): + if reaction.emoji.name == "🦆": + duck_count += 1 + duck_reactors.append(user.id) + return duck_count + + @Cog.listener() + async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None: + """Determine if a message should be sent to the duck pond. + + This will count the number of duck reactions on the message, and if this amount meets the + amount of ducks specified in the config under duck_pond/ducks_required, it will + send the message off to the duck pond. + """ + channel = discord.utils.get(self.bot.get_all_channels(), id=payload.channel_id) + message = await channel.fetch_message(payload.message_id) + member = discord.utils.get(message.guild.members, id=payload.user_id) + + # Is the member a staff member? + if not self.is_staff(member): + return + + # Bot reactions don't count. + if member.bot: + return + + # Is the emoji in the reaction a duck? + if payload.emoji.is_custom_emoji(): + if payload.emoji.id not in constants.DuckPond.duck_custom_emojis: + return + else: + if payload.emoji.name != "🦆": + return + + # Does the message already have a green checkmark? + if self.has_green_checkmark(message): + return + + # Time to count our ducks! + duck_count = await self.count_ducks(message) + + # If we've got more than the required amount of ducks, send the message to the duck_pond. + if duck_count >= constants.DuckPond.ducks_required: + clean_content = message.clean_content + + if clean_content: + await self.send_webhook( + content=message.clean_content, + username=message.author.display_name, + avatar_url=message.author.avatar_url + ) + + if message.attachments: + try: + await send_attachments(message, self.webhook) + except (errors.Forbidden, errors.NotFound): + e = Embed( + description=":x: **This message contained an attachment, but it could not be retrieved**", + color=Color.red() + ) + await self.send_webhook( + embed=e, + username=message.author.display_name, + avatar_url=message.author.avatar_url + ) + except discord.HTTPException as exc: + self.log.exception( + f"Failed to send an attachment to the webhook", + exc_info=exc + ) + await message.add_reaction("✅") + + @Cog.listener() + async def on_raw_reaction_remove(self, payload: RawReactionActionEvent) -> None: + """Ensure that people don't remove the green checkmark from duck ponded messages.""" + channel = discord.utils.get(self.bot.get_all_channels(), id=payload.channel_id) + message = await channel.fetch_message(payload.message_id) + + # Prevent the green checkmark from being removed + if isinstance(payload.emoji, str): + if payload.emoji == "✅": + duck_count = await self.count_ducks(message) + if duck_count >= constants.DuckPond.ducks_required: + await message.add_reaction("✅") + + elif isinstance(payload.emoji, PartialEmoji): + if payload.emoji.name == "✅": + duck_count = await self.count_ducks(message) + if duck_count >= constants.DuckPond.ducks_required: + await message.add_reaction("✅") + + +def setup(bot: Bot) -> None: + """Token Remover cog load.""" + bot.add_cog(DuckPond(bot)) + log.info("Cog loaded: DuckPond") From 38579ade38a7390e9aca428410a9703dd7ba9fac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leon=20Sand=C3=B8y?= Date: Sun, 27 Oct 2019 02:07:55 +0100 Subject: [PATCH 03/26] Appease the linter --- bot/cogs/duck_pond.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/bot/cogs/duck_pond.py b/bot/cogs/duck_pond.py index d5d528458c..b2b786a3f2 100644 --- a/bot/cogs/duck_pond.py +++ b/bot/cogs/duck_pond.py @@ -5,7 +5,7 @@ from discord import Color, Embed, Member, Message, PartialEmoji, RawReactionActionEvent, Reaction, User, errors from discord.ext.commands import Bot, Cog -import bot.constants as constants +from bot import constants from bot.utils.messages import send_attachments log = logging.getLogger(__name__) @@ -20,7 +20,7 @@ def __init__(self, bot: Bot): self.webhook_id = constants.Webhooks.duck_pond self.bot.loop.create_task(self.fetch_webhook()) - async def fetch_webhook(self): + async def fetch_webhook(self) -> None: """Fetches the webhook object, so we can post to it.""" await self.bot.wait_until_ready() @@ -31,7 +31,7 @@ async def fetch_webhook(self): @staticmethod def is_staff(member: Union[User, Member]) -> bool: - """Check if a specific member or user is staff""" + """Check if a specific member or user is staff.""" if hasattr(member, "roles"): for role in member.roles: if role.id in constants.STAFF_ROLES: @@ -64,6 +64,7 @@ async def send_webhook( avatar_url: Optional[str] = None, embed: Optional[Embed] = None, ) -> None: + """Send a webhook to the duck_pond channel.""" try: await self.webhook.send( content=content, @@ -77,8 +78,13 @@ async def send_webhook( exc_info=exc ) - async def count_ducks(self, message: Optional[Message] = None, reaction_list: Optional[List[Reaction]] = None) -> int: - """Count the number of ducks in the reactions of a specific message. + async def count_ducks( + self, + message: Optional[Message] = None, + reaction_list: Optional[List[Reaction]] = None + ) -> int: + """ + Count the number of ducks in the reactions of a specific message. Only counts ducks added by staff members. """ @@ -116,7 +122,8 @@ async def count_ducks(self, message: Optional[Message] = None, reaction_list: Op @Cog.listener() async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None: - """Determine if a message should be sent to the duck pond. + """ + Determine if a message should be sent to the duck pond. This will count the number of duck reactions on the message, and if this amount meets the amount of ducks specified in the config under duck_pond/ducks_required, it will From 0957bee71fafc575c99ac107f941e9bfe6a72397 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leon=20Sand=C3=B8y?= Date: Sun, 27 Oct 2019 02:16:41 +0100 Subject: [PATCH 04/26] Add correct values for constants from production server. --- config-default.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config-default.yml b/config-default.yml index bad9c72db3..074143f928 100644 --- a/config-default.yml +++ b/config-default.yml @@ -96,7 +96,7 @@ guild: defcon: &DEFCON 464469101889454091 devlog: &DEVLOG 622895325144940554 devtest: &DEVTEST 414574275865870337 - duck_pond: &DUCK_POND 000000000000000000 + duck_pond: &DUCK_POND 637820308341915648 help_0: 303906576991780866 help_1: 303906556754395136 help_2: 303906514266226689 @@ -147,7 +147,7 @@ guild: talent_pool: 569145364800602132 big_brother: 569133704568373283 reddit: 635408384794951680 - duck_pond: 637779355304722435 + duck_pond: 637821475327311927 filter: From c66cd6f236c9f8c68a39caafdbbba1f5724947a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leon=20Sand=C3=B8y?= Date: Sun, 27 Oct 2019 02:26:52 +0100 Subject: [PATCH 05/26] Fix broken constant tests --- bot/constants.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/bot/constants.py b/bot/constants.py index ed1e65cca9..d626fd4ba9 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -252,11 +252,6 @@ class Emojis(metaclass=YAMLGetter): defcon_enabled: str # noqa: E704 defcon_updated: str # noqa: E704 - green_chevron: str - red_chevron: str - white_chevron: str - bb_message: str - status_online: str status_offline: str status_idle: str @@ -267,6 +262,9 @@ class Emojis(metaclass=YAMLGetter): pencil: str cross_mark: str + ducky: int + ducky_blurple: int + class Icons(metaclass=YAMLGetter): section = "style" @@ -344,6 +342,7 @@ class Channels(metaclass=YAMLGetter): defcon: int devlog: int devtest: int + duck_pond: int help_0: int help_1: int help_2: int From dac975e8bf238b545b60f10cd1891a68f31dc1ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leon=20Sand=C3=B8y?= Date: Sun, 27 Oct 2019 02:28:03 +0100 Subject: [PATCH 06/26] Improve the setup() docstring Co-Authored-By: Mark --- bot/cogs/duck_pond.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/duck_pond.py b/bot/cogs/duck_pond.py index b2b786a3f2..6244bdf5a2 100644 --- a/bot/cogs/duck_pond.py +++ b/bot/cogs/duck_pond.py @@ -208,6 +208,6 @@ async def on_raw_reaction_remove(self, payload: RawReactionActionEvent) -> None: def setup(bot: Bot) -> None: - """Token Remover cog load.""" + """Load the duck pond cog.""" bot.add_cog(DuckPond(bot)) log.info("Cog loaded: DuckPond") From 51622223f4173e35a90d2a306a61e020fc0b422b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leon=20Sand=C3=B8y?= Date: Sun, 27 Oct 2019 15:16:35 +0100 Subject: [PATCH 07/26] Addressing review by Mark. This refactors the duck pond cog to have fewer redundancies, removes some unused features (like supporting reaction_list in the count_duck and has_green_checkbox helpers), and makes other various minor (mostly cosmetic) improvements. --- bot/cogs/duck_pond.py | 120 +++++++++++++----------------------------- bot/constants.py | 5 +- config-default.yml | 6 +-- 3 files changed, 42 insertions(+), 89 deletions(-) diff --git a/bot/cogs/duck_pond.py b/bot/cogs/duck_pond.py index b2b786a3f2..70cf0d2b0c 100644 --- a/bot/cogs/duck_pond.py +++ b/bot/cogs/duck_pond.py @@ -1,8 +1,8 @@ import logging -from typing import List, Optional, Union +from typing import Optional, Union import discord -from discord import Color, Embed, Member, Message, PartialEmoji, RawReactionActionEvent, Reaction, User, errors +from discord import Color, Embed, Member, Message, RawReactionActionEvent, User, errors from discord.ext.commands import Bot, Cog from bot import constants @@ -16,7 +16,6 @@ class DuckPond(Cog): def __init__(self, bot: Bot): self.bot = bot - self.log = log self.webhook_id = constants.Webhooks.duck_pond self.bot.loop.create_task(self.fetch_webhook()) @@ -27,7 +26,7 @@ async def fetch_webhook(self) -> None: try: self.webhook = await self.bot.fetch_webhook(self.webhook_id) except discord.HTTPException: - self.log.exception(f"Failed to fetch webhook with id `{self.webhook_id}`") + log.exception(f"Failed to fetch webhook with id `{self.webhook_id}`") @staticmethod def is_staff(member: Union[User, Member]) -> bool: @@ -39,22 +38,11 @@ def is_staff(member: Union[User, Member]) -> bool: return False @staticmethod - def has_green_checkmark(message: Optional[Message] = None, reaction_list: Optional[List[Reaction]] = None) -> bool: + def has_green_checkmark(message: Message) -> bool: """Check if the message has a green checkmark reaction.""" - assert message or reaction_list, "You can either pass message or reactions, but not both, or neither." - - if message: - reactions = message.reactions - else: - reactions = reaction_list - - for reaction in reactions: - if isinstance(reaction.emoji, str): - if reaction.emoji == "✅": - return True - elif isinstance(reaction.emoji, PartialEmoji): - if reaction.emoji.name == "✅": - return True + for reaction in message.reactions: + if reaction.emoji == "✅": + return True return False async def send_webhook( @@ -72,52 +60,34 @@ async def send_webhook( avatar_url=avatar_url, embed=embed ) - except discord.HTTPException as exc: - self.log.exception( - f"Failed to send a message to the Duck Pool webhook", - exc_info=exc - ) + except discord.HTTPException: + log.exception(f"Failed to send a message to the Duck Pool webhook") - async def count_ducks( - self, - message: Optional[Message] = None, - reaction_list: Optional[List[Reaction]] = None - ) -> int: + async def count_ducks(self, message: Message) -> int: """ Count the number of ducks in the reactions of a specific message. Only counts ducks added by staff members. """ - assert message or reaction_list, "You can either pass message or reactions, but not both, or neither." - duck_count = 0 duck_reactors = [] - if message: - reactions = message.reactions - else: - reactions = reaction_list - - for reaction in reactions: + for reaction in message.reactions: async for user in reaction.users(): # Is the user or member a staff member? - if self.is_staff(user) and user.id not in duck_reactors: - - # Is the emoji a duck? - if hasattr(reaction.emoji, "id"): - if reaction.emoji.id in constants.DuckPond.duck_custom_emojis: - duck_count += 1 - duck_reactors.append(user.id) - else: - if isinstance(reaction.emoji, str): - if reaction.emoji == "🦆": - duck_count += 1 - duck_reactors.append(user.id) - elif isinstance(reaction.emoji, PartialEmoji): - if reaction.emoji.name == "🦆": - duck_count += 1 - duck_reactors.append(user.id) + if not self.is_staff(user) or not user.id not in duck_reactors: + continue + + # Is the emoji a duck? + if hasattr(reaction.emoji, "id"): + if reaction.emoji.id in constants.DuckPond.custom_emojis: + duck_count += 1 + duck_reactors.append(user.id) + elif isinstance(reaction.emoji, str): + if reaction.emoji == "🦆": + duck_count += 1 + duck_reactors.append(user.id) return duck_count @Cog.listener() @@ -126,28 +96,23 @@ async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None: Determine if a message should be sent to the duck pond. This will count the number of duck reactions on the message, and if this amount meets the - amount of ducks specified in the config under duck_pond/ducks_required, it will + amount of ducks specified in the config under duck_pond/threshold, it will send the message off to the duck pond. """ channel = discord.utils.get(self.bot.get_all_channels(), id=payload.channel_id) message = await channel.fetch_message(payload.message_id) member = discord.utils.get(message.guild.members, id=payload.user_id) - # Is the member a staff member? - if not self.is_staff(member): - return - - # Bot reactions don't count. - if member.bot: + # Is the member a human and a staff member? + if not self.is_staff(member) or member.bot: return # Is the emoji in the reaction a duck? if payload.emoji.is_custom_emoji(): - if payload.emoji.id not in constants.DuckPond.duck_custom_emojis: - return - else: - if payload.emoji.name != "🦆": + if payload.emoji.id not in constants.DuckPond.custom_emojis: return + elif payload.emoji.name != "🦆": + return # Does the message already have a green checkmark? if self.has_green_checkmark(message): @@ -157,7 +122,7 @@ async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None: duck_count = await self.count_ducks(message) # If we've got more than the required amount of ducks, send the message to the duck_pond. - if duck_count >= constants.DuckPond.ducks_required: + if duck_count >= constants.DuckPond.threshold: clean_content = message.clean_content if clean_content: @@ -180,31 +145,22 @@ async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None: username=message.author.display_name, avatar_url=message.author.avatar_url ) - except discord.HTTPException as exc: - self.log.exception( - f"Failed to send an attachment to the webhook", - exc_info=exc - ) + except discord.HTTPException: + log.exception(f"Failed to send an attachment to the webhook") + await message.add_reaction("✅") @Cog.listener() async def on_raw_reaction_remove(self, payload: RawReactionActionEvent) -> None: """Ensure that people don't remove the green checkmark from duck ponded messages.""" channel = discord.utils.get(self.bot.get_all_channels(), id=payload.channel_id) - message = await channel.fetch_message(payload.message_id) # Prevent the green checkmark from being removed - if isinstance(payload.emoji, str): - if payload.emoji == "✅": - duck_count = await self.count_ducks(message) - if duck_count >= constants.DuckPond.ducks_required: - await message.add_reaction("✅") - - elif isinstance(payload.emoji, PartialEmoji): - if payload.emoji.name == "✅": - duck_count = await self.count_ducks(message) - if duck_count >= constants.DuckPond.ducks_required: - await message.add_reaction("✅") + if payload.emoji.name == "✅": + message = await channel.fetch_message(payload.message_id) + duck_count = await self.count_ducks(message) + if duck_count >= constants.DuckPond.threshold: + await message.add_reaction("✅") def setup(bot: Bot) -> None: diff --git a/bot/constants.py b/bot/constants.py index d626fd4ba9..79845711d1 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -239,9 +239,8 @@ class Colours(metaclass=YAMLGetter): class DuckPond(metaclass=YAMLGetter): section = "duck_pond" - ducks_required: int - duck_custom_emojis: List[int] - duck_pond_channel: int + threshold: int + custom_emojis: List[int] class Emojis(metaclass=YAMLGetter): diff --git a/config-default.yml b/config-default.yml index 074143f928..59087f51fd 100644 --- a/config-default.yml +++ b/config-default.yml @@ -96,7 +96,6 @@ guild: defcon: &DEFCON 464469101889454091 devlog: &DEVLOG 622895325144940554 devtest: &DEVTEST 414574275865870337 - duck_pond: &DUCK_POND 637820308341915648 help_0: 303906576991780866 help_1: 303906556754395136 help_2: 303906514266226689 @@ -383,9 +382,8 @@ redirect_output: delete_delay: 15 duck_pond: - ducks_required: 5 - duck_custom_emojis: [*DUCKY_EMOJI, *DUCKY_BLURPLE_EMOJI] - duck_pond_channel: *DUCK_POND + threshold: 5 + custom_emojis: [*DUCKY_EMOJI, *DUCKY_BLURPLE_EMOJI] config: required_keys: ['bot.token'] From 08d0c46c4aca4f181e60ef4cf6aa9a450c0db200 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leon=20Sand=C3=B8y?= Date: Sun, 27 Oct 2019 15:38:09 +0100 Subject: [PATCH 08/26] Adding kosas additional ducks to default-config --- config-default.yml | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/config-default.yml b/config-default.yml index 59087f51fd..76892677e8 100644 --- a/config-default.yml +++ b/config-default.yml @@ -32,8 +32,13 @@ style: new: "\U0001F195" cross_mark: "\u274C" - ducky: &DUCKY_EMOJI 574951975574175744 - ducky_blurple: &DUCKY_BLURPLE_EMOJI 574951975310065675 + ducky_yellow: &DUCKY_YELLOW 574951975574175744 + ducky_blurple: &DUCKY_BLURPLE 574951975310065675 + ducky_regal: &DUCKY_REGAL 637883439185395712 + ducky_camo: &DUCKY_CAMO 637914731566596096 + ducky_ninja: &DUCKY_NINJA 637923502535606293 + ducky_devil: &DUCKY_DEVIL 637925314982576139 + ducky_tube: &DUCKY_TUBE 637881368008851456 icons: crown_blurple: "https://cdn.discordapp.com/emojis/469964153289965568.png" @@ -383,7 +388,7 @@ redirect_output: duck_pond: threshold: 5 - custom_emojis: [*DUCKY_EMOJI, *DUCKY_BLURPLE_EMOJI] + custom_emojis: [*DUCKY_YELLOW, *DUCKY_BLURPLE, *DUCKY_CAMO, *DUCKY_DEVIL, *DUCKY_NINJA, *DUCKY_REGAL, *DUCKY_TUBE] config: required_keys: ['bot.token'] From a98485e173208cc2272f5c6355ddaf5858050403 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leon=20Sand=C3=B8y?= Date: Thu, 31 Oct 2019 07:39:03 +0100 Subject: [PATCH 09/26] Figure out which tests we need. This adds empty tests for all the tests I'd like to add to this pull request. It also adds a few more duckies to the emoji constant list, and adds a single line of clarification to the testing readme. --- bot/constants.py | 8 +++- tests/README.md | 1 + tests/bot/cogs/test_duck_pond.py | 80 ++++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 tests/bot/cogs/test_duck_pond.py diff --git a/bot/constants.py b/bot/constants.py index 79845711d1..dbbf320631 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -261,8 +261,13 @@ class Emojis(metaclass=YAMLGetter): pencil: str cross_mark: str - ducky: int + ducky_yellow: int ducky_blurple: int + ducky_regal: int + ducky_camo: int + ducky_ninja: int + ducky_devil: int + ducky_tube: int class Icons(metaclass=YAMLGetter): @@ -341,7 +346,6 @@ class Channels(metaclass=YAMLGetter): defcon: int devlog: int devtest: int - duck_pond: int help_0: int help_1: int help_2: int diff --git a/tests/README.md b/tests/README.md index 6ab9bc93e8..d052de2f62 100644 --- a/tests/README.md +++ b/tests/README.md @@ -15,6 +15,7 @@ We are using the following modules and packages for our unit tests: To ensure the results you obtain on your personal machine are comparable to those generated in the Azure pipeline, please make sure to run your tests with the virtual environment defined by our [Pipfile](/Pipfile). To run your tests with `pipenv`, we've provided two "scripts" shortcuts: - `pipenv run test` will run `unittest` with `coverage.py` +- `pipenv run test path/to/test.py` will run a specific test. - `pipenv run report` will generate a coverage report of the tests you've run with `pipenv run test`. If you append the `-m` flag to this command, the report will include the lines and branches not covered by tests in addition to the test coverage report. If you want a coverage report, make sure to run the tests with `pipenv run test` *first*. diff --git a/tests/bot/cogs/test_duck_pond.py b/tests/bot/cogs/test_duck_pond.py new file mode 100644 index 0000000000..79f11843b7 --- /dev/null +++ b/tests/bot/cogs/test_duck_pond.py @@ -0,0 +1,80 @@ +import logging +import unittest +from unittest.mock import MagicMock + +from bot.cogs import duck_pond +from tests.helpers import MockBot, MockMessage + + +class DuckPondTest(unittest.TestCase): + """Tests the `DuckPond` cog.""" + + def setUp(self): + """Adds the cog, a bot, and a message to the instance for usage in tests.""" + self.bot = MockBot() + self.cog = duck_pond.DuckPond(bot=self.bot) + + self.msg = MockMessage(message_id=555, content='') + self.msg.author.__str__ = MagicMock() + self.msg.author.__str__.return_value = 'lemon' + self.msg.author.bot = False + self.msg.author.avatar_url_as.return_value = 'picture-lemon.png' + self.msg.author.id = 42 + self.msg.author.mention = '@lemon' + self.msg.channel.mention = "#lemonade-stand" + + def test_is_staff_correctly_identifies_staff(self): + """A string decoding to numeric characters is a valid user ID.""" + pass + + def test_has_green_checkmark(self): + """A string decoding to numeric characters is a valid user ID.""" + pass + + def test_count_custom_duck_emojis(self): + """A string decoding to numeric characters is a valid user ID.""" + pass + + def test_count_unicode_duck_emojis(self): + """A string decoding to numeric characters is a valid user ID.""" + pass + + def test_count_mixed_duck_emojis(self): + """A string decoding to numeric characters is a valid user ID.""" + pass + + def test_raw_reaction_add_rejects_bot(self): + """A string decoding to numeric characters is a valid user ID.""" + pass + + def test_raw_reaction_add_rejects_non_staff(self): + """A string decoding to numeric characters is a valid user ID.""" + pass + + def test_raw_reaction_add_sends_message_on_valid_input(self): + """A string decoding to numeric characters is a valid user ID.""" + pass + + def test_raw_reaction_remove_rejects_non_checkmarks(self): + """A string decoding to numeric characters is a valid user ID.""" + pass + + def test_raw_reaction_remove_prevents_checkmark_removal(self): + """A string decoding to numeric characters is a valid user ID.""" + pass + + +class DuckPondSetupTests(unittest.TestCase): + """Tests setup of the `DuckPond` cog.""" + + def test_setup(self): + """Setup of the cog should log a message at `INFO` level.""" + bot = MockBot() + log = logging.getLogger('bot.cogs.duck_pond') + + with self.assertLogs(logger=log, level=logging.INFO) as log_watcher: + duck_pond.setup(bot) + line = log_watcher.output[0] + + bot.add_cog.assert_called_once() + self.assertIn("Cog loaded: DuckPond", line) From acb937dc24b30c84b4978f651a642208f562c36e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leon=20Sand=C3=B8y?= Date: Sun, 3 Nov 2019 22:34:16 +0100 Subject: [PATCH 10/26] Test is_staff and has_green_checkmark. --- tests/bot/cogs/test_duck_pond.py | 52 +++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/tests/bot/cogs/test_duck_pond.py b/tests/bot/cogs/test_duck_pond.py index 79f11843b7..31c7e9f891 100644 --- a/tests/bot/cogs/test_duck_pond.py +++ b/tests/bot/cogs/test_duck_pond.py @@ -1,35 +1,53 @@ import logging import unittest -from unittest.mock import MagicMock from bot.cogs import duck_pond -from tests.helpers import MockBot, MockMessage +from tests.helpers import MockBot, MockMember, MockMessage, MockReaction, MockRole class DuckPondTest(unittest.TestCase): """Tests the `DuckPond` cog.""" def setUp(self): - """Adds the cog, a bot, and a message to the instance for usage in tests.""" + """Adds the cog, a bot, and the mocks we'll need for our tests.""" self.bot = MockBot() self.cog = duck_pond.DuckPond(bot=self.bot) - self.msg = MockMessage(message_id=555, content='') - self.msg.author.__str__ = MagicMock() - self.msg.author.__str__.return_value = 'lemon' - self.msg.author.bot = False - self.msg.author.avatar_url_as.return_value = 'picture-lemon.png' - self.msg.author.id = 42 - self.msg.author.mention = '@lemon' - self.msg.channel.mention = "#lemonade-stand" + # Set up some roles + self.admin_role = MockRole(name="Admins", role_id=476190234653229056) + self.contrib_role = MockRole(name="Contributor", role_id=476190302659543061) - def test_is_staff_correctly_identifies_staff(self): - """A string decoding to numeric characters is a valid user ID.""" - pass + # Set up some users + self.admin_member = MockMember(roles=(self.admin_role,)) + self.contrib_member = MockMember(roles=(self.contrib_role,)) + self.no_role_member = MockMember() - def test_has_green_checkmark(self): - """A string decoding to numeric characters is a valid user ID.""" - pass + # Set up emojis + self.checkmark_emoji = "✅" + self.thumbs_up_emoji = "👍" + + # Set up reactions + self.checkmark_reaction = MockReaction(emoji=self.checkmark_emoji) + self.thumbs_up_reaction = MockReaction(emoji=self.thumbs_up_emoji) + + # Set up a messages + self.checkmark_message = MockMessage(reactions=(self.checkmark_reaction,)) + self.thumbs_up_message = MockMessage(reactions=(self.thumbs_up_reaction,)) + self.no_reaction_message = MockMessage() + + def test_is_staff_correctly_identifies_staff(self): + """Test that is_staff correctly identifies a staff member.""" + with self.subTest(): + self.assertTrue(duck_pond.DuckPond.is_staff(self.admin_member)) + self.assertFalse(duck_pond.DuckPond.is_staff(self.contrib_member)) + self.assertFalse(duck_pond.DuckPond.is_staff(self.no_role_member)) + + def test_has_green_checkmark_correctly_identifies_messages(self): + """Test that has_green_checkmark recognizes messages with checkmarks.""" + with self.subTest(): + self.assertTrue(duck_pond.DuckPond.has_green_checkmark(self.checkmark_message)) + self.assertFalse(duck_pond.DuckPond.has_green_checkmark(self.thumbs_up_message)) + self.assertFalse(duck_pond.DuckPond.has_green_checkmark(self.no_reaction_message)) def test_count_custom_duck_emojis(self): """A string decoding to numeric characters is a valid user ID.""" From aac8404f65b419e212e5372015b63871fab7f3d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leon=20Sand=C3=B8y?= Date: Mon, 11 Nov 2019 07:19:07 +0100 Subject: [PATCH 11/26] Adding ducky count tests and a new AsyncIteratorMock --- tests/bot/cogs/test_duck_pond.py | 70 +++++++++++++++++++++++++------- tests/helpers.py | 38 ++++++++++++++++- 2 files changed, 93 insertions(+), 15 deletions(-) diff --git a/tests/bot/cogs/test_duck_pond.py b/tests/bot/cogs/test_duck_pond.py index 31c7e9f891..af8ef0e4d7 100644 --- a/tests/bot/cogs/test_duck_pond.py +++ b/tests/bot/cogs/test_duck_pond.py @@ -1,8 +1,10 @@ +import asyncio import logging import unittest +from bot import constants from bot.cogs import duck_pond -from tests.helpers import MockBot, MockMember, MockMessage, MockReaction, MockRole +from tests.helpers import MockBot, MockEmoji, MockMember, MockMessage, MockReaction, MockRole class DuckPondTest(unittest.TestCase): @@ -13,49 +15,89 @@ def setUp(self): self.bot = MockBot() self.cog = duck_pond.DuckPond(bot=self.bot) + # Override the constants we'll be needing + constants.STAFF_ROLES = (123,) + constants.DuckPond.custom_emojis = (789,) + constants.DuckPond.threshold = 1 + # Set up some roles - self.admin_role = MockRole(name="Admins", role_id=476190234653229056) - self.contrib_role = MockRole(name="Contributor", role_id=476190302659543061) + self.admin_role = MockRole(name="Admins", role_id=123) + self.contrib_role = MockRole(name="Contributor", role_id=456) # Set up some users - self.admin_member = MockMember(roles=(self.admin_role,)) + self.admin_member_1 = MockMember(roles=(self.admin_role,), id=1) + self.admin_member_2 = MockMember(roles=(self.admin_role,), id=2) self.contrib_member = MockMember(roles=(self.contrib_role,)) self.no_role_member = MockMember() # Set up emojis self.checkmark_emoji = "✅" self.thumbs_up_emoji = "👍" + self.unicode_duck_emoji = "🦆" + self.yellow_ducky_emoji = MockEmoji(id=789) # Set up reactions - self.checkmark_reaction = MockReaction(emoji=self.checkmark_emoji) - self.thumbs_up_reaction = MockReaction(emoji=self.thumbs_up_emoji) + self.checkmark_reaction = MockReaction( + emoji=self.checkmark_emoji, + user_list=[self.admin_member_1] + ) + self.thumbs_up_reaction = MockReaction( + emoji=self.thumbs_up_emoji, + user_list=[self.admin_member_1, self.contrib_member] + ) + self.yellow_ducky_reaction = MockReaction( + emoji=self.yellow_ducky_emoji, + user_list=[self.admin_member_1, self.contrib_member] + ) + self.unicode_duck_reaction_1 = MockReaction( + emoji=self.unicode_duck_emoji, + user_list=[self.admin_member_1] + ) + self.unicode_duck_reaction_2 = MockReaction( + emoji=self.unicode_duck_emoji, + user_list=[self.admin_member_2] + ) # Set up a messages self.checkmark_message = MockMessage(reactions=(self.checkmark_reaction,)) self.thumbs_up_message = MockMessage(reactions=(self.thumbs_up_reaction,)) + self.yellow_ducky_message = MockMessage(reactions=(self.yellow_ducky_reaction,)) + self.unicode_duck_message = MockMessage(reactions=(self.unicode_duck_reaction_1,)) + self.double_duck_message = MockMessage(reactions=(self.unicode_duck_reaction_1, self.unicode_duck_reaction_2)) self.no_reaction_message = MockMessage() def test_is_staff_correctly_identifies_staff(self): """Test that is_staff correctly identifies a staff member.""" with self.subTest(): - self.assertTrue(duck_pond.DuckPond.is_staff(self.admin_member)) - self.assertFalse(duck_pond.DuckPond.is_staff(self.contrib_member)) - self.assertFalse(duck_pond.DuckPond.is_staff(self.no_role_member)) + self.assertTrue(self.cog.is_staff(self.admin_member_1)) + self.assertFalse(self.cog.is_staff(self.contrib_member)) + self.assertFalse(self.cog.is_staff(self.no_role_member)) def test_has_green_checkmark_correctly_identifies_messages(self): """Test that has_green_checkmark recognizes messages with checkmarks.""" with self.subTest(): - self.assertTrue(duck_pond.DuckPond.has_green_checkmark(self.checkmark_message)) - self.assertFalse(duck_pond.DuckPond.has_green_checkmark(self.thumbs_up_message)) - self.assertFalse(duck_pond.DuckPond.has_green_checkmark(self.no_reaction_message)) + self.assertTrue(self.cog.has_green_checkmark(self.checkmark_message)) + self.assertFalse(self.cog.has_green_checkmark(self.thumbs_up_message)) + self.assertFalse(self.cog.has_green_checkmark(self.no_reaction_message)) def test_count_custom_duck_emojis(self): """A string decoding to numeric characters is a valid user ID.""" - pass + count_one_duck = self.cog.count_ducks(self.yellow_ducky_message) + count_no_ducks = self.cog.count_ducks(self.thumbs_up_message) + with self.subTest(): + self.assertEqual(asyncio.run(count_one_duck), 1) + self.assertEqual(asyncio.run(count_no_ducks), 0) def test_count_unicode_duck_emojis(self): """A string decoding to numeric characters is a valid user ID.""" - pass + count_no_ducks = self.cog.count_ducks(self.thumbs_up_message) + count_one_duck = self.cog.count_ducks(self.unicode_duck_message) + count_two_ducks = self.cog.count_ducks(self.double_duck_message) + + with self.subTest(): + self.assertEqual(asyncio.run(count_no_ducks), 0) + self.assertEqual(asyncio.run(count_one_duck), 1) + self.assertEqual(asyncio.run(count_two_ducks), 2) def test_count_mixed_duck_emojis(self): """A string decoding to numeric characters is a valid user ID.""" diff --git a/tests/helpers.py b/tests/helpers.py index 8496ba0316..fd79141ecd 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -102,8 +102,30 @@ class AsyncMock(CustomMockMixin, unittest.mock.MagicMock): Python 3.8 will introduce an AsyncMock class in the standard library that will have some more features; this stand-in only overwrites the `__call__` method to an async version. """ + async def __call__(self, *args, **kwargs): - return super(AsyncMock, self).__call__(*args, **kwargs) + return super().__call__(*args, **kwargs) + + +class AsyncIteratorMock: + """ + A class to mock asyncronous iterators. + + This allows async for, which is used in certain Discord.py objects. For example, + an async iterator is returned by the Reaction.users() coroutine. + """ + + def __init__(self, sequence): + self.iter = iter(sequence) + + def __aiter__(self): + return self + + async def __anext__(self): + try: + return next(self.iter) + except StopIteration: + raise StopAsyncIteration # Create a guild instance to get a realistic Mock of `discord.Guild` @@ -155,6 +177,7 @@ class MockGuild(CustomMockMixin, unittest.mock.Mock, HashableMixin): For more info, see the `Mocking` section in `tests/README.md`. """ + def __init__( self, guild_id: int = 1, @@ -187,6 +210,7 @@ class MockRole(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): Instances of this class will follow the specifications of `discord.Role` instances. For more information, see the `MockGuild` docstring. """ + def __init__(self, name: str = "role", role_id: int = 1, position: int = 1, **kwargs) -> None: super().__init__(spec=role_instance, **kwargs) @@ -213,6 +237,7 @@ class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin Instances of this class will follow the specifications of `discord.Member` instances. For more information, see the `MockGuild` docstring. """ + def __init__( self, name: str = "member", @@ -243,6 +268,7 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): Instances of this class will follow the specifications of `discord.ext.commands.Bot` instances. For more information, see the `MockGuild` docstring. """ + def __init__(self, **kwargs) -> None: super().__init__(spec=bot_instance, **kwargs) @@ -279,6 +305,7 @@ class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): Instances of this class will follow the specifications of `discord.TextChannel` instances. For more information, see the `MockGuild` docstring. """ + def __init__(self, name: str = 'channel', channel_id: int = 1, **kwargs) -> None: super().__init__(spec=channel_instance, **kwargs) self.id = channel_id @@ -320,6 +347,7 @@ class MockContext(CustomMockMixin, unittest.mock.MagicMock): Instances of this class will follow the specifications of `discord.ext.commands.Context` instances. For more information, see the `MockGuild` docstring. """ + def __init__(self, **kwargs) -> None: super().__init__(spec=context_instance, **kwargs) self.bot = kwargs.get('bot', MockBot()) @@ -336,6 +364,7 @@ class MockMessage(CustomMockMixin, unittest.mock.MagicMock): Instances of this class will follow the specifications of `discord.Message` instances. For more information, see the `MockGuild` docstring. """ + def __init__(self, **kwargs) -> None: super().__init__(spec=message_instance, **kwargs) self.author = kwargs.get('author', MockMember()) @@ -353,6 +382,7 @@ class MockEmoji(CustomMockMixin, unittest.mock.MagicMock): Instances of this class will follow the specifications of `discord.Emoji` instances. For more information, see the `MockGuild` docstring. """ + def __init__(self, **kwargs) -> None: super().__init__(spec=emoji_instance, **kwargs) self.guild = kwargs.get('guild', MockGuild()) @@ -371,6 +401,7 @@ class MockPartialEmoji(CustomMockMixin, unittest.mock.MagicMock): Instances of this class will follow the specifications of `discord.PartialEmoji` instances. For more information, see the `MockGuild` docstring. """ + def __init__(self, **kwargs) -> None: super().__init__(spec=partial_emoji_instance, **kwargs) @@ -385,7 +416,12 @@ class MockReaction(CustomMockMixin, unittest.mock.MagicMock): Instances of this class will follow the specifications of `discord.Reaction` instances. For more information, see the `MockGuild` docstring. """ + def __init__(self, **kwargs) -> None: super().__init__(spec=reaction_instance, **kwargs) self.emoji = kwargs.get('emoji', MockEmoji()) self.message = kwargs.get('message', MockMessage()) + self.user_list = AsyncIteratorMock(kwargs.get('user_list', [])) + + def users(self): + return self.user_list From 98ccfbc218dc762e45f0146d0503dba1fe06fdb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leon=20Sand=C3=B8y?= Date: Mon, 11 Nov 2019 14:55:40 +0100 Subject: [PATCH 12/26] Implement a mixed duck test. Also gets started setting up for the final tests, which will require more mockwork. --- tests/bot/cogs/test_duck_pond.py | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/tests/bot/cogs/test_duck_pond.py b/tests/bot/cogs/test_duck_pond.py index af8ef0e4d7..211e8b084b 100644 --- a/tests/bot/cogs/test_duck_pond.py +++ b/tests/bot/cogs/test_duck_pond.py @@ -20,6 +20,10 @@ def setUp(self): constants.DuckPond.custom_emojis = (789,) constants.DuckPond.threshold = 1 + # Mock bot.get_all_channels() + CHANNEL_ID = 555 + USER_ID = 666 + # Set up some roles self.admin_role = MockRole(name="Admins", role_id=123) self.contrib_role = MockRole(name="Contributor", role_id=456) @@ -63,7 +67,12 @@ def setUp(self): self.thumbs_up_message = MockMessage(reactions=(self.thumbs_up_reaction,)) self.yellow_ducky_message = MockMessage(reactions=(self.yellow_ducky_reaction,)) self.unicode_duck_message = MockMessage(reactions=(self.unicode_duck_reaction_1,)) - self.double_duck_message = MockMessage(reactions=(self.unicode_duck_reaction_1, self.unicode_duck_reaction_2)) + self.double_unicode_duck_message = MockMessage( + reactions=(self.unicode_duck_reaction_1, self.unicode_duck_reaction_2) + ) + self.double_mixed_duck_message = MockMessage( + reactions=(self.unicode_duck_reaction_1, self.yellow_ducky_reaction) + ) self.no_reaction_message = MockMessage() def test_is_staff_correctly_identifies_staff(self): @@ -81,27 +90,28 @@ def test_has_green_checkmark_correctly_identifies_messages(self): self.assertFalse(self.cog.has_green_checkmark(self.no_reaction_message)) def test_count_custom_duck_emojis(self): - """A string decoding to numeric characters is a valid user ID.""" - count_one_duck = self.cog.count_ducks(self.yellow_ducky_message) + """Test that count_ducks counts custom ducks correctly.""" count_no_ducks = self.cog.count_ducks(self.thumbs_up_message) + count_one_duck = self.cog.count_ducks(self.yellow_ducky_message) with self.subTest(): - self.assertEqual(asyncio.run(count_one_duck), 1) self.assertEqual(asyncio.run(count_no_ducks), 0) + self.assertEqual(asyncio.run(count_one_duck), 1) def test_count_unicode_duck_emojis(self): - """A string decoding to numeric characters is a valid user ID.""" - count_no_ducks = self.cog.count_ducks(self.thumbs_up_message) + """Test that count_ducks counts unicode ducks correctly.""" count_one_duck = self.cog.count_ducks(self.unicode_duck_message) - count_two_ducks = self.cog.count_ducks(self.double_duck_message) + count_two_ducks = self.cog.count_ducks(self.double_unicode_duck_message) with self.subTest(): - self.assertEqual(asyncio.run(count_no_ducks), 0) self.assertEqual(asyncio.run(count_one_duck), 1) self.assertEqual(asyncio.run(count_two_ducks), 2) def test_count_mixed_duck_emojis(self): - """A string decoding to numeric characters is a valid user ID.""" - pass + """Test that count_ducks counts mixed ducks correctly.""" + count_two_ducks = self.cog.count_ducks(self.double_mixed_duck_message) + + with self.subTest(): + self.assertEqual(asyncio.run(count_two_ducks), 2) def test_raw_reaction_add_rejects_bot(self): """A string decoding to numeric characters is a valid user ID.""" From a89349ee32bbf2b3506cc278999575db1fbfde74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leon=20Sand=C3=B8y?= Date: Tue, 12 Nov 2019 22:05:28 +0100 Subject: [PATCH 13/26] Add tests for on_raw_reaction_add. Basically I suck at this and I can't get this return_value thing to work. I'll have Ves look at it to resolve it. As of right now, multiple tests are failing. --- tests/bot/cogs/test_duck_pond.py | 84 ++++++++++++++++++++++++++------ 1 file changed, 70 insertions(+), 14 deletions(-) diff --git a/tests/bot/cogs/test_duck_pond.py b/tests/bot/cogs/test_duck_pond.py index 211e8b084b..088d8ac797 100644 --- a/tests/bot/cogs/test_duck_pond.py +++ b/tests/bot/cogs/test_duck_pond.py @@ -1,10 +1,11 @@ import asyncio import logging import unittest +from unittest.mock import MagicMock from bot import constants from bot.cogs import duck_pond -from tests.helpers import MockBot, MockEmoji, MockMember, MockMessage, MockReaction, MockRole +from tests.helpers import MockBot, MockEmoji, MockMember, MockMessage, MockReaction, MockRole, MockTextChannel class DuckPondTest(unittest.TestCase): @@ -15,23 +16,27 @@ def setUp(self): self.bot = MockBot() self.cog = duck_pond.DuckPond(bot=self.bot) + # Set up some constants + self.CHANNEL_ID = 555 + self.MESSAGE_ID = 666 + self.BOT_ID = 777 + self.CONTRIB_ID = 888 + self.ADMIN_ID = 999 + # Override the constants we'll be needing constants.STAFF_ROLES = (123,) constants.DuckPond.custom_emojis = (789,) constants.DuckPond.threshold = 1 - # Mock bot.get_all_channels() - CHANNEL_ID = 555 - USER_ID = 666 - # Set up some roles self.admin_role = MockRole(name="Admins", role_id=123) self.contrib_role = MockRole(name="Contributor", role_id=456) # Set up some users - self.admin_member_1 = MockMember(roles=(self.admin_role,), id=1) - self.admin_member_2 = MockMember(roles=(self.admin_role,), id=2) - self.contrib_member = MockMember(roles=(self.contrib_role,)) + self.admin_member_1 = MockMember(roles=(self.admin_role,), id=self.ADMIN_ID) + self.admin_member_2 = MockMember(roles=(self.admin_role,), id=911) + self.contrib_member = MockMember(roles=(self.contrib_role,), id=self.CONTRIB_ID) + self.bot_member = MockMember(roles=(self.contrib_role,), id=self.BOT_ID, bot=True) self.no_role_member = MockMember() # Set up emojis @@ -61,6 +66,14 @@ def setUp(self): emoji=self.unicode_duck_emoji, user_list=[self.admin_member_2] ) + self.bot_reaction = MockReaction( + emoji=self.yellow_ducky_emoji, + user_list=[self.bot_member] + ) + self.contrib_reaction = MockReaction( + emoji=self.yellow_ducky_emoji, + user_list=[self.contrib_member] + ) # Set up a messages self.checkmark_message = MockMessage(reactions=(self.checkmark_reaction,)) @@ -73,8 +86,18 @@ def setUp(self): self.double_mixed_duck_message = MockMessage( reactions=(self.unicode_duck_reaction_1, self.yellow_ducky_reaction) ) + + self.bot_message = MockMessage(reactions=(self.bot_reaction,)) + self.contrib_message = MockMessage(reactions=(self.contrib_reaction,)) self.no_reaction_message = MockMessage() + # Set up some channels + self.text_channel = MockTextChannel(id=self.CHANNEL_ID) + + @staticmethod + def _mock_send_webhook(content, username, avatar_url, embed): + """Mock for the send_webhook method in DuckPond""" + def test_is_staff_correctly_identifies_staff(self): """Test that is_staff correctly identifies a staff member.""" with self.subTest(): @@ -114,16 +137,49 @@ def test_count_mixed_duck_emojis(self): self.assertEqual(asyncio.run(count_two_ducks), 2) def test_raw_reaction_add_rejects_bot(self): - """A string decoding to numeric characters is a valid user ID.""" - pass + """Test that send_webhook is not called if the user is a bot.""" + self.text_channel.fetch_message.return_value = self.bot_message + self.bot.get_all_channels.return_value = (self.text_channel,) + + payload = MagicMock( # RawReactionActionEvent + channel_id=self.CHANNEL_ID, + message_id=self.MESSAGE_ID, + user_id=self.BOT_ID, + ) + + with self.subTest(): + asyncio.run(self.cog.on_raw_reaction_add(payload)) + self.bot.cog.send_webhook.assert_not_called() def test_raw_reaction_add_rejects_non_staff(self): - """A string decoding to numeric characters is a valid user ID.""" - pass + """Test that send_webhook is not called if the user is not a member of staff.""" + self.text_channel.fetch_message.return_value = self.contrib_message + self.bot.get_all_channels.return_value = (self.text_channel,) + + payload = MagicMock( # RawReactionActionEvent + channel_id=self.CHANNEL_ID, + message_id=self.MESSAGE_ID, + user_id=self.CONTRIB_ID, + ) + + with self.subTest(): + asyncio.run(self.cog.on_raw_reaction_add(payload)) + self.bot.cog.send_webhook.assert_not_called() def test_raw_reaction_add_sends_message_on_valid_input(self): - """A string decoding to numeric characters is a valid user ID.""" - pass + """Test that send_webhook is called if payload is valid.""" + self.text_channel.fetch_message.return_value = self.unicode_duck_message + self.bot.get_all_channels.return_value = (self.text_channel,) + + payload = MagicMock( # RawReactionActionEvent + channel_id=self.CHANNEL_ID, + message_id=self.MESSAGE_ID, + user_id=self.ADMIN_ID, + ) + + with self.subTest(): + asyncio.run(self.cog.on_raw_reaction_add(payload)) + self.bot.cog.send_webhook.assert_called_once() def test_raw_reaction_remove_rejects_non_checkmarks(self): """A string decoding to numeric characters is a valid user ID.""" From ccda39c5e42e94011c9c1bd14080d004d3d61f02 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Thu, 14 Nov 2019 10:50:49 +0100 Subject: [PATCH 14/26] Add bot=False default value to MockMember By default, a mocked value is considered `truthy` in Python, like all non-empty/non-zero/non-None values in Python. This means that if an attribute is not explicitly set on a mock, it will evaluate at as truthy in a boolean context, since the mock will provide a truthy mocked value by default. This is not the best default value for the `bot` attribute of our MockMember type, since members are rarely bots. It makes much more intuitive sense to me to consider a member to not be a bot, unless we explicitly set `bot=True`. This commit sets that sensible default value that can be overwritten by passing `bot=False` to the constructor or setting the `object.bot` attribute to `False` after the creation of the mock. --- tests/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/helpers.py b/tests/helpers.py index 22f07934f4..199d457006 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -242,7 +242,7 @@ class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin information, see the `MockGuild` docstring. """ def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None: - default_kwargs = {'name': 'member', 'id': next(self.discord_id)} + default_kwargs = {'name': 'member', 'id': next(self.discord_id), 'bot': False} super().__init__(spec_set=member_instance, **collections.ChainMap(kwargs, default_kwargs)) self.roles = [MockRole(name="@everyone", position=1, id=0)] From 61051f9cc5abbf571dfa13c49324109ef16f78fc Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Thu, 14 Nov 2019 10:58:40 +0100 Subject: [PATCH 15/26] Add MockAttachment type and attachments default for MockMessage As stated from the start, our intention is to add custom mock types as we need them for testing. While writing tests for DuckPond, I noticed that we did not have a mock type for Attachments, so I added one with this commit. In addition, I think it's a very sensible for MockMessage to have an empty list as a default value for the `attachements` attribute. This is equal to what `discord.Message` returns for a message without attachments and makes sure that if you don't explicitely add an attachment to a message, `MockMessage.attachments` tests as falsey. --- tests/helpers.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/helpers.py b/tests/helpers.py index 199d457006..3e43679fec 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -355,6 +355,20 @@ def __init__(self, **kwargs) -> None: self.channel = kwargs.get('channel', MockTextChannel()) +attachment_instance = discord.Attachment(data=unittest.mock.MagicMock(id=1), state=unittest.mock.MagicMock()) + + +class MockAttachment(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Attachment objects. + + Instances of this class will follow the specifications of `discord.Attachment` instances. For + more information, see the `MockGuild` docstring. + """ + def __init__(self, **kwargs) -> None: + super().__init__(spec_set=attachment_instance, **kwargs) + + class MockMessage(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Message objects. @@ -364,7 +378,8 @@ class MockMessage(CustomMockMixin, unittest.mock.MagicMock): """ def __init__(self, **kwargs) -> None: - super().__init__(spec_set=message_instance, **kwargs) + default_kwargs = {'attachments': []} + super().__init__(spec_set=message_instance, **collections.ChainMap(kwargs, default_kwargs)) self.author = kwargs.get('author', MockMember()) self.channel = kwargs.get('channel', MockTextChannel()) From 8c64fc637dda73cfa4b79d1f3541d067380e51d8 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 15 Nov 2019 01:02:40 +0100 Subject: [PATCH 16/26] Check only for bot's green checkmark in DuckPond Previously, the presence of any green checkmark as a reaction would prevent a message from being relayed to the duck pond, regardless of the actor of that reaction. Since we only want to check if the bot has already processed this message, we should check for a checkmark added by the bot. This commit adds such a user check. --- bot/cogs/duck_pond.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/bot/cogs/duck_pond.py b/bot/cogs/duck_pond.py index 45bbc410b8..aac023a2e0 100644 --- a/bot/cogs/duck_pond.py +++ b/bot/cogs/duck_pond.py @@ -37,12 +37,13 @@ def is_staff(member: Union[User, Member]) -> bool: return True return False - @staticmethod - def has_green_checkmark(message: Message) -> bool: + async def has_green_checkmark(self, message: Message) -> bool: """Check if the message has a green checkmark reaction.""" for reaction in message.reactions: if reaction.emoji == "✅": - return True + async for user in reaction.users(): + if user == self.bot.user: + return True return False async def send_webhook( @@ -115,7 +116,7 @@ async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None: return # Does the message already have a green checkmark? - if self.has_green_checkmark(message): + if await self.has_green_checkmark(message): return # Time to count our ducks! From f56f6cebc5300ec3c1b52ec8988ae9c27571c14e Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 15 Nov 2019 01:09:22 +0100 Subject: [PATCH 17/26] Refactor DuckPond msg relay to separate method To allow for separate testing of the code that relays messages to the duck pond, I have moved this part of the code from the event listener to a separate method. The overall logic has remained unchanged. In addition, I've kaizened to things: - Removed unnecessary f-string without interpolation; - Removed double negative (not item not in list) --- bot/cogs/duck_pond.py | 62 +++++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/bot/cogs/duck_pond.py b/bot/cogs/duck_pond.py index aac023a2e0..b2b4ad0c24 100644 --- a/bot/cogs/duck_pond.py +++ b/bot/cogs/duck_pond.py @@ -62,7 +62,7 @@ async def send_webhook( embed=embed ) except discord.HTTPException: - log.exception(f"Failed to send a message to the Duck Pool webhook") + log.exception("Failed to send a message to the Duck Pool webhook") async def count_ducks(self, message: Message) -> int: """ @@ -76,8 +76,8 @@ async def count_ducks(self, message: Message) -> int: for reaction in message.reactions: async for user in reaction.users(): - # Is the user or member a staff member? - if not self.is_staff(user) or not user.id not in duck_reactors: + # Is the user a staff member and not already counted as reactor? + if not self.is_staff(user) or user.id in duck_reactors: continue # Is the emoji a duck? @@ -91,6 +91,35 @@ async def count_ducks(self, message: Message) -> int: duck_reactors.append(user.id) return duck_count + async def relay_message_to_duck_pond(self, message: Message) -> None: + """Relays the message's content and attachments to the duck pond channel.""" + clean_content = message.clean_content + + if clean_content: + await self.send_webhook( + content=message.clean_content, + username=message.author.display_name, + avatar_url=message.author.avatar_url + ) + + if message.attachments: + try: + await send_attachments(message, self.webhook) + except (errors.Forbidden, errors.NotFound): + e = Embed( + description=":x: **This message contained an attachment, but it could not be retrieved**", + color=Color.red() + ) + await self.send_webhook( + embed=e, + username=message.author.display_name, + avatar_url=message.author.avatar_url + ) + except discord.HTTPException: + log.exception(f"Failed to send an attachment to the webhook") + + await message.add_reaction("✅") + @Cog.listener() async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None: """ @@ -124,32 +153,7 @@ async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None: # If we've got more than the required amount of ducks, send the message to the duck_pond. if duck_count >= constants.DuckPond.threshold: - clean_content = message.clean_content - - if clean_content: - await self.send_webhook( - content=message.clean_content, - username=message.author.display_name, - avatar_url=message.author.avatar_url - ) - - if message.attachments: - try: - await send_attachments(message, self.webhook) - except (errors.Forbidden, errors.NotFound): - e = Embed( - description=":x: **This message contained an attachment, but it could not be retrieved**", - color=Color.red() - ) - await self.send_webhook( - embed=e, - username=message.author.display_name, - avatar_url=message.author.avatar_url - ) - except discord.HTTPException: - log.exception(f"Failed to send an attachment to the webhook") - - await message.add_reaction("✅") + await self.relay_message_to_duck_pond(message) @Cog.listener() async def on_raw_reaction_remove(self, payload: RawReactionActionEvent) -> None: From 89890d6e1b673622cba918be48f325540e45db9e Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 15 Nov 2019 01:15:01 +0100 Subject: [PATCH 18/26] Move payload checks to start of DuckPond.on_raw_reaction_add The `DuckPond.on_raw_message_add` event listener makes an API call to fetch the message the reaction was added to. However, we don't need to fetch the message if the reaction that was added is not relevant to the duck pond. To prevent such unnecessary API calls, I have moved the code that checks for the relevance of the reaction event to before the code that fetches the message. --- bot/cogs/duck_pond.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/bot/cogs/duck_pond.py b/bot/cogs/duck_pond.py index b2b4ad0c24..68fb094084 100644 --- a/bot/cogs/duck_pond.py +++ b/bot/cogs/duck_pond.py @@ -129,6 +129,13 @@ async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None: amount of ducks specified in the config under duck_pond/threshold, it will send the message off to the duck pond. """ + # Is the emoji in the reaction a duck? + if payload.emoji.is_custom_emoji(): + if payload.emoji.id not in constants.DuckPond.custom_emojis: + return + elif payload.emoji.name != "🦆": + return + channel = discord.utils.get(self.bot.get_all_channels(), id=payload.channel_id) message = await channel.fetch_message(payload.message_id) member = discord.utils.get(message.guild.members, id=payload.user_id) @@ -137,13 +144,6 @@ async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None: if not self.is_staff(member) or member.bot: return - # Is the emoji in the reaction a duck? - if payload.emoji.is_custom_emoji(): - if payload.emoji.id not in constants.DuckPond.custom_emojis: - return - elif payload.emoji.name != "🦆": - return - # Does the message already have a green checkmark? if await self.has_green_checkmark(message): return From 2779a912a8fbe29453543a8fd2888a842c3beb47 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 15 Nov 2019 01:21:33 +0100 Subject: [PATCH 19/26] Add `return_value` support and assertions to AsyncIteratorMock The AsyncIteratorMock included in Python 3.8 will work similarly to the mocks of callabes. This means that it allows you to set the items it will yield using the `return_value` attribute. It will also have support for the common Mock-specific assertions. This commit introduces some backports of those features in a slightly simplified way to make the transition to Python 3.8 easier in the future. --- tests/helpers.py | 58 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/tests/helpers.py b/tests/helpers.py index 3e43679fec..50652ef9a0 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -127,14 +127,20 @@ async def __call__(self, *args, **kwargs): class AsyncIteratorMock: """ - A class to mock asyncronous iterators. + A class to mock asynchronous iterators. This allows async for, which is used in certain Discord.py objects. For example, - an async iterator is returned by the Reaction.users() coroutine. + an async iterator is returned by the Reaction.users() method. """ - def __init__(self, sequence): - self.iter = iter(sequence) + def __init__(self, iterable: Iterable = None): + if iterable is None: + iterable = [] + + self.iter = iter(iterable) + self.iterable = iterable + + self.call_count = 0 def __aiter__(self): return self @@ -145,6 +151,50 @@ async def __anext__(self): except StopIteration: raise StopAsyncIteration + def __call__(self): + """ + Keeps track of the number of times an instance has been called. + + This is useful, since it typically shows that the iterator has actually been used somewhere after we have + instantiated the mock for an attribute that normally returns an iterator when called. + """ + self.call_count += 1 + return self + + @property + def return_value(self): + """Makes `self.iterable` accessible as self.return_value.""" + return self.iterable + + @return_value.setter + def return_value(self, iterable): + """Stores the `return_value` as `self.iterable` and its iterator as `self.iter`.""" + self.iter = iter(iterable) + self.iterable = iterable + + def assert_called(self): + """Asserts if the AsyncIteratorMock instance has been called at least once.""" + if self.call_count == 0: + raise AssertionError("Expected AsyncIteratorMock to have been called.") + + def assert_called_once(self): + """Asserts if the AsyncIteratorMock instance has been called exactly once.""" + if self.call_count != 1: + raise AssertionError( + f"Expected AsyncIteratorMock to have been called once. Called {self.call_count} times." + ) + + def assert_not_called(self): + """Asserts if the AsyncIteratorMock instance has not been called.""" + if self.call_count != 0: + raise AssertionError( + f"Expected AsyncIteratorMock to not have been called once. Called {self.call_count} times." + ) + + def reset_mock(self): + """Resets the call count, but not the return value or iterator.""" + self.call_count = 0 + # Create a guild instance to get a realistic Mock of `discord.Guild` guild_data = { From 2c77288eb3ff081e70508094bb8d030900860259 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 15 Nov 2019 01:28:56 +0100 Subject: [PATCH 20/26] Add MockUser to mock `discord.User` objects I have added a special mock that follows the specifications of a `discord.User` instance. This is useful, since `Users` have less attributes available than `discord.Members`. Since this difference in availability of information can be important, we should not use a `MockMember` to mock a `discord.user`. --- tests/helpers.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/helpers.py b/tests/helpers.py index 50652ef9a0..4da6bf84d3 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -303,6 +303,25 @@ def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None self.mention = f"@{self.name}" +# Create a User instance to get a realistic Mock of `discord.User` +user_instance = discord.User(data=unittest.mock.MagicMock(), state=unittest.mock.MagicMock()) + + +class MockUser(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): + """ + A Mock subclass to mock User objects. + + Instances of this class will follow the specifications of `discord.User` instances. For more + information, see the `MockGuild` docstring. + """ + def __init__(self, **kwargs) -> None: + default_kwargs = {'name': 'user', 'id': next(self.discord_id), 'bot': False} + super().__init__(spec_set=user_instance, **collections.ChainMap(kwargs, default_kwargs)) + + if 'mention' not in kwargs: + self.mention = f"@{self.name}" + + # Create a Bot instance to get a realistic MagicMock of `discord.ext.commands.Bot` bot_instance = Bot(command_prefix=unittest.mock.MagicMock()) bot_instance.http_session = None From 647370d7881d1ab242186599adb76a56a0815150 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 15 Nov 2019 01:34:46 +0100 Subject: [PATCH 21/26] Adjust MockReaction for new AsyncIteratorMock protocol The new AsyncIteratorMock no longer needs an additional method to be used with a Mock object. --- tests/helpers.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/helpers.py b/tests/helpers.py index 4da6bf84d3..13852397fe 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -500,7 +500,5 @@ def __init__(self, **kwargs) -> None: super().__init__(spec_set=reaction_instance, **kwargs) self.emoji = kwargs.get('emoji', MockEmoji()) self.message = kwargs.get('message', MockMessage()) - self.user_list = AsyncIteratorMock(kwargs.get('user_list', [])) + self.users = AsyncIteratorMock(kwargs.get('users', [])) - def users(self): - return self.user_list From b42a7b5b7f2c1c9f9924eeb9d39f7767306824ec Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 15 Nov 2019 01:37:02 +0100 Subject: [PATCH 22/26] Add MockAsyncWebhook to mock `discord.Webhook` objects I have added a mock type to mock `discord.Webhook` instances. Note that the current type is specifically meant to mock webhooks that use an AsyncAdaptor and therefore has AsyncMock/coroutine mocks for the "maybe-coroutine" methods specified in the `discord.py` docs. --- tests/helpers.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/helpers.py b/tests/helpers.py index 13852397fe..b2daae92d4 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -502,3 +502,24 @@ def __init__(self, **kwargs) -> None: self.message = kwargs.get('message', MockMessage()) self.users = AsyncIteratorMock(kwargs.get('users', [])) + +webhook_instance = discord.Webhook(data=unittest.mock.MagicMock(), adapter=unittest.mock.MagicMock()) + + +class MockAsyncWebhook(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Webhook objects using an AsyncWebhookAdapter. + + Instances of this class will follow the specifications of `discord.Webhook` instances. For + more information, see the `MockGuild` docstring. + """ + + def __init__(self, **kwargs) -> None: + super().__init__(spec_set=webhook_instance, **kwargs) + + # Because Webhooks can also use a synchronous "WebhookAdapter", the methods are not defined + # as coroutines. That's why we need to set the methods manually. + self.send = AsyncMock() + self.edit = AsyncMock() + self.delete = AsyncMock() + self.execute = AsyncMock() From a692a95896328adf1d52c5a5548e0c72540d6cbc Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 15 Nov 2019 01:39:51 +0100 Subject: [PATCH 23/26] Add unit tests with full coverage for `bot.cogs.duck_pond` This commit adds unit tests that provide a full branch coverage of the `bot.cogs.duck_pond` file. --- tests/bot/cogs/test_duck_pond.py | 649 +++++++++++++++++++++++-------- 1 file changed, 490 insertions(+), 159 deletions(-) diff --git a/tests/bot/cogs/test_duck_pond.py b/tests/bot/cogs/test_duck_pond.py index 088d8ac797..ceefc286f4 100644 --- a/tests/bot/cogs/test_duck_pond.py +++ b/tests/bot/cogs/test_duck_pond.py @@ -1,193 +1,524 @@ import asyncio import logging +import typing import unittest -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch + +import discord from bot import constants from bot.cogs import duck_pond -from tests.helpers import MockBot, MockEmoji, MockMember, MockMessage, MockReaction, MockRole, MockTextChannel +from tests import base +from tests import helpers + +MODULE_PATH = "bot.cogs.duck_pond" + + +class DuckPondTests(base.LoggingTestCase): + """Tests for DuckPond functionality.""" + + @classmethod + def setUpClass(cls): + """Sets up the objects that only have to be initialized once.""" + cls.nonstaff_member = helpers.MockMember(name="Non-staffer") + cls.staff_role = helpers.MockRole(name="Staff role", id=constants.STAFF_ROLES[0]) + cls.staff_member = helpers.MockMember(name="staffer", roles=[cls.staff_role]) -class DuckPondTest(unittest.TestCase): - """Tests the `DuckPond` cog.""" + cls.checkmark_emoji = "\N{White Heavy Check Mark}" + cls.thumbs_up_emoji = "\N{Thumbs Up Sign}" + cls.unicode_duck_emoji = "\N{Duck}" + cls.duck_pond_emoji = helpers.MockPartialEmoji(id=constants.DuckPond.custom_emojis[0]) + cls.non_duck_custom_emoji = helpers.MockPartialEmoji(id=123) def setUp(self): - """Adds the cog, a bot, and the mocks we'll need for our tests.""" - self.bot = MockBot() + """Sets up the objects that need to be refreshed before each test.""" + self.bot = helpers.MockBot(user=helpers.MockMember(id=46692)) self.cog = duck_pond.DuckPond(bot=self.bot) - # Set up some constants - self.CHANNEL_ID = 555 - self.MESSAGE_ID = 666 - self.BOT_ID = 777 - self.CONTRIB_ID = 888 - self.ADMIN_ID = 999 - - # Override the constants we'll be needing - constants.STAFF_ROLES = (123,) - constants.DuckPond.custom_emojis = (789,) - constants.DuckPond.threshold = 1 - - # Set up some roles - self.admin_role = MockRole(name="Admins", role_id=123) - self.contrib_role = MockRole(name="Contributor", role_id=456) - - # Set up some users - self.admin_member_1 = MockMember(roles=(self.admin_role,), id=self.ADMIN_ID) - self.admin_member_2 = MockMember(roles=(self.admin_role,), id=911) - self.contrib_member = MockMember(roles=(self.contrib_role,), id=self.CONTRIB_ID) - self.bot_member = MockMember(roles=(self.contrib_role,), id=self.BOT_ID, bot=True) - self.no_role_member = MockMember() - - # Set up emojis - self.checkmark_emoji = "✅" - self.thumbs_up_emoji = "👍" - self.unicode_duck_emoji = "🦆" - self.yellow_ducky_emoji = MockEmoji(id=789) - - # Set up reactions - self.checkmark_reaction = MockReaction( - emoji=self.checkmark_emoji, - user_list=[self.admin_member_1] - ) - self.thumbs_up_reaction = MockReaction( - emoji=self.thumbs_up_emoji, - user_list=[self.admin_member_1, self.contrib_member] - ) - self.yellow_ducky_reaction = MockReaction( - emoji=self.yellow_ducky_emoji, - user_list=[self.admin_member_1, self.contrib_member] - ) - self.unicode_duck_reaction_1 = MockReaction( - emoji=self.unicode_duck_emoji, - user_list=[self.admin_member_1] + def test_duck_pond_correctly_initializes(self): + """`__init__ should set `bot` and `webhook_id` attributes and schedule `fetch_webhook`.""" + bot = helpers.MockBot() + cog = MagicMock() + + duck_pond.DuckPond.__init__(cog, bot) + + self.assertEqual(cog.bot, bot) + self.assertEqual(cog.webhook_id, constants.Webhooks.duck_pond) + bot.loop.create_loop.called_once_with(cog.fetch_webhook()) + + def test_fetch_webhook_succeeds_without_connectivity_issues(self): + """The `fetch_webhook` method waits until `READY` event and sets the `webhook` attribute.""" + self.bot.fetch_webhook.return_value = "dummy webhook" + self.cog.webhook_id = 1 + + asyncio.run(self.cog.fetch_webhook()) + + self.bot.wait_until_ready.assert_called_once() + self.bot.fetch_webhook.assert_called_once_with(1) + self.assertEqual(self.cog.webhook, "dummy webhook") + + def test_fetch_webhook_logs_when_unable_to_fetch_webhook(self): + """The `fetch_webhook` method should log an exception when it fails to fetch the webhook.""" + self.bot.fetch_webhook.side_effect = discord.HTTPException(response=MagicMock(), message="Not found.") + self.cog.webhook_id = 1 + + log = logging.getLogger('bot.cogs.duck_pond') + with self.assertLogs(logger=log, level=logging.ERROR) as log_watcher: + asyncio.run(self.cog.fetch_webhook()) + + self.bot.wait_until_ready.assert_called_once() + self.bot.fetch_webhook.assert_called_once_with(1) + + self.assertEqual(len(log_watcher.records), 1) + + [record] = log_watcher.records + self.assertEqual(record.message, f"Failed to fetch webhook with id `{self.cog.webhook_id}`") + self.assertEqual(record.levelno, logging.ERROR) + + def test_is_staff_returns_correct_values_based_on_instance_passed(self): + """The `is_staff` method should return correct values based on the instance passed.""" + test_cases = ( + (helpers.MockUser(name="User instance"), False), + (helpers.MockMember(name="Member instance without staff role"), False), + (helpers.MockMember(name="Member instance with staff role", roles=[self.staff_role]), True) ) - self.unicode_duck_reaction_2 = MockReaction( - emoji=self.unicode_duck_emoji, - user_list=[self.admin_member_2] + + for user, expected_return in test_cases: + actual_return = self.cog.is_staff(user) + with self.subTest(user_type=user.name, expected_return=expected_return, actual_return=actual_return): + self.assertEqual(expected_return, actual_return) + + @helpers.async_test + async def test_has_green_checkmark_correctly_detects_presence_of_green_checkmark_emoji(self): + """The `has_green_checkmark` method should only return `True` if one is present.""" + test_cases = ( + ( + "No reactions", helpers.MockMessage(), False + ), + ( + "No green check mark reactions", + helpers.MockMessage(reactions=[ + helpers.MockReaction(emoji=self.unicode_duck_emoji), + helpers.MockReaction(emoji=self.thumbs_up_emoji) + ]), + False + ), + ( + "Green check mark reaction, but not from our bot", + helpers.MockMessage(reactions=[ + helpers.MockReaction(emoji=self.unicode_duck_emoji), + helpers.MockReaction(emoji=self.checkmark_emoji, users=[self.staff_member]) + ]), + False + ), + ( + "Green check mark reaction, with one from the bot", + helpers.MockMessage(reactions=[ + helpers.MockReaction(emoji=self.unicode_duck_emoji), + helpers.MockReaction(emoji=self.checkmark_emoji, users=[self.staff_member, self.bot.user]) + ]), + True + ) ) - self.bot_reaction = MockReaction( - emoji=self.yellow_ducky_emoji, - user_list=[self.bot_member] + + for description, message, expected_return in test_cases: + actual_return = await self.cog.has_green_checkmark(message) + with self.subTest( + test_case=description, + expected_return=expected_return, + actual_return=actual_return + ): + self.assertEqual(expected_return, actual_return) + + def test_send_webhook_correctly_passes_on_arguments(self): + """The `send_webhook` method should pass the arguments to the webhook correctly.""" + self.cog.webhook = helpers.MockAsyncWebhook() + + content = "fake content" + username = "fake username" + avatar_url = "fake avatar_url" + embed = "fake embed" + + asyncio.run(self.cog.send_webhook(content, username, avatar_url, embed)) + + self.cog.webhook.send.assert_called_once_with( + content=content, + username=username, + avatar_url=avatar_url, + embed=embed ) - self.contrib_reaction = MockReaction( - emoji=self.yellow_ducky_emoji, - user_list=[self.contrib_member] + + def test_send_webhook_logs_when_sending_message_fails(self): + """The `send_webhook` method should catch a `discord.HTTPException` and log accordingly.""" + self.cog.webhook = helpers.MockAsyncWebhook() + self.cog.webhook.send.side_effect = discord.HTTPException(response=MagicMock(), message="Something failed.") + + log = logging.getLogger('bot.cogs.duck_pond') + with self.assertLogs(logger=log, level=logging.ERROR) as log_watcher: + asyncio.run(self.cog.send_webhook()) + + self.assertEqual(len(log_watcher.records), 1) + + [record] = log_watcher.records + self.assertEqual(record.message, "Failed to send a message to the Duck Pool webhook") + self.assertEqual(record.levelno, logging.ERROR) + + def _get_reaction( + self, + emoji: typing.Union[str, helpers.MockEmoji], + staff: int = 0, + nonstaff: int = 0 + ) -> helpers.MockReaction: + staffers = [helpers.MockMember(roles=[self.staff_role]) for _ in range(staff)] + nonstaffers = [helpers.MockMember() for _ in range(nonstaff)] + return helpers.MockReaction(emoji=emoji, users=staffers + nonstaffers) + + @helpers.async_test + async def test_count_ducks_correctly_counts_the_number_of_eligible_duck_emojis(self): + """The `count_ducks` method should return the number of unique staffers who gave a duck.""" + test_cases = ( + # Simple test cases + # A message without reactions should return 0 + ( + "No reactions", + helpers.MockMessage(), + 0 + ), + # A message with a non-duck reaction from a non-staffer should return 0 + ( + "Non-duck reaction from non-staffer", + helpers.MockMessage(reactions=[self._get_reaction(emoji=self.thumbs_up_emoji, nonstaff=1)]), + 0 + ), + # A message with a non-duck reaction from a staffer should return 0 + ( + "Non-duck reaction from staffer", + helpers.MockMessage(reactions=[self._get_reaction(emoji=self.non_duck_custom_emoji, staff=1)]), + 0 + ), + # A message with a non-duck reaction from a non-staffer and staffer should return 0 + ( + "Non-duck reaction from staffer + non-staffer", + helpers.MockMessage(reactions=[self._get_reaction(emoji=self.thumbs_up_emoji, staff=1, nonstaff=1)]), + 0 + ), + # A message with a unicode duck reaction from a non-staffer should return 0 + ( + "Unicode Duck Reaction from non-staffer", + helpers.MockMessage(reactions=[self._get_reaction(emoji=self.unicode_duck_emoji, nonstaff=1)]), + 0 + ), + # A message with a unicode duck reaction from a staffer should return 1 + ( + "Unicode Duck Reaction from staffer", + helpers.MockMessage(reactions=[self._get_reaction(emoji=self.unicode_duck_emoji, staff=1)]), + 1 + ), + # A message with a unicode duck reaction from a non-staffer and staffer should return 1 + ( + "Unicode Duck Reaction from staffer + non-staffer", + helpers.MockMessage(reactions=[self._get_reaction(emoji=self.unicode_duck_emoji, staff=1, nonstaff=1)]), + 1 + ), + # A message with a duckpond duck reaction from a non-staffer should return 0 + ( + "Duckpond Duck Reaction from non-staffer", + helpers.MockMessage(reactions=[self._get_reaction(emoji=self.duck_pond_emoji, nonstaff=1)]), + 0 + ), + # A message with a duckpond duck reaction from a staffer should return 1 + ( + "Duckpond Duck Reaction from staffer", + helpers.MockMessage(reactions=[self._get_reaction(emoji=self.duck_pond_emoji, staff=1)]), + 1 + ), + # A message with a duckpond duck reaction from a non-staffer and staffer should return 1 + ( + "Duckpond Duck Reaction from staffer + non-staffer", + helpers.MockMessage(reactions=[self._get_reaction(emoji=self.duck_pond_emoji, staff=1, nonstaff=1)]), + 1 + ), + + # Complex test cases + # A message with duckpond duck reactions from 3 staffers and 2 non-staffers returns 3 + ( + "Duckpond Duck Reaction from 3 staffers + 2 non-staffers", + helpers.MockMessage(reactions=[self._get_reaction(emoji=self.duck_pond_emoji, staff=3, nonstaff=2)]), + 3 + ), + # A staffer with multiple duck reactions only counts once + ( + "Two different duck reactions from the same staffer", + helpers.MockMessage(reactions=[ + helpers.MockReaction(emoji=self.duck_pond_emoji, users=[self.staff_member]), + helpers.MockReaction(emoji=self.unicode_duck_emoji, users=[self.staff_member]), + ]), + 1 + ), + # A non-string emoji does not count (to test the `isinstance(reaction.emoji, str)` elif) + ( + "Reaction with non-Emoji/str emoij from 3 staffers + 2 non-staffers", + helpers.MockMessage(reactions=[self._get_reaction(emoji=100, staff=3, nonstaff=2)]), + 0 + ), + # We correctly sum when multiple reactions are provided. + ( + "Duckpond Duck Reaction from 3 staffers + 2 non-staffers", + helpers.MockMessage(reactions=[ + self._get_reaction(emoji=self.duck_pond_emoji, staff=3, nonstaff=2), + self._get_reaction(emoji=self.unicode_duck_emoji, staff=4, nonstaff=9), + ]), + 3+4 + ), ) - # Set up a messages - self.checkmark_message = MockMessage(reactions=(self.checkmark_reaction,)) - self.thumbs_up_message = MockMessage(reactions=(self.thumbs_up_reaction,)) - self.yellow_ducky_message = MockMessage(reactions=(self.yellow_ducky_reaction,)) - self.unicode_duck_message = MockMessage(reactions=(self.unicode_duck_reaction_1,)) - self.double_unicode_duck_message = MockMessage( - reactions=(self.unicode_duck_reaction_1, self.unicode_duck_reaction_2) + for description, message, expected_count in test_cases: + actual_count = await self.cog.count_ducks(message) + with self.subTest(test_case=description, expected_count=expected_count, actual_count=actual_count): + self.assertEqual(expected_count, actual_count) + + @helpers.async_test + async def test_relay_message_to_duck_pond_correctly_relays_content_and_attachments(self): + """The `relay_message_to_duck_pond` method should correctly relay message content and attachments.""" + send_webhook_path = f"{MODULE_PATH}.DuckPond.send_webhook" + send_attachments_path = f"{MODULE_PATH}.send_attachments" + + self.cog.webhook = helpers.MockAsyncWebhook() + + test_values = ( + (helpers.MockMessage(clean_content="", attachments=[]), False, False), + (helpers.MockMessage(clean_content="message", attachments=[]), True, False), + (helpers.MockMessage(clean_content="", attachments=["attachment"]), False, True), + (helpers.MockMessage(clean_content="message", attachments=["attachment"]), True, True), ) - self.double_mixed_duck_message = MockMessage( - reactions=(self.unicode_duck_reaction_1, self.yellow_ducky_reaction) + + for message, expect_webhook_call, expect_attachment_call in test_values: + with patch(send_webhook_path, new_callable=helpers.AsyncMock) as send_webhook: + with patch(send_attachments_path, new_callable=helpers.AsyncMock) as send_attachments: + with self.subTest(clean_content=message.clean_content, attachments=message.attachments): + await self.cog.relay_message_to_duck_pond(message) + + self.assertEqual(expect_webhook_call, send_webhook.called) + self.assertEqual(expect_attachment_call, send_attachments.called) + + message.add_reaction.assert_called_once_with(self.checkmark_emoji) + message.reset_mock() + + @patch(f"{MODULE_PATH}.DuckPond.send_webhook", new_callable=helpers.AsyncMock) + @patch(f"{MODULE_PATH}.send_attachments", new_callable=helpers.AsyncMock) + @helpers.async_test + async def test_relay_message_to_duck_pond_handles_send_attachments_exceptions(self, send_attachments, send_webhook): + """The `relay_message_to_duck_pond` method should handle exceptions when calling `send_attachment`.""" + + message = helpers.MockMessage(clean_content="message", attachments=["attachment"]) + side_effects = (discord.errors.Forbidden(MagicMock(), ""), discord.errors.NotFound(MagicMock(), "")) + + self.cog.webhook = helpers.MockAsyncWebhook() + log = logging.getLogger("bot.cogs.duck_pond") + + # Subtests for the first `except` block + for side_effect in side_effects: + send_attachments.side_effect = side_effect + with self.subTest(side_effect=type(side_effect).__name__): + with self.assertNotLogs(logger=log, level=logging.ERROR): + await self.cog.relay_message_to_duck_pond(message) + + self.assertEqual(send_webhook.call_count, 2) + send_webhook.reset_mock() + + # Subtests for the second `except` block + side_effect = discord.HTTPException(MagicMock(), "") + send_attachments.side_effect = side_effect + with self.subTest(side_effect=type(side_effect).__name__): + with self.assertLogs(logger=log, level=logging.ERROR) as log_watcher: + await self.cog.relay_message_to_duck_pond(message) + + send_webhook.assert_called_once_with( + content=message.clean_content, + username=message.author.display_name, + avatar_url=message.author.avatar_url + ) + + self.assertEqual(len(log_watcher.records), 1) + + [record] = log_watcher.records + self.assertEqual(record.message, "Failed to send an attachment to the webhook") + self.assertEqual(record.levelno, logging.ERROR) + + def _raw_reaction_mocks(self, channel_id, message_id, user_id): + """Sets up mocks for tests of the `on_raw_reaction_add` event listener.""" + channel = helpers.MockTextChannel(id=channel_id) + self.bot.get_all_channels.return_value = (channel,) + + message = helpers.MockMessage(id=message_id) + + channel.fetch_message.return_value = message + + member = helpers.MockMember(id=user_id, roles=[self.staff_role]) + message.guild.members = (member,) + + payload = MagicMock(channel_id=channel_id, message_id=message_id, user_id=user_id) + + return channel, message, member, payload + + @helpers.async_test + async def test_on_raw_reaction_add_returns_for_non_relevant_emojis(self): + """The `on_raw_reaction_add` event handler should ignore irrelevant emojis.""" + payload_custom_emoji = MagicMock(label="Non-Duck Custom Emoji") + payload_custom_emoji.emoji.is_custom_emoji.return_value = True + payload_custom_emoji.emoji.id = 12345 + + payload_unicode_emoji = MagicMock(label="Non-Duck Unicode Emoji") + payload_unicode_emoji.emoji.is_custom_emoji.return_value = False + payload_unicode_emoji.emoji.name = self.thumbs_up_emoji + + for payload in (payload_custom_emoji, payload_unicode_emoji): + with self.subTest(case=payload.label), patch(f"{MODULE_PATH}.discord.utils.get") as discord_utils_get: + self.assertIsNone(await self.cog.on_raw_reaction_add(payload)) + discord_utils_get.assert_not_called() + + @helpers.async_test + async def test_on_raw_reaction_add_returns_for_bot_and_non_staff_members(self): + """The `on_raw_reaction_add` event handler should return for bot users or non-staff members.""" + channel_id = 1234 + message_id = 2345 + user_id = 3456 + + channel, message, _, payload = self._raw_reaction_mocks(channel_id, message_id, user_id) + + test_cases = ( + ("non-staff member", helpers.MockMember(id=user_id)), + ("bot staff member", helpers.MockMember(id=user_id, roles=[self.staff_role], bot=True)), ) - self.bot_message = MockMessage(reactions=(self.bot_reaction,)) - self.contrib_message = MockMessage(reactions=(self.contrib_reaction,)) - self.no_reaction_message = MockMessage() - - # Set up some channels - self.text_channel = MockTextChannel(id=self.CHANNEL_ID) - - @staticmethod - def _mock_send_webhook(content, username, avatar_url, embed): - """Mock for the send_webhook method in DuckPond""" - - def test_is_staff_correctly_identifies_staff(self): - """Test that is_staff correctly identifies a staff member.""" - with self.subTest(): - self.assertTrue(self.cog.is_staff(self.admin_member_1)) - self.assertFalse(self.cog.is_staff(self.contrib_member)) - self.assertFalse(self.cog.is_staff(self.no_role_member)) - - def test_has_green_checkmark_correctly_identifies_messages(self): - """Test that has_green_checkmark recognizes messages with checkmarks.""" - with self.subTest(): - self.assertTrue(self.cog.has_green_checkmark(self.checkmark_message)) - self.assertFalse(self.cog.has_green_checkmark(self.thumbs_up_message)) - self.assertFalse(self.cog.has_green_checkmark(self.no_reaction_message)) - - def test_count_custom_duck_emojis(self): - """Test that count_ducks counts custom ducks correctly.""" - count_no_ducks = self.cog.count_ducks(self.thumbs_up_message) - count_one_duck = self.cog.count_ducks(self.yellow_ducky_message) - with self.subTest(): - self.assertEqual(asyncio.run(count_no_ducks), 0) - self.assertEqual(asyncio.run(count_one_duck), 1) - - def test_count_unicode_duck_emojis(self): - """Test that count_ducks counts unicode ducks correctly.""" - count_one_duck = self.cog.count_ducks(self.unicode_duck_message) - count_two_ducks = self.cog.count_ducks(self.double_unicode_duck_message) - - with self.subTest(): - self.assertEqual(asyncio.run(count_one_duck), 1) - self.assertEqual(asyncio.run(count_two_ducks), 2) - - def test_count_mixed_duck_emojis(self): - """Test that count_ducks counts mixed ducks correctly.""" - count_two_ducks = self.cog.count_ducks(self.double_mixed_duck_message) - - with self.subTest(): - self.assertEqual(asyncio.run(count_two_ducks), 2) - - def test_raw_reaction_add_rejects_bot(self): - """Test that send_webhook is not called if the user is a bot.""" - self.text_channel.fetch_message.return_value = self.bot_message - self.bot.get_all_channels.return_value = (self.text_channel,) - - payload = MagicMock( # RawReactionActionEvent - channel_id=self.CHANNEL_ID, - message_id=self.MESSAGE_ID, - user_id=self.BOT_ID, + payload.emoji = self.duck_pond_emoji + + for description, member in test_cases: + message.guild.members = (member, ) + with self.subTest(test_case=description), patch(f"{MODULE_PATH}.DuckPond.has_green_checkmark") as checkmark: + checkmark.side_effect = AssertionError( + "Expected method to return before calling `self.has_green_checkmark`." + ) + self.assertIsNone(await self.cog.on_raw_reaction_add(payload)) + + # Check that we did make it past the payload checks + channel.fetch_message.assert_called_once() + channel.fetch_message.reset_mock() + + @patch(f"{MODULE_PATH}.DuckPond.is_staff") + @patch(f"{MODULE_PATH}.DuckPond.count_ducks", new_callable=helpers.AsyncMock) + def test_on_raw_reaction_add_returns_on_message_with_green_checkmark_placed_by_bot(self, count_ducks, is_staff): + """The `on_raw_reaction_add` event should return when the message has a green check mark placed by the bot.""" + channel_id = 31415926535 + message_id = 27182818284 + user_id = 16180339887 + + channel, message, member, payload = self._raw_reaction_mocks(channel_id, message_id, user_id) + + payload.emoji = helpers.MockPartialEmoji(name=self.unicode_duck_emoji) + payload.emoji.is_custom_emoji.return_value = False + + message.reactions = [helpers.MockReaction(emoji=self.checkmark_emoji, users=[self.bot.user])] + + is_staff.return_value = True + count_ducks.side_effect = AssertionError("Expected method to return before calling `self.count_ducks`") + + self.assertIsNone(asyncio.run(self.cog.on_raw_reaction_add(payload))) + + # Assert that we've made it past `self.is_staff` + is_staff.assert_called_once() + + @patch(f"{MODULE_PATH}.DuckPond.relay_message_to_duck_pond", new_callable=helpers.AsyncMock) + @patch(f"{MODULE_PATH}.DuckPond.count_ducks", new_callable=helpers.AsyncMock) + @helpers.async_test + async def test_on_raw_reaction_add_does_not_relay_below_duck_threshold(self, count_ducks, message_relay): + """The `on_raw_reaction_add` listener should not relay messages or attachments below the duck threshold.""" + test_cases = ( + (constants.DuckPond.threshold-1, False), + (constants.DuckPond.threshold, True), + (constants.DuckPond.threshold+1, True), ) - with self.subTest(): - asyncio.run(self.cog.on_raw_reaction_add(payload)) - self.bot.cog.send_webhook.assert_not_called() + channel, message, member, payload = self._raw_reaction_mocks(channel_id=3, message_id=4, user_id=5) + + payload.emoji = self.duck_pond_emoji + + for duck_count, should_relay in test_cases: + count_ducks.return_value = duck_count + with self.subTest(duck_count=duck_count, should_relay=should_relay): + await self.cog.on_raw_reaction_add(payload) + + # Confirm that we've made it past counting + count_ducks.assert_called_once() + count_ducks.reset_mock() + + # Did we relay a message? + has_relayed = message_relay.called + self.assertEqual(has_relayed, should_relay) + + if should_relay: + message_relay.assert_called_once_with(message) + message_relay.reset_mock() - def test_raw_reaction_add_rejects_non_staff(self): - """Test that send_webhook is not called if the user is not a member of staff.""" - self.text_channel.fetch_message.return_value = self.contrib_message - self.bot.get_all_channels.return_value = (self.text_channel,) + @helpers.async_test + async def test_on_raw_reaction_remove_prevents_removal_of_green_checkmark_depending_on_the_duck_count(self): + """The `on_raw_reaction_remove` listener prevents removal of the check mark on messages with enough ducks.""" + checkmark = helpers.MockPartialEmoji(name=self.checkmark_emoji) - payload = MagicMock( # RawReactionActionEvent - channel_id=self.CHANNEL_ID, - message_id=self.MESSAGE_ID, - user_id=self.CONTRIB_ID, + message = helpers.MockMessage(id=1234) + + channel = helpers.MockTextChannel(id=98765) + channel.fetch_message.return_value = message + + self.bot.get_all_channels.return_value = (channel, ) + + payload = MagicMock(channel_id=channel.id, message_id=message.id, emoji=checkmark) + + test_cases = ( + (constants.DuckPond.threshold - 1, False), + (constants.DuckPond.threshold, True), + (constants.DuckPond.threshold + 1, True), ) + for duck_count, should_readd_checkmark in test_cases: + with patch(f"{MODULE_PATH}.DuckPond.count_ducks", new_callable=helpers.AsyncMock) as count_ducks: + count_ducks.return_value = duck_count + with self.subTest(duck_count=duck_count, should_readd_checkmark=should_readd_checkmark): + await self.cog.on_raw_reaction_remove(payload) + + # Check if we fetched the message + channel.fetch_message.assert_called_once_with(message.id) - with self.subTest(): - asyncio.run(self.cog.on_raw_reaction_add(payload)) - self.bot.cog.send_webhook.assert_not_called() + # Check if we actually counted the number of ducks + count_ducks.assert_called_once_with(message) - def test_raw_reaction_add_sends_message_on_valid_input(self): - """Test that send_webhook is called if payload is valid.""" - self.text_channel.fetch_message.return_value = self.unicode_duck_message - self.bot.get_all_channels.return_value = (self.text_channel,) + has_readded_checkmark = message.add_reaction.called + self.assertEqual(should_readd_checkmark, has_readded_checkmark) - payload = MagicMock( # RawReactionActionEvent - channel_id=self.CHANNEL_ID, - message_id=self.MESSAGE_ID, - user_id=self.ADMIN_ID, + if should_readd_checkmark: + message.add_reaction.assert_called_once_with(self.checkmark_emoji) + message.add_reaction.reset_mock() + + # reset mocks + channel.fetch_message.reset_mock() + count_ducks.reset_mock() + message.reset_mock() + + def test_on_raw_reaction_remove_ignores_removal_of_non_checkmark_reactions(self): + """The `on_raw_reaction_remove` listener should ignore the removal of non-check mark emojis.""" + channel = helpers.MockTextChannel(id=98765) + + channel.fetch_message.side_effect = AssertionError( + "Expected method to return before calling `channel.fetch_message`" ) - with self.subTest(): - asyncio.run(self.cog.on_raw_reaction_add(payload)) - self.bot.cog.send_webhook.assert_called_once() + self.bot.get_all_channels.return_value = (channel, ) + + payload = MagicMock(emoji=helpers.MockPartialEmoji(name=self.thumbs_up_emoji), channel_id=channel.id) - def test_raw_reaction_remove_rejects_non_checkmarks(self): - """A string decoding to numeric characters is a valid user ID.""" - pass + self.assertIsNone(asyncio.run(self.cog.on_raw_reaction_remove(payload))) - def test_raw_reaction_remove_prevents_checkmark_removal(self): - """A string decoding to numeric characters is a valid user ID.""" - pass + channel.fetch_message.assert_not_called() class DuckPondSetupTests(unittest.TestCase): @@ -195,7 +526,7 @@ class DuckPondSetupTests(unittest.TestCase): def test_setup(self): """Setup of the cog should log a message at `INFO` level.""" - bot = MockBot() + bot = helpers.MockBot() log = logging.getLogger('bot.cogs.duck_pond') with self.assertLogs(logger=log, level=logging.INFO) as log_watcher: From e67822a46621d20ff0a1a27de1322b14432e4eb9 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Sat, 16 Nov 2019 17:04:27 +0100 Subject: [PATCH 24/26] Apply suggestions from code review Co-Authored-By: Mark --- tests/bot/cogs/test_duck_pond.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/bot/cogs/test_duck_pond.py b/tests/bot/cogs/test_duck_pond.py index ceefc286f4..8f0c4f0688 100644 --- a/tests/bot/cogs/test_duck_pond.py +++ b/tests/bot/cogs/test_duck_pond.py @@ -269,7 +269,7 @@ async def test_count_ducks_correctly_counts_the_number_of_eligible_duck_emojis(s self._get_reaction(emoji=self.duck_pond_emoji, staff=3, nonstaff=2), self._get_reaction(emoji=self.unicode_duck_emoji, staff=4, nonstaff=9), ]), - 3+4 + 3 + 4 ), ) @@ -310,7 +310,6 @@ async def test_relay_message_to_duck_pond_correctly_relays_content_and_attachmen @helpers.async_test async def test_relay_message_to_duck_pond_handles_send_attachments_exceptions(self, send_attachments, send_webhook): """The `relay_message_to_duck_pond` method should handle exceptions when calling `send_attachment`.""" - message = helpers.MockMessage(clean_content="message", attachments=["attachment"]) side_effects = (discord.errors.Forbidden(MagicMock(), ""), discord.errors.NotFound(MagicMock(), "")) @@ -435,9 +434,9 @@ def test_on_raw_reaction_add_returns_on_message_with_green_checkmark_placed_by_b async def test_on_raw_reaction_add_does_not_relay_below_duck_threshold(self, count_ducks, message_relay): """The `on_raw_reaction_add` listener should not relay messages or attachments below the duck threshold.""" test_cases = ( - (constants.DuckPond.threshold-1, False), + (constants.DuckPond.threshold - 1, False), (constants.DuckPond.threshold, True), - (constants.DuckPond.threshold+1, True), + (constants.DuckPond.threshold + 1, True), ) channel, message, member, payload = self._raw_reaction_mocks(channel_id=3, message_id=4, user_id=5) From 4be37c1486ccb0a8fb680cb6dce51f8ad8028569 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 27 Nov 2019 17:42:48 +0100 Subject: [PATCH 25/26] Move duckpond payload emoji check to method to create testable unit I moved the check that tests if a payload contains a duck emoji to a separate method. This makes it easier to test this part of the code as a separate unit than when it's contained in the larger event listener. In addition, I kaizened the name `relay_message_to_duckpond` to the less verbose `relay_message`; that's already clear enough. --- bot/cogs/duck_pond.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/bot/cogs/duck_pond.py b/bot/cogs/duck_pond.py index 68fb094084..2d25cd17e5 100644 --- a/bot/cogs/duck_pond.py +++ b/bot/cogs/duck_pond.py @@ -91,7 +91,7 @@ async def count_ducks(self, message: Message) -> int: duck_reactors.append(user.id) return duck_count - async def relay_message_to_duck_pond(self, message: Message) -> None: + async def relay_message(self, message: Message) -> None: """Relays the message's content and attachments to the duck pond channel.""" clean_content = message.clean_content @@ -120,6 +120,17 @@ async def relay_message_to_duck_pond(self, message: Message) -> None: await message.add_reaction("✅") + @staticmethod + def _payload_has_duckpond_emoji(payload: RawReactionActionEvent) -> bool: + """Test if the RawReactionActionEvent payload contains a duckpond emoji.""" + if payload.emoji.is_custom_emoji(): + if payload.emoji.id in constants.DuckPond.custom_emojis: + return True + elif payload.emoji.name == "🦆": + return True + + return False + @Cog.listener() async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None: """ @@ -130,10 +141,7 @@ async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None: send the message off to the duck pond. """ # Is the emoji in the reaction a duck? - if payload.emoji.is_custom_emoji(): - if payload.emoji.id not in constants.DuckPond.custom_emojis: - return - elif payload.emoji.name != "🦆": + if not self._payload_has_duckpond_emoji(payload): return channel = discord.utils.get(self.bot.get_all_channels(), id=payload.channel_id) @@ -153,7 +161,7 @@ async def on_raw_reaction_add(self, payload: RawReactionActionEvent) -> None: # If we've got more than the required amount of ducks, send the message to the duck_pond. if duck_count >= constants.DuckPond.threshold: - await self.relay_message_to_duck_pond(message) + await self.relay_message(message) @Cog.listener() async def on_raw_reaction_remove(self, payload: RawReactionActionEvent) -> None: From 80d84e19d6877d2cbf2a6ce5029c1fd96b286b1e Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 27 Nov 2019 17:46:56 +0100 Subject: [PATCH 26/26] Apply review comments to duckpond's unit tests https://github.com/python-discord/bot/pull/621 I've changed to unit tests according to the comments made on the issue. Most changes are straightforward enough, but, for context, see the PR linked above. --- tests/bot/cogs/test_duck_pond.py | 200 ++++++++++++++++++++----------- 1 file changed, 128 insertions(+), 72 deletions(-) diff --git a/tests/bot/cogs/test_duck_pond.py b/tests/bot/cogs/test_duck_pond.py index 8f0c4f0688..b801e86f1a 100644 --- a/tests/bot/cogs/test_duck_pond.py +++ b/tests/bot/cogs/test_duck_pond.py @@ -72,8 +72,7 @@ def test_fetch_webhook_logs_when_unable_to_fetch_webhook(self): self.assertEqual(len(log_watcher.records), 1) - [record] = log_watcher.records - self.assertEqual(record.message, f"Failed to fetch webhook with id `{self.cog.webhook_id}`") + record = log_watcher.records[0] self.assertEqual(record.levelno, logging.ERROR) def test_is_staff_returns_correct_values_based_on_instance_passed(self): @@ -99,15 +98,15 @@ async def test_has_green_checkmark_correctly_detects_presence_of_green_checkmark ( "No green check mark reactions", helpers.MockMessage(reactions=[ - helpers.MockReaction(emoji=self.unicode_duck_emoji), - helpers.MockReaction(emoji=self.thumbs_up_emoji) + helpers.MockReaction(emoji=self.unicode_duck_emoji, users=[self.bot.user]), + helpers.MockReaction(emoji=self.thumbs_up_emoji, users=[self.bot.user]) ]), False ), ( "Green check mark reaction, but not from our bot", helpers.MockMessage(reactions=[ - helpers.MockReaction(emoji=self.unicode_duck_emoji), + helpers.MockReaction(emoji=self.unicode_duck_emoji, users=[self.bot.user]), helpers.MockReaction(emoji=self.checkmark_emoji, users=[self.staff_member]) ]), False @@ -115,7 +114,7 @@ async def test_has_green_checkmark_correctly_detects_presence_of_green_checkmark ( "Green check mark reaction, with one from the bot", helpers.MockMessage(reactions=[ - helpers.MockReaction(emoji=self.unicode_duck_emoji), + helpers.MockReaction(emoji=self.unicode_duck_emoji, users=[self.bot.user]), helpers.MockReaction(emoji=self.checkmark_emoji, users=[self.staff_member, self.bot.user]) ]), True @@ -160,8 +159,7 @@ def test_send_webhook_logs_when_sending_message_fails(self): self.assertEqual(len(log_watcher.records), 1) - [record] = log_watcher.records - self.assertEqual(record.message, "Failed to send a message to the Duck Pool webhook") + record = log_watcher.records[0] self.assertEqual(record.levelno, logging.ERROR) def _get_reaction( @@ -250,10 +248,12 @@ async def test_count_ducks_correctly_counts_the_number_of_eligible_duck_emojis(s # A staffer with multiple duck reactions only counts once ( "Two different duck reactions from the same staffer", - helpers.MockMessage(reactions=[ - helpers.MockReaction(emoji=self.duck_pond_emoji, users=[self.staff_member]), - helpers.MockReaction(emoji=self.unicode_duck_emoji, users=[self.staff_member]), - ]), + helpers.MockMessage( + reactions=[ + helpers.MockReaction(emoji=self.duck_pond_emoji, users=[self.staff_member]), + helpers.MockReaction(emoji=self.unicode_duck_emoji, users=[self.staff_member]), + ] + ), 1 ), # A non-string emoji does not count (to test the `isinstance(reaction.emoji, str)` elif) @@ -265,10 +265,12 @@ async def test_count_ducks_correctly_counts_the_number_of_eligible_duck_emojis(s # We correctly sum when multiple reactions are provided. ( "Duckpond Duck Reaction from 3 staffers + 2 non-staffers", - helpers.MockMessage(reactions=[ - self._get_reaction(emoji=self.duck_pond_emoji, staff=3, nonstaff=2), - self._get_reaction(emoji=self.unicode_duck_emoji, staff=4, nonstaff=9), - ]), + helpers.MockMessage( + reactions=[ + self._get_reaction(emoji=self.duck_pond_emoji, staff=3, nonstaff=2), + self._get_reaction(emoji=self.unicode_duck_emoji, staff=4, nonstaff=9), + ] + ), 3 + 4 ), ) @@ -279,8 +281,8 @@ async def test_count_ducks_correctly_counts_the_number_of_eligible_duck_emojis(s self.assertEqual(expected_count, actual_count) @helpers.async_test - async def test_relay_message_to_duck_pond_correctly_relays_content_and_attachments(self): - """The `relay_message_to_duck_pond` method should correctly relay message content and attachments.""" + async def test_relay_message_correctly_relays_content_and_attachments(self): + """The `relay_message` method should correctly relay message content and attachments.""" send_webhook_path = f"{MODULE_PATH}.DuckPond.send_webhook" send_attachments_path = f"{MODULE_PATH}.send_attachments" @@ -297,41 +299,47 @@ async def test_relay_message_to_duck_pond_correctly_relays_content_and_attachmen with patch(send_webhook_path, new_callable=helpers.AsyncMock) as send_webhook: with patch(send_attachments_path, new_callable=helpers.AsyncMock) as send_attachments: with self.subTest(clean_content=message.clean_content, attachments=message.attachments): - await self.cog.relay_message_to_duck_pond(message) + await self.cog.relay_message(message) self.assertEqual(expect_webhook_call, send_webhook.called) self.assertEqual(expect_attachment_call, send_attachments.called) message.add_reaction.assert_called_once_with(self.checkmark_emoji) - message.reset_mock() - @patch(f"{MODULE_PATH}.DuckPond.send_webhook", new_callable=helpers.AsyncMock) @patch(f"{MODULE_PATH}.send_attachments", new_callable=helpers.AsyncMock) @helpers.async_test - async def test_relay_message_to_duck_pond_handles_send_attachments_exceptions(self, send_attachments, send_webhook): - """The `relay_message_to_duck_pond` method should handle exceptions when calling `send_attachment`.""" + async def test_relay_message_handles_irretrievable_attachment_exceptions(self, send_attachments): + """The `relay_message` method should handle irretrievable attachments.""" message = helpers.MockMessage(clean_content="message", attachments=["attachment"]) side_effects = (discord.errors.Forbidden(MagicMock(), ""), discord.errors.NotFound(MagicMock(), "")) self.cog.webhook = helpers.MockAsyncWebhook() log = logging.getLogger("bot.cogs.duck_pond") - # Subtests for the first `except` block for side_effect in side_effects: send_attachments.side_effect = side_effect - with self.subTest(side_effect=type(side_effect).__name__): - with self.assertNotLogs(logger=log, level=logging.ERROR): - await self.cog.relay_message_to_duck_pond(message) + with patch(f"{MODULE_PATH}.DuckPond.send_webhook", new_callable=helpers.AsyncMock) as send_webhook: + with self.subTest(side_effect=type(side_effect).__name__): + with self.assertNotLogs(logger=log, level=logging.ERROR): + await self.cog.relay_message(message) + + self.assertEqual(send_webhook.call_count, 2) + + @patch(f"{MODULE_PATH}.DuckPond.send_webhook", new_callable=helpers.AsyncMock) + @patch(f"{MODULE_PATH}.send_attachments", new_callable=helpers.AsyncMock) + @helpers.async_test + async def test_relay_message_handles_attachment_http_error(self, send_attachments, send_webhook): + """The `relay_message` method should handle irretrievable attachments.""" + message = helpers.MockMessage(clean_content="message", attachments=["attachment"]) - self.assertEqual(send_webhook.call_count, 2) - send_webhook.reset_mock() + self.cog.webhook = helpers.MockAsyncWebhook() + log = logging.getLogger("bot.cogs.duck_pond") - # Subtests for the second `except` block side_effect = discord.HTTPException(MagicMock(), "") send_attachments.side_effect = side_effect with self.subTest(side_effect=type(side_effect).__name__): with self.assertLogs(logger=log, level=logging.ERROR) as log_watcher: - await self.cog.relay_message_to_duck_pond(message) + await self.cog.relay_message(message) send_webhook.assert_called_once_with( content=message.clean_content, @@ -341,10 +349,75 @@ async def test_relay_message_to_duck_pond_handles_send_attachments_exceptions(se self.assertEqual(len(log_watcher.records), 1) - [record] = log_watcher.records - self.assertEqual(record.message, "Failed to send an attachment to the webhook") + record = log_watcher.records[0] self.assertEqual(record.levelno, logging.ERROR) + def _mock_payload(self, label: str, is_custom_emoji: bool, id_: int, emoji_name: str): + """Creates a mock `on_raw_reaction_add` payload with the specified emoji data.""" + payload = MagicMock(name=label) + payload.emoji.is_custom_emoji.return_value = is_custom_emoji + payload.emoji.id = id_ + payload.emoji.name = emoji_name + return payload + + @helpers.async_test + async def test_payload_has_duckpond_emoji_correctly_detects_relevant_emojis(self): + """The `on_raw_reaction_add` event handler should ignore irrelevant emojis.""" + test_values = ( + # Custom Emojis + ( + self._mock_payload( + label="Custom Duckpond Emoji", + is_custom_emoji=True, + id_=constants.DuckPond.custom_emojis[0], + emoji_name="" + ), + True + ), + ( + self._mock_payload( + label="Custom Non-Duckpond Emoji", + is_custom_emoji=True, + id_=123, + emoji_name="" + ), + False + ), + # Unicode Emojis + ( + self._mock_payload( + label="Unicode Duck Emoji", + is_custom_emoji=False, + id_=1, + emoji_name=self.unicode_duck_emoji + ), + True + ), + ( + self._mock_payload( + label="Unicode Non-Duck Emoji", + is_custom_emoji=False, + id_=1, + emoji_name=self.thumbs_up_emoji + ), + False + ), + ) + + for payload, expected_return in test_values: + actual_return = self.cog._payload_has_duckpond_emoji(payload) + with self.subTest(case=payload._mock_name, expected_return=expected_return, actual_return=actual_return): + self.assertEqual(expected_return, actual_return) + + @patch(f"{MODULE_PATH}.discord.utils.get") + @patch(f"{MODULE_PATH}.DuckPond._payload_has_duckpond_emoji", new=MagicMock(return_value=False)) + def test_on_raw_reaction_add_returns_early_with_payload_without_duck_emoji(self, utils_get): + """The `on_raw_reaction_add` method should return early if the payload does not contain a duck emoji.""" + self.assertIsNone(asyncio.run(self.cog.on_raw_reaction_add(payload=MagicMock()))) + + # Ensure we've returned before making an unnecessary API call in the lines of code after the emoji check + utils_get.assert_not_called() + def _raw_reaction_mocks(self, channel_id, message_id, user_id): """Sets up mocks for tests of the `on_raw_reaction_add` event listener.""" channel = helpers.MockTextChannel(id=channel_id) @@ -361,22 +434,6 @@ def _raw_reaction_mocks(self, channel_id, message_id, user_id): return channel, message, member, payload - @helpers.async_test - async def test_on_raw_reaction_add_returns_for_non_relevant_emojis(self): - """The `on_raw_reaction_add` event handler should ignore irrelevant emojis.""" - payload_custom_emoji = MagicMock(label="Non-Duck Custom Emoji") - payload_custom_emoji.emoji.is_custom_emoji.return_value = True - payload_custom_emoji.emoji.id = 12345 - - payload_unicode_emoji = MagicMock(label="Non-Duck Unicode Emoji") - payload_unicode_emoji.emoji.is_custom_emoji.return_value = False - payload_unicode_emoji.emoji.name = self.thumbs_up_emoji - - for payload in (payload_custom_emoji, payload_unicode_emoji): - with self.subTest(case=payload.label), patch(f"{MODULE_PATH}.discord.utils.get") as discord_utils_get: - self.assertIsNone(await self.cog.on_raw_reaction_add(payload)) - discord_utils_get.assert_not_called() - @helpers.async_test async def test_on_raw_reaction_add_returns_for_bot_and_non_staff_members(self): """The `on_raw_reaction_add` event handler should return for bot users or non-staff members.""" @@ -428,10 +485,8 @@ def test_on_raw_reaction_add_returns_on_message_with_green_checkmark_placed_by_b # Assert that we've made it past `self.is_staff` is_staff.assert_called_once() - @patch(f"{MODULE_PATH}.DuckPond.relay_message_to_duck_pond", new_callable=helpers.AsyncMock) - @patch(f"{MODULE_PATH}.DuckPond.count_ducks", new_callable=helpers.AsyncMock) @helpers.async_test - async def test_on_raw_reaction_add_does_not_relay_below_duck_threshold(self, count_ducks, message_relay): + async def test_on_raw_reaction_add_does_not_relay_below_duck_threshold(self): """The `on_raw_reaction_add` listener should not relay messages or attachments below the duck threshold.""" test_cases = ( (constants.DuckPond.threshold - 1, False), @@ -444,21 +499,21 @@ async def test_on_raw_reaction_add_does_not_relay_below_duck_threshold(self, cou payload.emoji = self.duck_pond_emoji for duck_count, should_relay in test_cases: - count_ducks.return_value = duck_count - with self.subTest(duck_count=duck_count, should_relay=should_relay): - await self.cog.on_raw_reaction_add(payload) + with patch(f"{MODULE_PATH}.DuckPond.relay_message", new_callable=helpers.AsyncMock) as relay_message: + with patch(f"{MODULE_PATH}.DuckPond.count_ducks", new_callable=helpers.AsyncMock) as count_ducks: + count_ducks.return_value = duck_count + with self.subTest(duck_count=duck_count, should_relay=should_relay): + await self.cog.on_raw_reaction_add(payload) - # Confirm that we've made it past counting - count_ducks.assert_called_once() - count_ducks.reset_mock() + # Confirm that we've made it past counting + count_ducks.assert_called_once() - # Did we relay a message? - has_relayed = message_relay.called - self.assertEqual(has_relayed, should_relay) + # Did we relay a message? + has_relayed = relay_message.called + self.assertEqual(has_relayed, should_relay) - if should_relay: - message_relay.assert_called_once_with(message) - message_relay.reset_mock() + if should_relay: + relay_message.assert_called_once_with(message) @helpers.async_test async def test_on_raw_reaction_remove_prevents_removal_of_green_checkmark_depending_on_the_duck_count(self): @@ -479,10 +534,10 @@ async def test_on_raw_reaction_remove_prevents_removal_of_green_checkmark_depend (constants.DuckPond.threshold, True), (constants.DuckPond.threshold + 1, True), ) - for duck_count, should_readd_checkmark in test_cases: + for duck_count, should_re_add_checkmark in test_cases: with patch(f"{MODULE_PATH}.DuckPond.count_ducks", new_callable=helpers.AsyncMock) as count_ducks: count_ducks.return_value = duck_count - with self.subTest(duck_count=duck_count, should_readd_checkmark=should_readd_checkmark): + with self.subTest(duck_count=duck_count, should_re_add_checkmark=should_re_add_checkmark): await self.cog.on_raw_reaction_remove(payload) # Check if we fetched the message @@ -491,16 +546,15 @@ async def test_on_raw_reaction_remove_prevents_removal_of_green_checkmark_depend # Check if we actually counted the number of ducks count_ducks.assert_called_once_with(message) - has_readded_checkmark = message.add_reaction.called - self.assertEqual(should_readd_checkmark, has_readded_checkmark) + has_re_added_checkmark = message.add_reaction.called + self.assertEqual(should_re_add_checkmark, has_re_added_checkmark) - if should_readd_checkmark: + if should_re_add_checkmark: message.add_reaction.assert_called_once_with(self.checkmark_emoji) message.add_reaction.reset_mock() # reset mocks channel.fetch_message.reset_mock() - count_ducks.reset_mock() message.reset_mock() def test_on_raw_reaction_remove_ignores_removal_of_non_checkmark_reactions(self): @@ -530,7 +584,9 @@ def test_setup(self): with self.assertLogs(logger=log, level=logging.INFO) as log_watcher: duck_pond.setup(bot) - line = log_watcher.output[0] + + self.assertEqual(len(log_watcher.records), 1) + record = log_watcher.records[0] + self.assertEqual(record.levelno, logging.INFO) bot.add_cog.assert_called_once() - self.assertIn("Cog loaded: DuckPond", line)