From 538048189c8babfa72c70563a81a4b725fcb90e4 Mon Sep 17 00:00:00 2001 From: kosayoda Date: Mon, 28 Oct 2019 13:44:46 +0800 Subject: Chain words before length check --- bot/cogs/off_topic_names.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/bot/cogs/off_topic_names.py b/bot/cogs/off_topic_names.py index 1f9fb0b4f..78792240f 100644 --- a/bot/cogs/off_topic_names.py +++ b/bot/cogs/off_topic_names.py @@ -24,6 +24,9 @@ class OffTopicName(Converter): """Attempt to replace any invalid characters with their approximate Unicode equivalent.""" allowed_characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ!?'`-" + # Chain multiple words to a single one + argument = "-".join(argument.split()) + if not (2 <= len(argument) <= 96): raise BadArgument("Channel name must be between 2 and 96 chars long") @@ -97,15 +100,12 @@ class OffTopicNames(Cog): @otname_group.command(name='add', aliases=('a',)) @with_role(*MODERATION_ROLES) - async def add_command(self, ctx: Context, *names: OffTopicName) -> None: + async def add_command(self, ctx: Context, *, name: OffTopicName) -> None: """ Adds a new off-topic name to the rotation. The name is not added if it is too similar to an existing name. """ - # Chain multiple words to a single one - name = "-".join(names) - existing_names = await self.bot.api_client.get('bot/off-topic-channel-names') close_match = difflib.get_close_matches(name, existing_names, n=1, cutoff=0.8) @@ -123,10 +123,8 @@ class OffTopicNames(Cog): @otname_group.command(name='forceadd', aliases=('fa',)) @with_role(*MODERATION_ROLES) - async def force_add_command(self, ctx: Context, *names: OffTopicName) -> None: + async def force_add_command(self, ctx: Context, *, name: OffTopicName) -> None: """Forcefully adds a new off-topic name to the rotation.""" - # Chain multiple words to a single one - name = "-".join(names) await self._add_name(ctx, name) async def _add_name(self, ctx: Context, name: str) -> None: @@ -138,10 +136,8 @@ class OffTopicNames(Cog): @otname_group.command(name='delete', aliases=('remove', 'rm', 'del', 'd')) @with_role(*MODERATION_ROLES) - async def delete_command(self, ctx: Context, *names: OffTopicName) -> None: + async def delete_command(self, ctx: Context, *, name: OffTopicName) -> None: """Removes a off-topic name from the rotation.""" - # Chain multiple words to a single one - name = "-".join(names) await self.bot.api_client.delete(f'bot/off-topic-channel-names/{name}') log.info(f"{ctx.author} deleted the off-topic channel name '{name}'") -- cgit v1.2.3 From c5d0eb473a9a1dc486dd2dd60603463435e49da4 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Mon, 28 Oct 2019 15:56:34 +0100 Subject: Change generation of child mocks - https://docs.python.org/3/library/unittest.mock.html We previously used an override of the `__new__` method to prevent our custom mock types from instantiating their children with their own type instead of a general mock type like `MagicMock` or `Mock`. As it turns out, the Python documentation suggests another method of doing this that does not involve overriding `__new__`. This commit implements this new method to make sure we're using the idiomatic way of handling this. The suggested method is overriding the `_get_child_mock` method in the subclass. To make our code DRY, I've created a mixin that should come BEFORE the mock type we're subclassing in the MRO. --- In addition, I have also added this new mixin to our `AsyncMock` class to make sure that its `__call__` method returns a proper mock object after it has been awaited. This makes sure that subsequent attribute access on the returned object is mocked as expected. --- tests/helpers.py | 85 +++++++++++++++++++++++++++++++-------------------- tests/test_helpers.py | 18 +++++++++-- 2 files changed, 67 insertions(+), 36 deletions(-) diff --git a/tests/helpers.py b/tests/helpers.py index 892d42e6c..9375d0986 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -24,19 +24,6 @@ def async_test(wrapped): return wrapper -# TODO: Remove me in Python 3.8 -class AsyncMock(unittest.mock.MagicMock): - """ - A MagicMock subclass to mock async callables. - - 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) - - class HashableMixin(discord.mixins.EqualityComparable): """ Mixin that provides similar hashing and equality functionality as discord.py's `Hashable` mixin. @@ -61,15 +48,43 @@ class ColourMixin: self.colour = color -class AttributeMock: +class GetChildMockMixin: """Ensures attributes of our mock types will be instantiated with the correct mock type.""" - def __new__(cls, *args, **kwargs): - """Stops the regular parent class from propagating to newly mocked attributes.""" - if 'parent' in kwargs: - return cls.attribute_mocktype(*args, **kwargs) + def _get_child_mock(self, **kw): + """ + Overwrite of the `_get_child_mock` method to stop the propagation of our custom mock classes. + + Mock objects automatically create children when you access an attribute or call a method on them. By default, + the class of these children is the type of the parent itself. However, this would mean that the children created + for our custom mock types would also be instances of that custom mock type. This is not desirable, as attributes + of, e.g., a `Bot` object are not `Bot` objects themselves. The Python docs for `unittest.mock` hint that + overwriting this method is the best way to deal with that. + + This override will look for an attribute called `child_mock_type` and use that as the type of the child mock. + """ + klass = self.child_mock_type - return super().__new__(cls) + if self._mock_sealed: + attribute = "." + kw["name"] if "name" in kw else "()" + mock_name = self._extract_mock_name() + attribute + raise AttributeError(mock_name) + + return klass(**kw) + + +# TODO: Remove me in Python 3.8 +class AsyncMock(GetChildMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock async callables. + + 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. + """ + child_mock_type = unittest.mock.MagicMock + + async def __call__(self, *args, **kwargs): + return super(AsyncMock, self).__call__(*args, **kwargs) # Create a guild instance to get a realistic Mock of `discord.Guild` @@ -95,7 +110,7 @@ guild_data = { guild_instance = discord.Guild(data=guild_data, state=unittest.mock.MagicMock()) -class MockGuild(AttributeMock, unittest.mock.Mock, HashableMixin): +class MockGuild(GetChildMockMixin, unittest.mock.Mock, HashableMixin): """ A `Mock` subclass to mock `discord.Guild` objects. @@ -122,7 +137,7 @@ class MockGuild(AttributeMock, unittest.mock.Mock, HashableMixin): For more info, see the `Mocking` section in `tests/README.md`. """ - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__( self, @@ -175,7 +190,7 @@ role_data = {'name': 'role', 'id': 1} role_instance = discord.Role(guild=guild_instance, state=unittest.mock.MagicMock(), data=role_data) -class MockRole(AttributeMock, unittest.mock.Mock, ColourMixin, HashableMixin): +class MockRole(GetChildMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): """ A Mock subclass to mock `discord.Role` objects. @@ -183,7 +198,7 @@ class MockRole(AttributeMock, unittest.mock.Mock, ColourMixin, HashableMixin): information, see the `MockGuild` docstring. """ - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__(self, name: str = "role", role_id: int = 1, position: int = 1, **kwargs) -> None: super().__init__(spec=role_instance, **kwargs) @@ -208,7 +223,7 @@ state_mock = unittest.mock.MagicMock() member_instance = discord.Member(data=member_data, guild=guild_instance, state=state_mock) -class MockMember(AttributeMock, unittest.mock.Mock, ColourMixin, HashableMixin): +class MockMember(GetChildMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): """ A Mock subclass to mock Member objects. @@ -216,7 +231,7 @@ class MockMember(AttributeMock, unittest.mock.Mock, ColourMixin, HashableMixin): information, see the `MockGuild` docstring. """ - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__( self, @@ -254,7 +269,7 @@ class MockMember(AttributeMock, unittest.mock.Mock, ColourMixin, HashableMixin): bot_instance = Bot(command_prefix=unittest.mock.MagicMock()) -class MockBot(AttributeMock, unittest.mock.MagicMock): +class MockBot(GetChildMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Bot objects. @@ -262,11 +277,15 @@ class MockBot(AttributeMock, unittest.mock.MagicMock): For more information, see the `MockGuild` docstring. """ - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock 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() + # `discord.ext.commands.Bot` coroutines self._before_invoke = AsyncMock() self._after_invoke = AsyncMock() @@ -303,7 +322,7 @@ class MockBot(AttributeMock, unittest.mock.MagicMock): context_instance = Context(message=unittest.mock.MagicMock(), prefix=unittest.mock.MagicMock()) -class MockContext(AttributeMock, unittest.mock.MagicMock): +class MockContext(GetChildMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Context objects. @@ -311,7 +330,7 @@ class MockContext(AttributeMock, unittest.mock.MagicMock): instances. For more information, see the `MockGuild` docstring. """ - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__(self, **kwargs) -> None: super().__init__(spec=context_instance, **kwargs) @@ -346,7 +365,7 @@ guild = unittest.mock.MagicMock() channel_instance = discord.TextChannel(state=state, guild=guild, data=channel_data) -class MockTextChannel(AttributeMock, unittest.mock.Mock, HashableMixin): +class MockTextChannel(GetChildMockMixin, unittest.mock.Mock, HashableMixin): """ A MagicMock subclass to mock TextChannel objects. @@ -354,7 +373,7 @@ class MockTextChannel(AttributeMock, unittest.mock.Mock, HashableMixin): more information, see the `MockGuild` docstring. """ - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__(self, name: str = 'channel', channel_id: int = 1, **kwargs) -> None: super().__init__(spec=channel_instance, **kwargs) @@ -402,7 +421,7 @@ channel = unittest.mock.MagicMock() message_instance = discord.Message(state=state, channel=channel, data=message_data) -class MockMessage(AttributeMock, unittest.mock.MagicMock): +class MockMessage(GetChildMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Message objects. @@ -410,7 +429,7 @@ class MockMessage(AttributeMock, unittest.mock.MagicMock): information, see the `MockGuild` docstring. """ - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__(self, **kwargs) -> None: super().__init__(spec=message_instance, **kwargs) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index f08239981..62007ff4e 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -221,10 +221,10 @@ class DiscordMocksTests(unittest.TestCase): @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.AttributeMock, unittest.mock.MagicMock): + class FakeMockBot(helpers.GetChildMockMixin, unittest.mock.MagicMock): """Fake MockBot class with invalid attribute/method `release_the_walrus`.""" - attribute_mocktype = unittest.mock.MagicMock + child_mock_type = unittest.mock.MagicMock def __init__(self, **kwargs): super().__init__(spec=helpers.bot_instance, **kwargs) @@ -331,6 +331,18 @@ class MockObjectTests(unittest.TestCase): self.assertFalse(instance_one != instance_two) self.assertTrue(instance_one != instance_three) + def test_get_child_mock_mixin_accepts_mock_seal(self): + """The `GetChildMockMixin` should support `unittest.mock.seal`.""" + class MyMock(helpers.GetChildMockMixin, unittest.mock.MagicMock): + + child_mock_type = unittest.mock.MagicMock + pass + + mock = MyMock() + unittest.mock.seal(mock) + with self.assertRaises(AttributeError, msg="MyMock.shirayuki"): + mock.shirayuki = "hello!" + def test_spec_propagation_of_mock_subclasses(self): """Test if the `spec` does not propagate to attributes of the mock object.""" test_values = ( @@ -346,7 +358,7 @@ class MockObjectTests(unittest.TestCase): mock = mock_type() self.assertTrue(isinstance(mock, mock_type)) attribute = getattr(mock, valid_attribute) - self.assertTrue(isinstance(attribute, mock_type.attribute_mocktype)) + self.assertTrue(isinstance(attribute, mock_type.child_mock_type)) def test_async_mock_provides_coroutine_for_dunder_call(self): """Test if AsyncMock objects have a coroutine for their __call__ method.""" -- cgit v1.2.3 From 1a1ef440aa8c1c63e9ed38742641d18da68c30c3 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Tue, 29 Oct 2019 13:44:13 +0100 Subject: Enhance Reddit webhook embeds https://github.com/python-discord/bot/issues/634 I have changes the appearance of the embed generated for the reddit webhook. The changes: - Bold markdown around the links to prevent it breaking on android. - Stylized the meta-data line with newly created emoji-based icons. - Removed redundant mentions of the subreddit from the embed itself. The emojis were uploaded to the `Emojis II` guild and the IDs have been added to the constants files. In addition, I've set the reddit logo as the default avatar for the webhook in the guild. This commit closes #634 --- bot/cogs/reddit.py | 6 +++--- bot/constants.py | 4 ++++ config-default.yml | 4 ++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index 7749d237f..f947a7d78 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -9,7 +9,7 @@ from discord import Colour, Embed, TextChannel from discord.ext.commands import Bot, Cog, Context, group from discord.ext.tasks import loop -from bot.constants import Channels, ERROR_REPLIES, Reddit as RedditConfig, STAFF_ROLES, Webhooks +from bot.constants import Channels, ERROR_REPLIES, Emojis, Reddit as RedditConfig, STAFF_ROLES, Webhooks from bot.converters import Subreddit from bot.decorators import with_role from bot.pagination import LinePaginator @@ -117,9 +117,9 @@ class Reddit(Cog): link = self.URL + data["permalink"] embed.description += ( - f"[**{title}**]({link})\n" + f"**[{title}]({link})**\n" f"{text}" - f"| {ups} upvotes | {comments} comments | u/{author} | {subreddit} |\n\n" + f"{Emojis.upvotes} {ups} {Emojis.comments} {comments} {Emojis.user} {author}\n\n" ) embed.colour = Colour.blurple() diff --git a/bot/constants.py b/bot/constants.py index 838fe7a79..d3e79b4c2 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -259,6 +259,10 @@ class Emojis(metaclass=YAMLGetter): pencil: str cross_mark: str + upvotes: str + comments: str + user: str + class Icons(metaclass=YAMLGetter): section = "style" diff --git a/config-default.yml b/config-default.yml index 4638a89ee..9c7830077 100644 --- a/config-default.yml +++ b/config-default.yml @@ -37,6 +37,10 @@ style: new: "\U0001F195" cross_mark: "\u274C" + upvotes: "<:upvotes:638706000714792962>" + comments: "<:comments:638706001159258132>" + user: "<:user:638706001217978368>" + icons: crown_blurple: "https://cdn.discordapp.com/emojis/469964153289965568.png" crown_green: "https://cdn.discordapp.com/emojis/469964154719961088.png" -- cgit v1.2.3 From 55b83342f389aeacdbd5d2558dea4981c90aa2c4 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Tue, 29 Oct 2019 14:27:56 +0100 Subject: Use 'local' emojis for reddit webhook embed Apparently, webhooks cannot use external emojis. For now, I have added the emojis to the guild directly and updated the IDs in this commit. I don't really like having "utility" emojis on the main guild, but the alternative would be to not use webhooks, which is a change we will need to discuss before we apply it. (I don't see anything against it now we just send a daily digest, although, in principle, this is precisely what a webhook is meant for.) --- config-default.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config-default.yml b/config-default.yml index 9c7830077..bce6ea266 100644 --- a/config-default.yml +++ b/config-default.yml @@ -37,9 +37,9 @@ style: new: "\U0001F195" cross_mark: "\u274C" - upvotes: "<:upvotes:638706000714792962>" - comments: "<:comments:638706001159258132>" - user: "<:user:638706001217978368>" + upvotes: "<:upvotes:638729835245731840>" + comments: "<:comments:638729835073765387>" + user: "<:user:638729835442602003>" icons: crown_blurple: "https://cdn.discordapp.com/emojis/469964153289965568.png" -- cgit v1.2.3 From 618ba6a523dababde230382e1965ecc89f23aaf5 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Tue, 29 Oct 2019 10:04:37 +0100 Subject: Enhance custom mock helpers I have enhanced the custom mocks defined in `tests/helpers.py` in a couple of important ways. 1. Automatically create AsyncMock attributes using `inspect` Our previous approach, hard-coding AsynckMock attributes for all the coroutine function methods defined for the class we are trying to mock is prone to human error and not resilient against changes introduced in updates of the library we are using. Instead, I have now created a helper method in our `CustomMockMixin` (formerly `GetChildMockMixin`) that automatically inspects the spec instance we've passed for `coroutine functions` using the `inspect` module. It then sets the according attributes with instances of the AsyncMock class. There is one caveat: `discord.py` very rarely defines regular methods that return a coroutine object. Since the returned coroutine should still be awaited, these regular methods should also be mocked with an AsyncMock. However, since they are regular methods, `inspect` does not detect them and they have to be added manually. (The only case of this I've found so far is `Client.wait_for`.) 2. Properly set special attributes using `kwargs.get` As we want attributes that point to other discord.py objects to use our custom mocks (.e.g, `Message.author` should use `MockMember`), the `__init__` method of our custom mocks make sure to correctly instantiate these attributes. However, the way we previously did that means we can't instantiate the custom mock with a mock instance we provide, since this special instantiation would overwrite the custom object we'd passed. I've solved this by using `kwargs.get`, with a new mock as the default value. This makes sure we only create a new mock if we didn't pass a custom one: ```py class MockMesseage: def __init__(self, **kwargs): self.author = kwargs.get('author', MockMember()) ``` As you can see, we will only create a new MockMember if we did not pass an `author` argument. 3. Factoring out duplicate lines Since our `CustomMockMixin` is a parent to all of our custom mock types, it makes sense to use it to factor out common code of all of our custom mocks. I've made the following changes: - Set a default child mock type in the mixin. - Create an `__init__` that takes care of the `inspect` of point 1 This means we won't have to repeat this in all of the child classes. 4. Three new Mock types: Emoji, PartialEmoji, and Reaction I have added three more custom mocks: - MockEmoji - MockPartialEmoji - MockReaction --- tests/helpers.py | 253 +++++++++++++++++++------------------------------- tests/test_helpers.py | 58 +++++++++++- 2 files changed, 150 insertions(+), 161 deletions(-) diff --git a/tests/helpers.py b/tests/helpers.py index 9375d0986..673beae3f 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -2,8 +2,9 @@ from __future__ import annotations import asyncio import functools +import inspect import unittest.mock -from typing import Iterable, Optional +from typing import Any, Iterable, Optional import discord from discord.ext.commands import Bot, Context @@ -48,9 +49,16 @@ class ColourMixin: self.colour = color -class GetChildMockMixin: +class CustomMockMixin: """Ensures attributes of our mock types will be instantiated with the correct mock type.""" + child_mock_type = unittest.mock.MagicMock + + def __init__(self, spec: Any = None, **kwargs): + super().__init__(spec=spec, **kwargs) + if spec: + self._extract_coroutine_methods_from_spec_instance(spec) + def _get_child_mock(self, **kw): """ Overwrite of the `_get_child_mock` method to stop the propagation of our custom mock classes. @@ -72,17 +80,20 @@ class GetChildMockMixin: return klass(**kw) + def _extract_coroutine_methods_from_spec_instance(self, source: Any) -> None: + """Automatically detect coroutine functions in `source` and set them as AsyncMock attributes.""" + for name, _method in inspect.getmembers(source, inspect.iscoroutinefunction): + setattr(self, name, AsyncMock()) + # TODO: Remove me in Python 3.8 -class AsyncMock(GetChildMockMixin, unittest.mock.MagicMock): +class AsyncMock(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock async callables. 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. """ - child_mock_type = unittest.mock.MagicMock - async def __call__(self, *args, **kwargs): return super(AsyncMock, self).__call__(*args, **kwargs) @@ -110,7 +121,7 @@ guild_data = { guild_instance = discord.Guild(data=guild_data, state=unittest.mock.MagicMock()) -class MockGuild(GetChildMockMixin, unittest.mock.Mock, HashableMixin): +class MockGuild(CustomMockMixin, unittest.mock.Mock, HashableMixin): """ A `Mock` subclass to mock `discord.Guild` objects. @@ -136,9 +147,6 @@ class MockGuild(GetChildMockMixin, unittest.mock.Mock, HashableMixin): For more info, see the `Mocking` section in `tests/README.md`. """ - - child_mock_type = unittest.mock.MagicMock - def __init__( self, guild_id: int = 1, @@ -158,39 +166,13 @@ class MockGuild(GetChildMockMixin, unittest.mock.Mock, HashableMixin): if members: self.members.extend(members) - # `discord.Guild` coroutines - self.create_category_channel = AsyncMock() - self.ban = AsyncMock() - self.bans = AsyncMock() - self.create_category = AsyncMock() - self.create_custom_emoji = AsyncMock() - self.create_role = AsyncMock() - self.create_text_channel = AsyncMock() - self.create_voice_channel = AsyncMock() - self.delete = AsyncMock() - self.edit = AsyncMock() - self.estimate_pruned_members = AsyncMock() - self.fetch_ban = AsyncMock() - self.fetch_channels = AsyncMock() - self.fetch_emoji = AsyncMock() - self.fetch_emojis = AsyncMock() - self.fetch_member = AsyncMock() - self.invites = AsyncMock() - self.kick = AsyncMock() - self.leave = AsyncMock() - self.prune_members = AsyncMock() - self.unban = AsyncMock() - self.vanity_invite = AsyncMock() - self.webhooks = AsyncMock() - self.widget = AsyncMock() - # Create a Role instance to get a realistic Mock of `discord.Role` role_data = {'name': 'role', 'id': 1} role_instance = discord.Role(guild=guild_instance, state=unittest.mock.MagicMock(), data=role_data) -class MockRole(GetChildMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): +class MockRole(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): """ A Mock subclass to mock `discord.Role` objects. @@ -208,10 +190,6 @@ class MockRole(GetChildMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin self.position = position self.mention = f'&{self.name}' - # 'discord.Role' coroutines - self.delete = AsyncMock() - self.edit = AsyncMock() - def __lt__(self, other): """Simplified position-based comparisons similar to those of `discord.Role`.""" return self.position < other.position @@ -223,16 +201,13 @@ state_mock = unittest.mock.MagicMock() member_instance = discord.Member(data=member_data, guild=guild_instance, state=state_mock) -class MockMember(GetChildMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): +class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): """ A Mock subclass to mock Member objects. Instances of this class will follow the specifications of `discord.Member` instances. For more information, see the `MockGuild` docstring. """ - - child_mock_type = unittest.mock.MagicMock - def __init__( self, name: str = "member", @@ -251,34 +226,18 @@ class MockMember(GetChildMockMixin, unittest.mock.Mock, ColourMixin, HashableMix self.mention = f"@{self.name}" - # `discord.Member` coroutines - self.add_roles = AsyncMock() - self.ban = AsyncMock() - self.edit = AsyncMock() - self.fetch_message = AsyncMock() - self.kick = AsyncMock() - self.move_to = AsyncMock() - self.pins = AsyncMock() - self.remove_roles = AsyncMock() - self.send = AsyncMock() - self.trigger_typing = AsyncMock() - self.unban = AsyncMock() - # Create a Bot instance to get a realistic MagicMock of `discord.ext.commands.Bot` bot_instance = Bot(command_prefix=unittest.mock.MagicMock()) -class MockBot(GetChildMockMixin, unittest.mock.MagicMock): +class MockBot(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Bot objects. Instances of this class will follow the specifications of `discord.ext.commands.Bot` instances. For more information, see the `MockGuild` docstring. """ - - child_mock_type = unittest.mock.MagicMock - def __init__(self, **kwargs) -> None: super().__init__(spec=bot_instance, **kwargs) @@ -286,69 +245,12 @@ class MockBot(GetChildMockMixin, unittest.mock.MagicMock): self.http_session = unittest.mock.MagicMock() self.api_client = unittest.mock.MagicMock() - # `discord.ext.commands.Bot` coroutines - self._before_invoke = AsyncMock() - self._after_invoke = AsyncMock() - self.application_info = AsyncMock() - self.change_presence = AsyncMock() - self.connect = AsyncMock() - self.close = AsyncMock() - self.create_guild = AsyncMock() - self.delete_invite = AsyncMock() - self.fetch_channel = AsyncMock() - self.fetch_guild = AsyncMock() - self.fetch_guilds = AsyncMock() - self.fetch_invite = AsyncMock() - self.fetch_user = AsyncMock() - self.fetch_user_profile = AsyncMock() - self.fetch_webhook = AsyncMock() - self.fetch_widget = AsyncMock() - self.get_context = AsyncMock() - self.get_prefix = AsyncMock() - self.invoke = AsyncMock() - self.is_owner = AsyncMock() - self.login = AsyncMock() - self.logout = AsyncMock() - self.on_command_error = AsyncMock() - self.on_error = AsyncMock() - self.process_commands = AsyncMock() - self.request_offline_members = AsyncMock() - self.start = AsyncMock() - self.wait_until_ready = AsyncMock() + # 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 + # is technically incorrect, since it's a regular def.) self.wait_for = AsyncMock() -# Create a Context instance to get a realistic MagicMock of `discord.ext.commands.Context` -context_instance = Context(message=unittest.mock.MagicMock(), prefix=unittest.mock.MagicMock()) - - -class MockContext(GetChildMockMixin, unittest.mock.MagicMock): - """ - A MagicMock subclass to mock Context objects. - - Instances of this class will follow the specifications of `discord.ext.commands.Context` - instances. For more information, see the `MockGuild` docstring. - """ - - child_mock_type = unittest.mock.MagicMock - - def __init__(self, **kwargs) -> None: - super().__init__(spec=context_instance, **kwargs) - self.bot = MockBot() - self.guild = MockGuild() - self.author = MockMember() - self.command = unittest.mock.MagicMock() - - # `discord.ext.commands.Context` coroutines - self.fetch_message = AsyncMock() - self.invoke = AsyncMock() - self.pins = AsyncMock() - self.reinvoke = AsyncMock() - self.send = AsyncMock() - self.send_help = AsyncMock() - self.trigger_typing = AsyncMock() - - # Create a TextChannel instance to get a realistic MagicMock of `discord.TextChannel` channel_data = { 'id': 1, @@ -365,39 +267,20 @@ guild = unittest.mock.MagicMock() channel_instance = discord.TextChannel(state=state, guild=guild, data=channel_data) -class MockTextChannel(GetChildMockMixin, unittest.mock.Mock, HashableMixin): +class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): """ A MagicMock subclass to mock TextChannel objects. Instances of this class will follow the specifications of `discord.TextChannel` instances. For more information, see the `MockGuild` docstring. """ - - child_mock_type = unittest.mock.MagicMock - 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 = MockGuild() + self.guild = kwargs.get('guild', MockGuild()) self.mention = f"#{self.name}" - # `discord.TextChannel` coroutines - self.clone = AsyncMock() - self.create_invite = AsyncMock() - self.create_webhook = AsyncMock() - self.delete = AsyncMock() - self.delete_messages = AsyncMock() - self.edit = AsyncMock() - self.fetch_message = AsyncMock() - self.invites = AsyncMock() - self.pins = AsyncMock() - self.purge = AsyncMock() - self.send = AsyncMock() - self.set_permissions = AsyncMock() - self.trigger_typing = AsyncMock() - self.webhooks = AsyncMock() - # Create a Message instance to get a realistic MagicMock of `discord.Message` message_data = { @@ -421,27 +304,83 @@ channel = unittest.mock.MagicMock() message_instance = discord.Message(state=state, channel=channel, data=message_data) -class MockMessage(GetChildMockMixin, unittest.mock.MagicMock): +# Create a Context instance to get a realistic MagicMock of `discord.ext.commands.Context` +context_instance = Context(message=unittest.mock.MagicMock(), prefix=unittest.mock.MagicMock()) + + +class MockContext(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Context objects. + + 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()) + 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): """ A MagicMock subclass to mock Message objects. 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()) + self.channel = kwargs.get('channel', MockTextChannel()) - child_mock_type = unittest.mock.MagicMock +emoji_data = {'require_colons': True, 'managed': True, 'id': 1, 'name': 'hyperlemon'} +emoji_instance = discord.Emoji(guild=MockGuild(), state=unittest.mock.MagicMock(), data=emoji_data) + + +class MockEmoji(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Emoji objects. + + 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=message_instance, **kwargs) - self.author = MockMember() - self.channel = MockTextChannel() - - # `discord.Message` coroutines - self.ack = AsyncMock() - self.add_reaction = AsyncMock() - self.clear_reactions = AsyncMock() - self.delete = AsyncMock() - self.edit = AsyncMock() - self.pin = AsyncMock() - self.remove_reaction = AsyncMock() - self.unpin = AsyncMock() + super().__init__(spec=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') + + +class MockPartialEmoji(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock PartialEmoji objects. + + 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) + + +reaction_instance = discord.Reaction(message=MockMessage(), data={'me': True}, emoji=MockEmoji()) + + +class MockReaction(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Reaction objects. + + 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()) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 62007ff4e..2b58634dd 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -221,7 +221,7 @@ class DiscordMocksTests(unittest.TestCase): @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.GetChildMockMixin, unittest.mock.MagicMock): + class FakeMockBot(helpers.CustomMockMixin, unittest.mock.MagicMock): """Fake MockBot class with invalid attribute/method `release_the_walrus`.""" child_mock_type = unittest.mock.MagicMock @@ -331,9 +331,9 @@ class MockObjectTests(unittest.TestCase): self.assertFalse(instance_one != instance_two) self.assertTrue(instance_one != instance_three) - def test_get_child_mock_mixin_accepts_mock_seal(self): - """The `GetChildMockMixin` should support `unittest.mock.seal`.""" - class MyMock(helpers.GetChildMockMixin, unittest.mock.MagicMock): + def test_custom_mock_mixin_accepts_mock_seal(self): + """The `CustomMockMixin` should support `unittest.mock.seal`.""" + class MyMock(helpers.CustomMockMixin, unittest.mock.MagicMock): child_mock_type = unittest.mock.MagicMock pass @@ -351,6 +351,10 @@ class MockObjectTests(unittest.TestCase): (helpers.MockMember, "display_name"), (helpers.MockBot, "owner_id"), (helpers.MockContext, "command_failed"), + (helpers.MockMessage, "mention_everyone"), + (helpers.MockEmoji, 'managed'), + (helpers.MockPartialEmoji, 'url'), + (helpers.MockReaction, 'me'), ) for mock_type, valid_attribute in test_values: @@ -360,6 +364,52 @@ class MockObjectTests(unittest.TestCase): attribute = getattr(mock, valid_attribute) self.assertTrue(isinstance(attribute, mock_type.child_mock_type)) + def test_extract_coroutine_methods_from_spec_instance_should_extract_all_and_only_coroutines(self): + """Test if all coroutine functions are extracted, but not regular methods or attributes.""" + class CoroutineDonor: + def __init__(self): + self.some_attribute = 'alpha' + + async def first_coroutine(): + """This coroutine function should be extracted.""" + + async def second_coroutine(): + """This coroutine function should be extracted.""" + + def regular_method(): + """This regular function should not be extracted.""" + + class Receiver: + pass + + donor = CoroutineDonor() + receiver = Receiver() + + helpers.CustomMockMixin._extract_coroutine_methods_from_spec_instance(receiver, donor) + + self.assertIsInstance(receiver.first_coroutine, helpers.AsyncMock) + self.assertIsInstance(receiver.second_coroutine, helpers.AsyncMock) + self.assertFalse(hasattr(receiver, 'regular_method')) + self.assertFalse(hasattr(receiver, 'some_attribute')) + + @unittest.mock.patch("builtins.super", new=unittest.mock.MagicMock()) + @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" + + helpers.CustomMockMixin(spec=spec) + + extract_method_mock.assert_called_once_with(spec) + + @unittest.mock.patch("builtins.super", new=unittest.mock.MagicMock()) + @unittest.mock.patch("tests.helpers.CustomMockMixin._extract_coroutine_methods_from_spec_instance") + def test_custom_mock_mixin_init_without_spec(self, extract_method_mock): + """Test if CustomMockMixin correctly passes on spec/kwargs and calls the extraction method.""" + helpers.CustomMockMixin() + + extract_method_mock.assert_not_called() + def test_async_mock_provides_coroutine_for_dunder_call(self): """Test if AsyncMock objects have a coroutine for their __call__ method.""" async_mock = helpers.AsyncMock() -- cgit v1.2.3 From 586ae18842de0dd92e93945c63ed5e6cd158c1f7 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 30 Oct 2019 22:54:27 +0100 Subject: Update docstring and remove redundant attribute I accidentally forgot to update the docstring of `CustomMockMixin`, which changed quite dramatically in scope with the last commit. This commit remedies that. In addition, I inadvertently forgot to remove the `child_mock_type` class attribute from `MockRole`. Since it uses the default value, it is no longer necessary to specify it in the child class as well. --- tests/helpers.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/helpers.py b/tests/helpers.py index 673beae3f..8496ba031 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -50,7 +50,15 @@ class ColourMixin: class CustomMockMixin: - """Ensures attributes of our mock types will be instantiated with the correct mock type.""" + """ + Provides common functionality for our custom Mock types. + + The cooperative `__init__` automatically creates `AsyncMock` attributes for every coroutine + function `inspect` detects in the `spec` instance we provide. In addition, this mixin takes care + of making sure child mocks are instantiated with the correct class. By default, the mock of the + children will be `unittest.mock.MagicMock`, but this can be overwritten by setting the attribute + `child_mock_type` on the custom mock inheriting from this mixin. + """ child_mock_type = unittest.mock.MagicMock @@ -179,9 +187,6 @@ 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. """ - - child_mock_type = unittest.mock.MagicMock - def __init__(self, name: str = "role", role_id: int = 1, position: int = 1, **kwargs) -> None: super().__init__(spec=role_instance, **kwargs) -- cgit v1.2.3 From eee3d54318b3edd5eeb7229f4175323b139e754a Mon Sep 17 00:00:00 2001 From: Dan Herrera Date: Thu, 31 Oct 2019 21:59:50 -0400 Subject: Use timedelta to correctly calculate next midnight --- bot/cogs/reddit.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index f947a7d78..0d06e9c26 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -2,7 +2,7 @@ import asyncio import logging import random import textwrap -from datetime import datetime +from datetime import datetime, timedelta from typing import List from discord import Colour, Embed, TextChannel @@ -130,7 +130,8 @@ class Reddit(Cog): """Post the top 5 posts daily, and the top 5 posts weekly.""" # once we upgrade to d.py 1.3 this can be removed and the loop can use the `time=datetime.time.min` parameter now = datetime.utcnow() - midnight_tomorrow = now.replace(day=now.day + 1, hour=0, minute=0, second=0) + tomorrow = now + timedelta(days=1) + midnight_tomorrow = tomorrow.replace(hour=0, minute=0, second=0) seconds_until = (midnight_tomorrow - now).total_seconds() await asyncio.sleep(seconds_until) -- cgit v1.2.3