Merge branch 'master' into ban-kick-reason-length

6 files changed, 183 insertions, 11 deletions

diff --git a/tests/bot/cogs/sync/test_base.py b/tests/bot/cogs/sync/test_base.py
index 6ee9dfda6..70aea2bab 100644
--- a/tests/bot/cogs/sync/test_base.py
+++ b/tests/bot/cogs/sync/test_base.py
@@ -1,3 +1,4 @@
+import asyncio
 import unittest
 from unittest import mock
 
@@ -211,7 +212,7 @@ class SyncerConfirmationTests(unittest.IsolatedAsyncioTestCase):
         subtests = (
             (constants.Emojis.check_mark, True, None),
             ("InVaLiD", False, None),
-            (None, False, TimeoutError),
+            (None, False, asyncio.TimeoutError),
         )
 
         for emoji, ret_val, side_effect in subtests:
diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py
index 39f6492cb..fdda59a8f 100644
--- a/tests/bot/cogs/test_cogs.py
+++ b/tests/bot/cogs/test_cogs.py
@@ -31,7 +31,7 @@ class CommandNameTests(unittest.TestCase):
     def walk_modules() -> t.Iterator[ModuleType]:
         """Yield imported modules from the bot.cogs subpackage."""
         def on_error(name: str) -> t.NoReturn:
-            raise ImportError(name=name)
+            raise ImportError(name=name)  # pragma: no cover
 
         # The mock prevents asyncio.get_event_loop() from being called.
         with mock.patch("discord.ext.tasks.loop"):
@@ -71,7 +71,7 @@ class CommandNameTests(unittest.TestCase):
 
             for name in self.get_qualified_names(cmd):
                 with self.subTest(cmd=func_name, name=name):
-                    if name in all_names:
+                    if name in all_names:  # pragma: no cover
                         conflicts = ", ".join(all_names.get(name, ""))
                         self.fail(
                             f"Name '{name}' of the command {func_name} conflicts with {conflicts}."
diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py
index 3c26374f5..b5f928dd6 100644
--- a/tests/bot/cogs/test_information.py
+++ b/tests/bot/cogs/test_information.py
@@ -7,7 +7,7 @@ import discord
 
 from bot import constants
 from bot.cogs import information
-from bot.decorators import InChannelCheckFailure
+from bot.decorators import InWhitelistCheckFailure
 from tests import helpers
 
 
@@ -485,7 +485,7 @@ class UserEmbedTests(unittest.TestCase):
         user.avatar_url_as.return_value = "avatar url"
         embed = asyncio.run(self.cog.create_user_embed(ctx, user))
 
-        user.avatar_url_as.assert_called_once_with(format="png")
+        user.avatar_url_as.assert_called_once_with(static_format="png")
         self.assertEqual(embed.thumbnail.url, "avatar url")
 
 
@@ -525,7 +525,7 @@ class UserCommandTests(unittest.TestCase):
         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):
+        with self.assertRaises(InWhitelistCheckFailure, msg=msg):
             asyncio.run(self.cog.user_info.callback(self.cog, ctx))
 
     @unittest.mock.patch("bot.cogs.information.Information.create_user_embed", new_callable=unittest.mock.AsyncMock)
diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py
index 1dec0ccaf..14299e766 100644
--- a/tests/bot/cogs/test_snekbox.py
+++ b/tests/bot/cogs/test_snekbox.py
@@ -208,10 +208,9 @@ class SnekboxTests(unittest.IsolatedAsyncioTestCase):
 
     async def test_eval_command_call_help(self):
         """Test if the eval command call the help command if no code is provided."""
-        ctx = MockContext()
-        ctx.invoke = AsyncMock()
+        ctx = MockContext(command="sentinel")
         await self.cog.eval_command.callback(self.cog, ctx=ctx, code='')
-        ctx.invoke.assert_called_once_with(self.bot.get_command("help"), "eval")
+        ctx.send_help.assert_called_once_with("sentinel")
 
     async def test_send_eval(self):
         """Test the send_eval function."""
