From 38d9e705c77003008f1562188cba079a4245061b Mon Sep 17 00:00:00 2001 From: kwzrd Date: Fri, 1 Nov 2019 23:01:45 +0100 Subject: Add unit test for links antispam rule --- tests/bot/rules/test_links.py | 93 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 tests/bot/rules/test_links.py diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py new file mode 100644 index 000000000..f71a8e6bb --- /dev/null +++ b/tests/bot/rules/test_links.py @@ -0,0 +1,93 @@ +import unittest +from typing import List, NamedTuple, Tuple + +from bot.rules import links +from tests.helpers import async_test + + +class FakeMessage(NamedTuple): + author: str + content: str + + +class Case(NamedTuple): + recent_messages: List[FakeMessage] + relevant_messages: Tuple[FakeMessage] + culprit: Tuple[str] + total_links: int + + +def msg(author: str, total_links: int) -> FakeMessage: + """Makes a message with `total_links` links.""" + content = " ".join(["https://pydis.com"] * total_links) + return FakeMessage(author=author, content=content) + + +class LinksTests(unittest.TestCase): + """Tests applying the `links` rule.""" + + def setUp(self): + self.config = { + "max": 2, + "interval": 10 + } + + @async_test + async def test_links_within_limit(self): + """Messages with an allowed amount of links.""" + cases = ( + [msg("bob", 0)], + [msg("bob", 2)], + [msg("bob", 3)], + [msg("bob", 3), msg("alice", 3)] + ) + + for recent_messages in cases: + last_message = recent_messages[0] + with self.subTest( + last_message=last_message, + recent_messages=recent_messages, + config=self.config + ): + self.assertIsNone( + await links.apply(last_message, recent_messages, self.config) + ) + + @async_test + async def test_links_exceeding_limit(self): + """Messages with a a higher than allowed amount of links.""" + cases = ( + Case( + [msg("bob", 1), msg("bob", 2)], + (msg("bob", 1), msg("bob", 2)), + ("bob",), + 3 + ), + Case( + [msg("alice", 2), msg("bob", 3), msg("alice", 1)], + (msg("alice", 2), msg("alice", 1)), + ("alice",), + 3 + ) + ) + + for recent_messages, relevant_messages, culprit, total_links in cases: + last_message = recent_messages[0] + + with self.subTest( + last_message=last_message, + recent_messages=recent_messages, + relevant_messages=relevant_messages, + culprit=culprit, + total_links=total_links, + config=self.config + ): + desired_output = ( + f"sent {total_links} links in {self.config['interval']}s", + culprit, + relevant_messages + ) + self.assertTupleEqual( + await links.apply(last_message, recent_messages, self.config), + desired_output + ) -- cgit v1.2.3 From a21670459953599c8f13286595520e033da4199a Mon Sep 17 00:00:00 2001 From: kwzrd Date: Fri, 1 Nov 2019 23:24:20 +0100 Subject: Add two more test cases for links rule unit test --- tests/bot/rules/test_links.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index f71a8e6bb..f043495cf 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -39,6 +39,7 @@ class LinksTests(unittest.TestCase): [msg("bob", 0)], [msg("bob", 2)], [msg("bob", 3)], + [msg("bob", 1), msg("bob", 1)], [msg("bob", 3), msg("alice", 3)] ) @@ -63,6 +64,12 @@ class LinksTests(unittest.TestCase): ("bob",), 3 ), + Case( + [msg("alice", 1), msg("alice", 1), msg("alice", 1)], + (msg("alice", 1), msg("alice", 1), msg("alice", 1)), + ("alice",), + 3 + ), Case( [msg("alice", 2), msg("bob", 3), msg("alice", 1)], (msg("alice", 2), msg("alice", 1)), -- cgit v1.2.3 From 9e16b0cf17e93295167956d31ad9f92396ca3402 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 2 Nov 2019 01:19:07 +0100 Subject: Annotate unclear test cases with inline comments --- tests/bot/rules/test_links.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index f043495cf..a04b90c13 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -38,9 +38,9 @@ class LinksTests(unittest.TestCase): cases = ( [msg("bob", 0)], [msg("bob", 2)], - [msg("bob", 3)], + [msg("bob", 3)], # Filter only applies if len(messages_with_links) > 1 [msg("bob", 1), msg("bob", 1)], - [msg("bob", 3), msg("alice", 3)] + [msg("bob", 3), msg("alice", 3)] # Only messages from latest author count ) for recent_messages in cases: -- cgit v1.2.3 From f6ed29c8692759dc0c8b003ebf2d4ce3d5ca6ed9 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 2 Nov 2019 01:34:46 +0100 Subject: Adjust case to only test a single aspect --- tests/bot/rules/test_links.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index a04b90c13..95f107f91 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -40,7 +40,7 @@ class LinksTests(unittest.TestCase): [msg("bob", 2)], [msg("bob", 3)], # Filter only applies if len(messages_with_links) > 1 [msg("bob", 1), msg("bob", 1)], - [msg("bob", 3), msg("alice", 3)] # Only messages from latest author count + [msg("bob", 2), msg("alice", 2)] # Only messages from latest author count ) for recent_messages in cases: -- cgit v1.2.3 From 4dccacaa5794751546336de1aa42cd5a9095b706 Mon Sep 17 00:00:00 2001 From: Derek Date: Mon, 4 Nov 2019 17:48:48 -0500 Subject: Reword periodic #checkpoint message --- bot/cogs/verification.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 5b115deaa..3d85edae6 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -31,7 +31,7 @@ If you'd like to unsubscribe from the announcement notifications, simply send `! PERIODIC_PING = ( f"@everyone To verify that you have read our rules, please type `{BotConfig.prefix}accept`." - f" Ping <@&{Roles.admin}> if you encounter any problems during the verification process." + f" If you encounter any problems during the verification process, ping the <@&{Roles.admin}> role in this channel." ) -- cgit v1.2.3 From a916ec15867187907abe9214eb4bfa12a9e982f4 Mon Sep 17 00:00:00 2001 From: kosayoda Date: Fri, 8 Nov 2019 11:58:21 +0800 Subject: Allow helpers to nominate people --- bot/cogs/watchchannels/talentpool.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/bot/cogs/watchchannels/talentpool.py b/bot/cogs/watchchannels/talentpool.py index 176c6f760..4ec42dcc1 100644 --- a/bot/cogs/watchchannels/talentpool.py +++ b/bot/cogs/watchchannels/talentpool.py @@ -7,14 +7,13 @@ from discord import Color, Embed, Member, User from discord.ext.commands import Bot, Cog, Context, group from bot.api import ResponseCodeError -from bot.constants import Channels, Guild, Roles, Webhooks +from bot.constants import Channels, Guild, MODERATION_ROLES, STAFF_ROLES, Webhooks from bot.decorators import with_role from bot.pagination import LinePaginator from bot.utils import time from .watchchannel import WatchChannel, proxy_user log = logging.getLogger(__name__) -STAFF_ROLES = Roles.owner, Roles.admin, Roles.moderator, Roles.helpers # <- In constants after the merge? class TalentPool(WatchChannel, Cog, name="Talentpool"): @@ -31,13 +30,13 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): ) @group(name='talentpool', aliases=('tp', 'talent', 'nomination', 'n'), invoke_without_command=True) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def nomination_group(self, ctx: Context) -> None: """Highlights the activity of helper nominees by relaying their messages to the talent pool channel.""" await ctx.invoke(self.bot.get_command("help"), "talentpool") @nomination_group.command(name='watched', aliases=('all', 'list')) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def watched_command(self, ctx: Context, update_cache: bool = True) -> None: """ Shows the users that are currently being monitored in the talent pool. @@ -48,7 +47,7 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): await self.list_watched_users(ctx, update_cache) @nomination_group.command(name='watch', aliases=('w', 'add', 'a')) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*STAFF_ROLES) async def watch_command(self, ctx: Context, user: Union[Member, User, proxy_user], *, reason: str) -> None: """ Relay messages sent by the given `user` to the `#talent-pool` channel. @@ -113,7 +112,7 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): await ctx.send(msg) @nomination_group.command(name='history', aliases=('info', 'search')) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def history_command(self, ctx: Context, user: Union[User, proxy_user]) -> None: """Shows the specified user's nomination history.""" result = await self.bot.api_client.get( @@ -142,7 +141,7 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): ) @nomination_group.command(name='unwatch', aliases=('end', )) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def unwatch_command(self, ctx: Context, user: Union[User, proxy_user], *, reason: str) -> None: """ Ends the active nomination of the specified user with the given reason. @@ -170,13 +169,13 @@ class TalentPool(WatchChannel, Cog, name="Talentpool"): self._remove_user(user.id) @nomination_group.group(name='edit', aliases=('e',), invoke_without_command=True) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def nomination_edit_group(self, ctx: Context) -> None: """Commands to edit nominations.""" await ctx.invoke(self.bot.get_command("help"), "talentpool", "edit") @nomination_edit_group.command(name='reason') - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def edit_reason_command(self, ctx: Context, nomination_id: int, *, reason: str) -> None: """ Edits the reason/unnominate reason for the nomination with the given `id` depending on the status. -- cgit v1.2.3 From 25f7f6019772e22072a2e1d16d6c4ff57862022f Mon Sep 17 00:00:00 2001 From: kosayoda Date: Fri, 8 Nov 2019 12:11:47 +0800 Subject: Refactor bigbrother to use constants --- bot/cogs/watchchannels/bigbrother.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bot/cogs/watchchannels/bigbrother.py b/bot/cogs/watchchannels/bigbrother.py index c516508ca..49783bb09 100644 --- a/bot/cogs/watchchannels/bigbrother.py +++ b/bot/cogs/watchchannels/bigbrother.py @@ -6,7 +6,7 @@ from discord import User from discord.ext.commands import Bot, Cog, Context, group from bot.cogs.moderation.utils import post_infraction -from bot.constants import Channels, Roles, Webhooks +from bot.constants import Channels, MODERATION_ROLES, Webhooks from bot.decorators import with_role from .watchchannel import WatchChannel, proxy_user @@ -27,13 +27,13 @@ class BigBrother(WatchChannel, Cog, name="Big Brother"): ) @group(name='bigbrother', aliases=('bb',), invoke_without_command=True) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def bigbrother_group(self, ctx: Context) -> None: """Monitors users by relaying their messages to the Big Brother watch channel.""" await ctx.invoke(self.bot.get_command("help"), "bigbrother") @bigbrother_group.command(name='watched', aliases=('all', 'list')) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def watched_command(self, ctx: Context, update_cache: bool = True) -> None: """ Shows the users that are currently being monitored by Big Brother. @@ -44,7 +44,7 @@ class BigBrother(WatchChannel, Cog, name="Big Brother"): await self.list_watched_users(ctx, update_cache) @bigbrother_group.command(name='watch', aliases=('w',)) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def watch_command(self, ctx: Context, user: Union[User, proxy_user], *, reason: str) -> None: """ Relay messages sent by the given `user` to the `#big-brother` channel. @@ -91,7 +91,7 @@ class BigBrother(WatchChannel, Cog, name="Big Brother"): await ctx.send(msg) @bigbrother_group.command(name='unwatch', aliases=('uw',)) - @with_role(Roles.owner, Roles.admin, Roles.moderator) + @with_role(*MODERATION_ROLES) async def unwatch_command(self, ctx: Context, user: Union[User, proxy_user], *, reason: str) -> None: """Stop relaying messages by the given `user`.""" active_watches = await self.bot.api_client.get( -- cgit v1.2.3 From 9000d92ab8ceaf855d23c0e6dfdb09d05a40e59a Mon Sep 17 00:00:00 2001 From: kwzrd Date: Fri, 8 Nov 2019 22:07:00 +0100 Subject: Add whitespace for readability, consistency & allure --- tests/bot/rules/test_links.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index 95f107f91..40336beb0 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -45,6 +45,7 @@ class LinksTests(unittest.TestCase): for recent_messages in cases: last_message = recent_messages[0] + with self.subTest( last_message=last_message, recent_messages=recent_messages, -- cgit v1.2.3 From 760f265339b1462b60ea7d5af1fd8d4476c134f0 Mon Sep 17 00:00:00 2001 From: kwzrd <44734341+kwzrd@users.noreply.github.com> Date: Fri, 8 Nov 2019 22:52:47 +0100 Subject: Update docstring to use asterisks when referring to argument names Co-Authored-By: Kyle Stanley --- tests/bot/rules/test_links.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index 40336beb0..be832843b 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -18,7 +18,7 @@ class Case(NamedTuple): def msg(author: str, total_links: int) -> FakeMessage: - """Makes a message with `total_links` links.""" + """Makes a message with *total_links* links.""" content = " ".join(["https://pydis.com"] * total_links) return FakeMessage(author=author, content=content) -- cgit v1.2.3 From 01ab4ad37dc68f38dd0fd15487dbff9bbd58c24e Mon Sep 17 00:00:00 2001 From: Derek Date: Sat, 9 Nov 2019 20:08:08 -0500 Subject: Forward user/role pings in checkpoint to mod-alerts [kaizen] limit on_message listener to verification channel --- bot/cogs/verification.py | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 3d85edae6..cd4311916 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -1,12 +1,16 @@ import logging from datetime import datetime -from discord import Message, NotFound, Object +from discord import Colour, Message, NotFound, Object from discord.ext import tasks from discord.ext.commands import Bot, Cog, Context, command from bot.cogs.moderation import ModLog -from bot.constants import Bot as BotConfig, Channels, Event, Roles +from bot.constants import ( + Bot as BotConfig, + Channels, Colours, Event, + Filter, Icons, Roles +) from bot.decorators import InChannelCheckFailure, in_channel, without_role log = logging.getLogger(__name__) @@ -53,6 +57,34 @@ class Verification(Cog): if message.author.bot: return # They're a bot, ignore + if message.channel.id != Channels.verification: + return # Only listen for #checkpoint messages + + # if a user mentions a role or guild member + # alert the mods in mod-alerts channel + if message.mentions or message.role_mentions: + log.debug( + f"{message.author} mentioned one or more users " + f"and/or roles in {message.channel.name}" + ) + + embed_text = ( + f"{message.author.mention} sent a message in " + f"{message.channel.mention} that contained user and/or role mentions." + f"\n\n**Original message:**\n>>> {message.content}" + ) + + # Send pretty mod log embed to mod-alerts + await self.mod_log.send_log_message( + icon_url=Icons.filtering, + colour=Colour(Colours.soft_red), + title=f"User/Role mentioned in {message.channel.name}", + text=embed_text, + thumbnail=message.author.avatar_url_as(static_format="png"), + channel_id=Channels.mod_alerts, + ping_everyone=Filter.ping_everyone, + ) + ctx = await self.bot.get_context(message) # type: Context if ctx.command is not None and ctx.command.name == "accept": -- cgit v1.2.3 From 0425e6c49ecf091871a06762a064755ea3dda04c Mon Sep 17 00:00:00 2001 From: Derek Date: Sat, 9 Nov 2019 21:07:11 -0500 Subject: [kaizen] Remove now duplicate channel check Unindent subsequent lines after check --- bot/cogs/verification.py | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index cd4311916..b5e8d4357 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -90,27 +90,26 @@ class Verification(Cog): if ctx.command is not None and ctx.command.name == "accept": return # They used the accept command - if ctx.channel.id == Channels.verification: # We're in the verification channel - for role in ctx.author.roles: - if role.id == Roles.verified: - log.warning(f"{ctx.author} posted '{ctx.message.content}' " - "in the verification channel, but is already verified.") - return # They're already verified - - log.debug(f"{ctx.author} posted '{ctx.message.content}' in the verification " - "channel. We are providing instructions how to verify.") - await ctx.send( - f"{ctx.author.mention} Please type `!accept` to verify that you accept our rules, " - f"and gain access to the rest of the server.", - delete_after=20 - ) + for role in ctx.author.roles: + if role.id == Roles.verified: + log.warning(f"{ctx.author} posted '{ctx.message.content}' " + "in the verification channel, but is already verified.") + return # They're already verified + + log.debug(f"{ctx.author} posted '{ctx.message.content}' in the verification " + "channel. We are providing instructions how to verify.") + await ctx.send( + f"{ctx.author.mention} Please type `!accept` to verify that you accept our rules, " + f"and gain access to the rest of the server.", + delete_after=20 + ) - log.trace(f"Deleting the message posted by {ctx.author}") + log.trace(f"Deleting the message posted by {ctx.author}") - try: - await ctx.message.delete() - except NotFound: - log.trace("No message found, it must have been deleted by another bot.") + try: + await ctx.message.delete() + except NotFound: + log.trace("No message found, it must have been deleted by another bot.") @command(name='accept', aliases=('verify', 'verified', 'accepted'), hidden=True) @without_role(Roles.verified) -- cgit v1.2.3 From dceafb83e829548638e8589c88f80364e8009821 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 13 Nov 2019 15:32:13 +0100 Subject: Prevent unwanted logging while running tests Previously, logging messages would output to std.out. when running individual test files (instead of running the entire suite). To prevent this, I've added a `for`-loop to `tests.helpers` that sets the level of all registered loggers to `CRITICAL`. The reason for adding this to `tests.helpers` is simple: It's the most common file to be imported in individual tests, increasing the chance of the code being run for individual test files. A small downside of this way of handling logging is that when we are trying to assert logging messages are being emitted, we need to set the logger explicitly in the `self.assertLogs` context manager. This is a small downside, though, and probably good practice anyway. There was one test in `tests.bot.test_api` that did not do this, so I have changed this to make the test compatible with the new set-up. --- tests/bot/test_api.py | 4 +++- tests/helpers.py | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/bot/test_api.py b/tests/bot/test_api.py index e0ede0eb1..5a88adc5c 100644 --- a/tests/bot/test_api.py +++ b/tests/bot/test_api.py @@ -121,7 +121,9 @@ class LoggingHandlerTests(LoggingTestCase): def test_schedule_queued_tasks_for_nonempty_queue(self): """`APILoggingHandler` should schedule logs when the queue is not empty.""" - with self.assertLogs(level=logging.DEBUG) as logs, patch('asyncio.create_task') as create_task: + log = logging.getLogger("bot.api") + + with self.assertLogs(logger=log, level=logging.DEBUG) as logs, patch('asyncio.create_task') as create_task: self.log_handler.queue = [555] self.log_handler.schedule_queued_tasks() self.assertListEqual(self.log_handler.queue, []) diff --git a/tests/helpers.py b/tests/helpers.py index 8496ba031..8d661513d 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio import functools import inspect +import logging import unittest.mock from typing import Any, Iterable, Optional @@ -10,6 +11,16 @@ import discord from discord.ext.commands import Bot, Context +for logger in logging.Logger.manager.loggerDict.values(): + # Set all loggers to CRITICAL by default to prevent screen clutter during testing + + if not isinstance(logger, logging.Logger): + # There might be some logging.PlaceHolder objects in there + continue + + logger.setLevel(logging.CRITICAL) + + def async_test(wrapped): """ Run a test case via asyncio. -- cgit v1.2.3 From bf7720f16fa69716f15b16e3dcd0f20c186958b8 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 13 Nov 2019 16:06:51 +0100 Subject: Allow `name` attribute to be set during Mock init The `name` keyword argument has a special meaning for the default mockobjects provided by `unittest.mock`. This means that by default, the common d.py `name` attribute can't be set during initalization of one of our custom Mock-objects by passing it to the constructor. Since it's unlikely for us to make use of the special `name` feature of mocks and more likely to want to set the d.py `name` attribute, I added special handling of the `name` kwarg. --- tests/helpers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/helpers.py b/tests/helpers.py index 8d661513d..5dc7a0d2f 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -74,7 +74,10 @@ class CustomMockMixin: child_mock_type = unittest.mock.MagicMock def __init__(self, spec: Any = None, **kwargs): + name = kwargs.pop('name', None) # `name` has special meaning for Mock classes, so we need to set it manually. super().__init__(spec=spec, **kwargs) + if name: + self.name = name if spec: self._extract_coroutine_methods_from_spec_instance(spec) -- cgit v1.2.3 From 36e9de480dcabb3c844090a4fd87561534536c04 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 13 Nov 2019 11:24:58 +0100 Subject: Prevent setting unknown attributes on d.py mocks Our custom `discord.py` now follow the specifications of the object they are mocking more strictly by using the `spec_set` instead of the `spec` kwarg to initialize the specifications. This means that trying to set an attribute that does not follow the specifications will now also result in an `AttributeError`. To make sure we are not trying to set illegal attributes during the default initialization of the mock objects, I've changed the way we handle default values of parameters. This does introduce a breaking change: Instead of passing a `suffix_id`, the `id` attribute should now be passed using the exact name. `id`. This commit also makes sure existing tests follow this change. --- tests/bot/cogs/test_information.py | 58 ++++++++++----------- tests/bot/cogs/test_token_remover.py | 2 +- tests/bot/utils/test_checks.py | 6 +-- tests/helpers.py | 94 +++++++++++++--------------------- tests/test_helpers.py | 99 ++++++++++++------------------------ 5 files changed, 101 insertions(+), 158 deletions(-) diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 5c34541d8..4496a2ae0 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -19,7 +19,7 @@ class InformationCogTests(unittest.TestCase): @classmethod def setUpClass(cls): - cls.moderator_role = helpers.MockRole(name="Moderator", role_id=constants.Roles.moderator) + cls.moderator_role = helpers.MockRole(name="Moderator", id=constants.Roles.moderator) def setUp(self): """Sets up fresh objects for each test.""" @@ -54,7 +54,7 @@ class InformationCogTests(unittest.TestCase): """Tests the `role info` command.""" dummy_role = helpers.MockRole( name="Dummy", - role_id=112233445566778899, + id=112233445566778899, colour=discord.Colour.blurple(), position=10, members=[self.ctx.author], @@ -63,7 +63,7 @@ class InformationCogTests(unittest.TestCase): admin_role = helpers.MockRole( name="Admins", - role_id=998877665544332211, + id=998877665544332211, colour=discord.Colour.red(), position=3, members=[self.ctx.author], @@ -176,7 +176,7 @@ class UserInfractionHelperMethodTests(unittest.TestCase): self.bot = helpers.MockBot() self.bot.api_client.get = helpers.AsyncMock() self.cog = information.Information(self.bot) - self.member = helpers.MockMember(user_id=1234) + self.member = helpers.MockMember(id=1234) def test_user_command_helper_method_get_requests(self): """The helper methods should form the correct get requests.""" @@ -351,7 +351,7 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) def test_create_user_embed_uses_string_representation_of_user_in_title_if_nick_is_not_available(self): """The embed should use the string representation of the user if they don't have a nick.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=1)) user = helpers.MockMember() user.nick = None user.__str__ = unittest.mock.Mock(return_value="Mr. Hemlock") @@ -363,7 +363,7 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) def test_create_user_embed_uses_nick_in_title_if_available(self): """The embed should use the nick if it's available.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=1)) user = helpers.MockMember() user.nick = "Cat lover" user.__str__ = unittest.mock.Mock(return_value="Mr. Hemlock") @@ -375,8 +375,8 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) def test_create_user_embed_ignores_everyone_role(self): """Created `!user` embeds should not contain mention of the @everyone-role.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) - admins_role = helpers.MockRole('Admins') + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=1)) + admins_role = helpers.MockRole(name='Admins') admins_role.colour = 100 # A `MockMember` has the @Everyone role by default; we add the Admins to that. @@ -391,15 +391,15 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.user_nomination_counts", new_callable=helpers.AsyncMock) def test_create_user_embed_expanded_information_in_moderation_channels(self, nomination_counts, infraction_counts): """The embed should contain expanded infractions and nomination info in mod channels.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=50)) + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=50)) - moderators_role = helpers.MockRole('Moderators') + moderators_role = helpers.MockRole(name='Moderators') moderators_role.colour = 100 infraction_counts.return_value = "expanded infractions info" nomination_counts.return_value = "nomination info" - user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + user = helpers.MockMember(id=314, roles=[moderators_role], top_role=moderators_role) embed = asyncio.run(self.cog.create_user_embed(ctx, user)) infraction_counts.assert_called_once_with(user) @@ -426,14 +426,14 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new_callable=helpers.AsyncMock) def test_create_user_embed_basic_information_outside_of_moderation_channels(self, infraction_counts): """The embed should contain only basic infraction data outside of mod channels.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=100)) + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=100)) - moderators_role = helpers.MockRole('Moderators') + moderators_role = helpers.MockRole(name='Moderators') moderators_role.colour = 100 infraction_counts.return_value = "basic infractions info" - user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + user = helpers.MockMember(id=314, roles=[moderators_role], top_role=moderators_role) embed = asyncio.run(self.cog.create_user_embed(ctx, user)) infraction_counts.assert_called_once_with(user) @@ -459,10 +459,10 @@ class UserEmbedTests(unittest.TestCase): """The embed should be created with the colour of the top role, if a top role is available.""" ctx = helpers.MockContext() - moderators_role = helpers.MockRole('Moderators') + moderators_role = helpers.MockRole(name='Moderators') moderators_role.colour = 100 - user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + user = helpers.MockMember(id=314, roles=[moderators_role], top_role=moderators_role) embed = asyncio.run(self.cog.create_user_embed(ctx, user)) self.assertEqual(embed.colour, discord.Colour(moderators_role.colour)) @@ -472,7 +472,7 @@ class UserEmbedTests(unittest.TestCase): """The embed should be created with a blurple colour if the user has no assigned roles.""" ctx = helpers.MockContext() - user = helpers.MockMember(user_id=217) + user = helpers.MockMember(id=217) embed = asyncio.run(self.cog.create_user_embed(ctx, user)) self.assertEqual(embed.colour, discord.Colour.blurple()) @@ -482,7 +482,7 @@ class UserEmbedTests(unittest.TestCase): """The embed thumbnail should be set to the user's avatar in `png` format.""" ctx = helpers.MockContext() - user = helpers.MockMember(user_id=217) + user = helpers.MockMember(id=217) user.avatar_url_as.return_value = "avatar url" embed = asyncio.run(self.cog.create_user_embed(ctx, user)) @@ -499,13 +499,13 @@ class UserCommandTests(unittest.TestCase): self.bot = helpers.MockBot() self.cog = information.Information(self.bot) - self.moderator_role = helpers.MockRole("Moderators", role_id=2, position=10) - self.flautist_role = helpers.MockRole("Flautists", role_id=3, position=2) - self.bassist_role = helpers.MockRole("Bassists", role_id=4, position=3) + self.moderator_role = helpers.MockRole(name="Moderators", id=2, position=10) + self.flautist_role = helpers.MockRole(name="Flautists", id=3, position=2) + self.bassist_role = helpers.MockRole(name="Bassists", id=4, position=3) - self.author = helpers.MockMember(user_id=1, name="syntaxaire") - self.moderator = helpers.MockMember(user_id=2, name="riffautae", roles=[self.moderator_role]) - self.target = helpers.MockMember(user_id=3, name="__fluzz__") + self.author = helpers.MockMember(id=1, name="syntaxaire") + self.moderator = helpers.MockMember(id=2, name="riffautae", roles=[self.moderator_role]) + self.target = helpers.MockMember(id=3, name="__fluzz__") def test_regular_member_cannot_target_another_member(self, constants): """A regular user should not be able to use `!user` targeting another user.""" @@ -523,7 +523,7 @@ class UserCommandTests(unittest.TestCase): constants.STAFF_ROLES = [self.moderator_role.id] constants.Channels.bot = 50 - ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=100)) + ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=100)) msg = "Sorry, but you may only use this command within <#50>." with self.assertRaises(InChannelCheckFailure, msg=msg): @@ -535,7 +535,7 @@ class UserCommandTests(unittest.TestCase): constants.STAFF_ROLES = [self.moderator_role.id] constants.Channels.bot = 50 - ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=50)) + ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=50)) asyncio.run(self.cog.user_info.callback(self.cog, ctx)) @@ -548,7 +548,7 @@ class UserCommandTests(unittest.TestCase): constants.STAFF_ROLES = [self.moderator_role.id] constants.Channels.bot = 50 - ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=50)) + ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=50)) asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.author)) @@ -561,7 +561,7 @@ class UserCommandTests(unittest.TestCase): constants.STAFF_ROLES = [self.moderator_role.id] constants.Channels.bot = 50 - ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(channel_id=200)) + ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(id=200)) asyncio.run(self.cog.user_info.callback(self.cog, ctx)) @@ -574,7 +574,7 @@ class UserCommandTests(unittest.TestCase): constants.MODERATION_ROLES = [self.moderator_role.id] constants.STAFF_ROLES = [self.moderator_role.id] - ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(channel_id=50)) + ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(id=50)) asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.target)) diff --git a/tests/bot/cogs/test_token_remover.py b/tests/bot/cogs/test_token_remover.py index dfb1bafc9..3276cf5a5 100644 --- a/tests/bot/cogs/test_token_remover.py +++ b/tests/bot/cogs/test_token_remover.py @@ -24,7 +24,7 @@ class TokenRemoverTests(unittest.TestCase): self.bot.get_cog.return_value.send_log_message = AsyncMock() self.cog = TokenRemover(bot=self.bot) - self.msg = MockMessage(message_id=555, content='') + self.msg = MockMessage(id=555, content='') self.msg.author.__str__ = MagicMock() self.msg.author.__str__.return_value = 'lemon' self.msg.author.bot = False diff --git a/tests/bot/utils/test_checks.py b/tests/bot/utils/test_checks.py index 19b758336..9610771e5 100644 --- a/tests/bot/utils/test_checks.py +++ b/tests/bot/utils/test_checks.py @@ -22,7 +22,7 @@ class ChecksTests(unittest.TestCase): def test_with_role_check_with_guild_and_required_role(self): """`with_role_check` returns `True` if `Context.author` has the required role.""" - self.ctx.author.roles.append(MockRole(role_id=10)) + self.ctx.author.roles.append(MockRole(id=10)) self.assertTrue(checks.with_role_check(self.ctx, 10)) def test_without_role_check_without_guild(self): @@ -33,13 +33,13 @@ class ChecksTests(unittest.TestCase): def test_without_role_check_returns_false_with_unwanted_role(self): """`without_role_check` returns `False` if `Context.author` has unwanted role.""" role_id = 42 - self.ctx.author.roles.append(MockRole(role_id=role_id)) + self.ctx.author.roles.append(MockRole(id=role_id)) self.assertFalse(checks.without_role_check(self.ctx, role_id)) def test_without_role_check_returns_true_without_unwanted_role(self): """`without_role_check` returns `True` if `Context.author` does not have unwanted role.""" role_id = 42 - self.ctx.author.roles.append(MockRole(role_id=role_id)) + self.ctx.author.roles.append(MockRole(id=role_id)) self.assertTrue(checks.without_role_check(self.ctx, role_id + 10)) def test_in_channel_check_for_correct_channel(self): diff --git a/tests/helpers.py b/tests/helpers.py index 5dc7a0d2f..35f2c288c 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -1,8 +1,10 @@ from __future__ import annotations import asyncio +import collections import functools import inspect +import itertools import logging import unittest.mock from typing import Any, Iterable, Optional @@ -72,14 +74,16 @@ class CustomMockMixin: """ child_mock_type = unittest.mock.MagicMock + discord_id = itertools.count(0) - def __init__(self, spec: Any = None, **kwargs): + def __init__(self, spec_set: Any = None, **kwargs): name = kwargs.pop('name', None) # `name` has special meaning for Mock classes, so we need to set it manually. - super().__init__(spec=spec, **kwargs) + super().__init__(spec_set=spec_set, **kwargs) + if name: self.name = name - if spec: - self._extract_coroutine_methods_from_spec_instance(spec) + if spec_set: + self._extract_coroutine_methods_from_spec_instance(spec_set) def _get_child_mock(self, **kw): """ @@ -169,25 +173,14 @@ 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, - roles: Optional[Iterable[MockRole]] = None, - members: Optional[Iterable[MockMember]] = None, - **kwargs, - ) -> None: - super().__init__(spec=guild_instance, **kwargs) - - self.id = guild_id - - self.roles = [MockRole("@everyone", 1)] + def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None: + default_kwargs = {'id': next(self.discord_id), 'members': []} + super().__init__(spec_set=guild_instance, **collections.ChainMap(kwargs, default_kwargs)) + + self.roles = [MockRole(name="@everyone", position=1, id=0)] if roles: self.roles.extend(roles) - self.members = [] - if members: - self.members.extend(members) - # Create a Role instance to get a realistic Mock of `discord.Role` role_data = {'name': 'role', 'id': 1} @@ -201,13 +194,12 @@ 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) + def __init__(self, **kwargs) -> None: + default_kwargs = {'id': next(self.discord_id), 'name': 'role', 'position': 1} + super().__init__(spec_set=role_instance, **collections.ChainMap(kwargs, default_kwargs)) - self.name = name - self.id = role_id - self.position = position - self.mention = f'&{self.name}' + if 'mention' not in kwargs: + self.mention = f'&{self.name}' def __lt__(self, other): """Simplified position-based comparisons similar to those of `discord.Role`.""" @@ -227,19 +219,11 @@ 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", - user_id: int = 1, - roles: Optional[Iterable[MockRole]] = None, - **kwargs, - ) -> None: - super().__init__(spec=member_instance, **kwargs) - - self.name = name - self.id = user_id - - self.roles = [MockRole("@everyone", 1)] + def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None: + default_kwargs = {'name': 'member', 'id': next(self.discord_id)} + super().__init__(spec_set=member_instance, **collections.ChainMap(kwargs, default_kwargs)) + + self.roles = [MockRole(name="@everyone", position=1, id=0)] if roles: self.roles.extend(roles) @@ -248,6 +232,8 @@ class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin # 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 +bot_instance.api_client = None class MockBot(CustomMockMixin, unittest.mock.MagicMock): @@ -258,11 +244,7 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): For more information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=bot_instance, **kwargs) - - # Our custom attributes and methods - self.http_session = unittest.mock.MagicMock() - self.api_client = unittest.mock.MagicMock() + super().__init__(spec_set=bot_instance, **kwargs) # self.wait_for is *not* a coroutine function, but returns a coroutine nonetheless and # and should therefore be awaited. (The documentation calls it a coroutine as well, which @@ -294,11 +276,11 @@ class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): 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 - self.name = name - self.guild = kwargs.get('guild', MockGuild()) - self.mention = f"#{self.name}" + default_kwargs = {'id': next(self.discord_id), 'name': 'channel', 'guild': MockGuild()} + super().__init__(spec_set=channel_instance, **collections.ChainMap(kwargs, default_kwargs)) + + if 'mention' not in kwargs: + self.mention = f"#{self.name}" # Create a Message instance to get a realistic MagicMock of `discord.Message` @@ -335,12 +317,11 @@ class MockContext(CustomMockMixin, unittest.mock.MagicMock): instances. For more information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=context_instance, **kwargs) + super().__init__(spec_set=context_instance, **kwargs) self.bot = kwargs.get('bot', MockBot()) self.guild = kwargs.get('guild', MockGuild()) self.author = kwargs.get('author', MockMember()) self.channel = kwargs.get('channel', MockTextChannel()) - self.command = kwargs.get('command', unittest.mock.MagicMock()) class MockMessage(CustomMockMixin, unittest.mock.MagicMock): @@ -351,7 +332,7 @@ class MockMessage(CustomMockMixin, unittest.mock.MagicMock): information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=message_instance, **kwargs) + super().__init__(spec_set=message_instance, **kwargs) self.author = kwargs.get('author', MockMember()) self.channel = kwargs.get('channel', MockTextChannel()) @@ -368,12 +349,9 @@ class MockEmoji(CustomMockMixin, unittest.mock.MagicMock): information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=emoji_instance, **kwargs) + super().__init__(spec_set=emoji_instance, **kwargs) self.guild = kwargs.get('guild', MockGuild()) - # Get all coroutine functions and set them as AsyncMock attributes - self._extract_coroutine_methods_from_spec_instance(emoji_instance) - partial_emoji_instance = discord.PartialEmoji(animated=False, name='guido') @@ -386,7 +364,7 @@ class MockPartialEmoji(CustomMockMixin, unittest.mock.MagicMock): more information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=partial_emoji_instance, **kwargs) + super().__init__(spec_set=partial_emoji_instance, **kwargs) reaction_instance = discord.Reaction(message=MockMessage(), data={'me': True}, emoji=MockEmoji()) @@ -400,6 +378,6 @@ class MockReaction(CustomMockMixin, unittest.mock.MagicMock): more information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=reaction_instance, **kwargs) + super().__init__(spec_set=reaction_instance, **kwargs) self.emoji = kwargs.get('emoji', MockEmoji()) self.message = kwargs.get('message', MockMessage()) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 2b58634dd..e879ef97a 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -19,7 +19,6 @@ class DiscordMocksTests(unittest.TestCase): self.assertIsInstance(role, discord.Role) self.assertEqual(role.name, "role") - self.assertEqual(role.id, 1) self.assertEqual(role.position, 1) self.assertEqual(role.mention, "&role") @@ -27,7 +26,7 @@ class DiscordMocksTests(unittest.TestCase): """Test if MockRole initializes with the arguments provided.""" role = helpers.MockRole( name="Admins", - role_id=90210, + id=90210, position=10, ) @@ -67,22 +66,21 @@ class DiscordMocksTests(unittest.TestCase): self.assertIsInstance(member, discord.Member) self.assertEqual(member.name, "member") - self.assertEqual(member.id, 1) - self.assertListEqual(member.roles, [helpers.MockRole("@everyone", 1)]) + self.assertListEqual(member.roles, [helpers.MockRole(name="@everyone", position=1, id=0)]) self.assertEqual(member.mention, "@member") def test_mock_member_alternative_arguments(self): """Test if MockMember initializes with the arguments provided.""" - core_developer = helpers.MockRole("Core Developer", 2) + core_developer = helpers.MockRole(name="Core Developer", position=2) member = helpers.MockMember( name="Mark", - user_id=12345, + id=12345, roles=[core_developer] ) self.assertEqual(member.name, "Mark") self.assertEqual(member.id, 12345) - self.assertListEqual(member.roles, [helpers.MockRole("@everyone", 1), core_developer]) + self.assertListEqual(member.roles, [helpers.MockRole(name="@everyone", position=1, id=0), core_developer]) self.assertEqual(member.mention, "@Mark") def test_mock_member_accepts_dynamic_arguments(self): @@ -102,19 +100,19 @@ class DiscordMocksTests(unittest.TestCase): # The `spec` argument makes sure `isistance` checks with `discord.Guild` pass self.assertIsInstance(guild, discord.Guild) - self.assertListEqual(guild.roles, [helpers.MockRole("@everyone", 1)]) + self.assertListEqual(guild.roles, [helpers.MockRole(name="@everyone", position=1, id=0)]) self.assertListEqual(guild.members, []) def test_mock_guild_alternative_arguments(self): """Test if MockGuild initializes with the arguments provided.""" - core_developer = helpers.MockRole("Core Developer", 2) + core_developer = helpers.MockRole(name="Core Developer", position=2) guild = helpers.MockGuild( roles=[core_developer], - members=[helpers.MockMember(user_id=54321)], + members=[helpers.MockMember(id=54321)], ) - self.assertListEqual(guild.roles, [helpers.MockRole("@everyone", 1), core_developer]) - self.assertListEqual(guild.members, [helpers.MockMember(user_id=54321)]) + self.assertListEqual(guild.roles, [helpers.MockRole(name="@everyone", position=1, id=0), core_developer]) + self.assertListEqual(guild.members, [helpers.MockMember(id=54321)]) def test_mock_guild_accepts_dynamic_arguments(self): """Test if MockGuild accepts and sets abitrary keyword arguments.""" @@ -191,51 +189,18 @@ class DiscordMocksTests(unittest.TestCase): with self.assertRaises(AttributeError): mock.the_cake_is_a_lie - def test_custom_mock_methods_are_valid_discord_object_methods(self): - """The `AsyncMock` attributes of the mocks should be valid for the class they're mocking.""" - mocks = ( - (helpers.MockGuild, helpers.guild_instance), - (helpers.MockRole, helpers.role_instance), - (helpers.MockMember, helpers.member_instance), - (helpers.MockBot, helpers.bot_instance), - (helpers.MockContext, helpers.context_instance), - (helpers.MockTextChannel, helpers.channel_instance), - (helpers.MockMessage, helpers.message_instance), + def test_mocks_use_mention_when_provided_as_kwarg(self): + """The mock should use the passed `mention` instead of the default one if present.""" + test_cases = ( + (helpers.MockRole, "role mention"), + (helpers.MockMember, "member mention"), + (helpers.MockTextChannel, "channel mention"), ) - for mock_class, instance in mocks: - mock = mock_class() - async_methods = ( - attr for attr in dir(mock) if isinstance(getattr(mock, attr), helpers.AsyncMock) - ) - - # spec_mock = unittest.mock.MagicMock(spec=instance) - for method in async_methods: - with self.subTest(mock_class=mock_class, method=method): - try: - getattr(instance, method) - except AttributeError: - msg = f"method {method} is not a method attribute of {instance.__class__}" - self.fail(msg) - - @unittest.mock.patch(f'{__name__}.DiscordMocksTests.subTest') - def test_the_custom_mock_methods_test(self, subtest_mock): - """The custom method test should raise AssertionError for invalid methods.""" - class FakeMockBot(helpers.CustomMockMixin, unittest.mock.MagicMock): - """Fake MockBot class with invalid attribute/method `release_the_walrus`.""" - - child_mock_type = unittest.mock.MagicMock - - def __init__(self, **kwargs): - super().__init__(spec=helpers.bot_instance, **kwargs) - - # Fake attribute - self.release_the_walrus = helpers.AsyncMock() - - with unittest.mock.patch("tests.helpers.MockBot", new=FakeMockBot): - msg = "method release_the_walrus is not a valid method of " - with self.assertRaises(AssertionError, msg=msg): - self.test_custom_mock_methods_are_valid_discord_object_methods() + for mock_type, mention in test_cases: + with self.subTest(mock_type=mock_type, mention=mention): + mock = mock_type(mention=mention) + self.assertEqual(mock.mention, mention) class MockObjectTests(unittest.TestCase): @@ -266,14 +231,14 @@ class MockObjectTests(unittest.TestCase): def test_hashable_mixin_uses_id_for_equality_comparison(self): """Test if the HashableMixing uses the id attribute for hashing.""" - class MockScragly(unittest.mock.Mock, helpers.HashableMixin): + class MockScragly(helpers.HashableMixin): pass - scragly = MockScragly(spec=object) + scragly = MockScragly() scragly.id = 10 - eevee = MockScragly(spec=object) + eevee = MockScragly() eevee.id = 10 - python = MockScragly(spec=object) + python = MockScragly() python.id = 20 self.assertTrue(scragly == eevee) @@ -281,14 +246,14 @@ class MockObjectTests(unittest.TestCase): def test_hashable_mixin_uses_id_for_nonequality_comparison(self): """Test if the HashableMixing uses the id attribute for hashing.""" - class MockScragly(unittest.mock.Mock, helpers.HashableMixin): + class MockScragly(helpers.HashableMixin): pass - scragly = MockScragly(spec=object) + scragly = MockScragly() scragly.id = 10 - eevee = MockScragly(spec=object) + eevee = MockScragly() eevee.id = 10 - python = MockScragly(spec=object) + python = MockScragly() python.id = 20 self.assertTrue(scragly != python) @@ -298,7 +263,7 @@ class MockObjectTests(unittest.TestCase): """Test if the MagicMock subclasses that implement the HashableMixin use id for hash.""" for mock in self.hashable_mocks: with self.subTest(mock_class=mock): - instance = helpers.MockRole(role_id=100) + instance = helpers.MockRole(id=100) self.assertEqual(hash(instance), instance.id) def test_mock_class_with_hashable_mixin_uses_id_for_equality(self): @@ -396,11 +361,11 @@ class MockObjectTests(unittest.TestCase): @unittest.mock.patch("tests.helpers.CustomMockMixin._extract_coroutine_methods_from_spec_instance") def test_custom_mock_mixin_init_with_spec(self, extract_method_mock): """Test if CustomMockMixin correctly passes on spec/kwargs and calls the extraction method.""" - spec = "pydis" + spec_set = "pydis" - helpers.CustomMockMixin(spec=spec) + helpers.CustomMockMixin(spec_set=spec_set) - extract_method_mock.assert_called_once_with(spec) + extract_method_mock.assert_called_once_with(spec_set) @unittest.mock.patch("builtins.super", new=unittest.mock.MagicMock()) @unittest.mock.patch("tests.helpers.CustomMockMixin._extract_coroutine_methods_from_spec_instance") -- cgit v1.2.3 From 7f4829e9fab007690d48188f499bfcc1a7baa437 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 13 Nov 2019 17:29:09 +0100 Subject: Prevent await warnings for MockBot's create_task Previously, the coroutine object passed to `MockBot.loop.create_task` would trigger a `RuntimeWarning` for not being awaited as we do not actually create a task for it. To prevent these warnings, coroutine objects passed will now automatically be closed. --- tests/helpers.py | 8 +++++++- tests/test_helpers.py | 12 ++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/helpers.py b/tests/helpers.py index 35f2c288c..8a14aeef4 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -227,7 +227,8 @@ class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin if roles: self.roles.extend(roles) - self.mention = f"@{self.name}" + if 'mention' not in kwargs: + self.mention = f"@{self.name}" # Create a Bot instance to get a realistic MagicMock of `discord.ext.commands.Bot` @@ -251,6 +252,11 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): # is technically incorrect, since it's a regular def.) self.wait_for = AsyncMock() + # Since calling `create_task` on our MockBot does not actually schedule the coroutine object + # as a task in the asyncio loop, this `side_effect` calls `close()` on the coroutine object + # to prevent "has not been awaited"-warnings. + self.loop.create_task.side_effect = lambda coroutine: coroutine.close() + # Create a TextChannel instance to get a realistic MagicMock of `discord.TextChannel` channel_data = { diff --git a/tests/test_helpers.py b/tests/test_helpers.py index e879ef97a..7894e104a 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -202,6 +202,18 @@ class DiscordMocksTests(unittest.TestCase): mock = mock_type(mention=mention) self.assertEqual(mock.mention, mention) + def test_create_test_on_mock_bot_closes_passed_coroutine(self): + """`bot.loop.create_task` should close the passed coroutine object to prevent warnings.""" + async def dementati(): + """Dummy coroutine for testing purposes.""" + + coroutine_object = dementati() + + bot = helpers.MockBot() + bot.loop.create_task(coroutine_object) + with self.assertRaises(RuntimeError, msg="cannot reuse already awaited coroutine"): + asyncio.run(coroutine_object) + class MockObjectTests(unittest.TestCase): """Tests the mock objects and mixins we've defined.""" -- cgit v1.2.3