@@ -291,7 +290,11 @@ class SnekboxTests(unittest.IsolatedAsyncioTestCase):
         self.assertEqual(actual, expected)
         self.bot.wait_for.assert_has_awaits(
             (
-                call('message_edit', check=partial_mock(snekbox.predicate_eval_message_edit, ctx), timeout=10),
+                call(
+                    'message_edit',
+                    check=partial_mock(snekbox.predicate_eval_message_edit, ctx),
+                    timeout=snekbox.REEVAL_TIMEOUT,
+                ),
                 call('reaction_add', check=partial_mock(snekbox.predicate_eval_emoji_reaction, ctx), timeout=10)
             )
         )
diff --git a/tests/bot/test_decorators.py b/tests/bot/test_decorators.py
new file mode 100644
index 000000000..a17dd3e16
--- /dev/null
+++ b/tests/bot/test_decorators.py
@@ -0,0 +1,147 @@
+import collections
+import unittest
+import unittest.mock
+
+from bot import constants
+from bot.decorators import InWhitelistCheckFailure, in_whitelist
+from tests import helpers
+
+
+InWhitelistTestCase = collections.namedtuple("WhitelistedContextTestCase", ("kwargs", "ctx", "description"))
+
+
+class InWhitelistTests(unittest.TestCase):
+    """Tests for the `in_whitelist` check."""
+
+    @classmethod
+    def setUpClass(cls):
+        """Set up helpers that only need to be defined once."""
+        cls.bot_commands = helpers.MockTextChannel(id=123456789, category_id=123456)
+        cls.help_channel = helpers.MockTextChannel(id=987654321, category_id=987654)
+        cls.non_whitelisted_channel = helpers.MockTextChannel(id=666666)
+        cls.dm_channel = helpers.MockDMChannel()
+
+        cls.non_staff_member = helpers.MockMember()
+        cls.staff_role = helpers.MockRole(id=121212)
+        cls.staff_member = helpers.MockMember(roles=(cls.staff_role,))
+
+        cls.channels = (cls.bot_commands.id,)
+        cls.categories = (cls.help_channel.category_id,)
+        cls.roles = (cls.staff_role.id,)
+
+    def test_predicate_returns_true_for_whitelisted_context(self):
+        """The predicate should return `True` if a whitelisted context was passed to it."""
+        test_cases = (
+            InWhitelistTestCase(
+                kwargs={"channels": self.channels},
+                ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member),
+                description="In whitelisted channels by members without whitelisted roles",
+            ),
+            InWhitelistTestCase(
+                kwargs={"redirect": self.bot_commands.id},
+                ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member),
+                description="`redirect` should be implicitly added to `channels`",
+            ),
+            InWhitelistTestCase(
+                kwargs={"categories": self.categories},
+                ctx=helpers.MockContext(channel=self.help_channel, author=self.non_staff_member),
+                description="Whitelisted category without whitelisted role",
+            ),
+            InWhitelistTestCase(
+                kwargs={"roles": self.roles},
+                ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.staff_member),
+                description="Whitelisted role outside of whitelisted channel/category"
+            ),
+            InWhitelistTestCase(
+                kwargs={
+                    "channels": self.channels,
+                    "categories": self.categories,
+                    "roles": self.roles,
+                    "redirect": self.bot_commands,
+                },
+                ctx=helpers.MockContext(channel=self.help_channel, author=self.staff_member),
+                description="Case with all whitelist kwargs used",
+            ),
+        )
+
+        for test_case in test_cases:
+            # patch `commands.check` with a no-op lambda that just returns the predicate passed to it
+            # so we can test the predicate that was generated from the specified kwargs.
+            with unittest.mock.patch("bot.decorators.commands.check", new=lambda predicate: predicate):
+                predicate = in_whitelist(**test_case.kwargs)
+
+            with self.subTest(test_description=test_case.description):
+                self.assertTrue(predicate(test_case.ctx))
+
+    def test_predicate_raises_exception_for_non_whitelisted_context(self):
+        """The predicate should raise `InWhitelistCheckFailure` for a non-whitelisted context."""
+        test_cases = (
+            # Failing check with explicit `redirect`
+            InWhitelistTestCase(
+                kwargs={
+                    "categories": self.categories,
+                    "channels": self.channels,
+                    "roles": self.roles,
+                    "redirect": self.bot_commands.id,
+                },
+                ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member),
+                description="Failing check with an explicit redirect channel",
+            ),
+
+            # Failing check with implicit `redirect`
+            InWhitelistTestCase(
+                kwargs={
+                    "categories": self.categories,
+                    "channels": self.channels,
+                    "roles": self.roles,
+                },
+                ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member),
+                description="Failing check with an implicit redirect channel",
+            ),
+
+            # Failing check without `redirect`
+            InWhitelistTestCase(
+                kwargs={
+                    "categories": self.categories,
+                    "channels": self.channels,
+                    "roles": self.roles,
+                    "redirect": None,
+                },
+                ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member),
+                description="Failing check without a redirect channel",
+            ),
+
+            # Command issued in DM channel
+            InWhitelistTestCase(
+                kwargs={
+                    "categories": self.categories,
+                    "channels": self.channels,
+                    "roles": self.roles,
+                    "redirect": None,
+                },
+                ctx=helpers.MockContext(channel=self.dm_channel, author=self.dm_channel.me),
+                description="Commands issued in DM channel should be rejected",
+            ),
+        )
+
+        for test_case in test_cases:
+            if "redirect" not in test_case.kwargs or test_case.kwargs["redirect"] is not None:
+                # There are two cases in which we have a redirect channel:
+                #   1. No redirect channel was passed; the default value of `bot_commands` is used
+                #   2. An explicit `redirect` is set that is "not None"
+                redirect_channel = test_case.kwargs.get("redirect", constants.Channels.bot_commands)
+                redirect_message = f" here. Please use the <#{redirect_channel}> channel instead"
+            else:
+                # If an explicit `None` was passed for `redirect`, there is no redirect channel
+                redirect_message = ""
+
+            exception_message = f"You are not allowed to use that command{redirect_message}."
+
+            # patch `commands.check` with a no-op lambda that just returns the predicate passed to it
+            # so we can test the predicate that was generated from the specified kwargs.
+            with unittest.mock.patch("bot.decorators.commands.check", new=lambda predicate: predicate):
+                predicate = in_whitelist(**test_case.kwargs)
+
+            with self.subTest(test_description=test_case.description):
+                with self.assertRaisesRegex(InWhitelistCheckFailure, exception_message):
+                    predicate(test_case.ctx)
diff --git a/tests/helpers.py b/tests/helpers.py
index 227bac95f..91d814b3a 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -319,7 +319,7 @@ class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin):
     """
     spec_set = channel_instance
 
-    def __init__(self, name: str = 'channel', channel_id: int = 1, **kwargs) -> None:
+    def __init__(self, **kwargs) -> None:
         default_kwargs = {'id': next(self.discord_id), 'name': 'channel', 'guild': MockGuild()}
         super().__init__(**collections.ChainMap(kwargs, default_kwargs))
 
@@ -327,6 +327,27 @@ class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin):
             self.mention = f"#{self.name}"
 
 
+# Create data for the DMChannel instance
+state = unittest.mock.MagicMock()
+me = unittest.mock.MagicMock()
+dm_channel_data = {"id": 1, "recipients": [unittest.mock.MagicMock()]}
+dm_channel_instance = discord.DMChannel(me=me, state=state, data=dm_channel_data)
+
+
+class MockDMChannel(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.
+    """
+    spec_set = dm_channel_instance
+
+    def __init__(self, **kwargs) -> None:
+        default_kwargs = {'id': next(self.discord_id), 'recipient': MockUser(), "me": MockUser()}
+        super().__init__(**collections.ChainMap(kwargs, default_kwargs))
+
+
 # Create a Message instance to get a realistic MagicMock of `discord.Message`
 message_data = {
     'id': 1,