From 1140e9690644e46196a1c8cad900272ffb3ae09a Mon Sep 17 00:00:00 2001
From: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
Date: Mon, 20 Apr 2020 18:46:30 +0200
Subject: Replace `in_channel` decorator by `in_whitelisted_context`

The `in_channel` decorator that served as a factory for `in_channel` checks was replaced by the broaded `in_whitelisted_context` decorator. This means that we can now whitelist commands using channel IDs, category IDs, and/or role IDs. The whitelists will be applied in an "OR" fashion, meaning that as soon as some part of the context happens to be whitelisted, the `predicate` check the decorator produces will return `True`.

To reflect that this is now a broader decorator that checks for a whitelisted *context* (as opposed to just whitelisted channels), the exception the predicate raises has been changed to `InWhitelistedContextCheckFailure` to reflect the broader scope of the decorator.

I've updated all the commands that used the previous version, `in_channel`, to use the replacement.
---
 tests/bot/cogs/test_information.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'tests')

diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py
index 3c26374f5..4a36fe030 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 InWhitelistedContextCheckFailure
 from tests import helpers
 
 
@@ -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(InWhitelistedContextCheckFailure, 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)
-- 
cgit v1.2.3


From 00291d7d5f859e4131cb5c94541a90f80f358376 Mon Sep 17 00:00:00 2001
From: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
Date: Mon, 20 Apr 2020 18:53:31 +0200
Subject: Remove vestigial kwargs from MockTextChannel.__init__

---
 tests/helpers.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'tests')

diff --git a/tests/helpers.py b/tests/helpers.py
index 8e13f0f28..9001deedf 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -315,7 +315,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))
 
-- 
cgit v1.2.3


From 57e69925af9a941dfe32acc0431a9699eda027f5 Mon Sep 17 00:00:00 2001
From: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
Date: Mon, 20 Apr 2020 18:57:12 +0200
Subject: Add tests for `in_whitelisted_context` decorator

I have added tests for the new `in_whitelisted_context` decorator. They work by calling the decorator with different kwargs to generate a specific predicate callable. That callable is then called to assess if it comes to the right conclusion.
---
 tests/bot/test_decorators.py | 115 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)
 create mode 100644 tests/bot/test_decorators.py

(limited to 'tests')

diff --git a/tests/bot/test_decorators.py b/tests/bot/test_decorators.py
new file mode 100644
index 000000000..fae7c0c52
--- /dev/null
+++ b/tests/bot/test_decorators.py
@@ -0,0 +1,115 @@
+import collections
+import unittest
+import unittest.mock
+
+from bot.decorators import InWhitelistedContextCheckFailure, in_whitelisted_context
+from tests import helpers
+
+
+WhitelistedContextTestCase = collections.namedtuple("WhitelistedContextTestCase", ("kwargs", "ctx"))
+
+
+class InWhitelistedContextTests(unittest.TestCase):
+    """Tests for the `in_whitelisted_context` 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.non_staff_member = helpers.MockMember()
+        cls.staff_role = helpers.MockRole(id=121212)
+        cls.staff_member = helpers.MockMember(roles=(cls.staff_role,))
+
+        cls.whitelisted_channels = (cls.bot_commands.id,)
+        cls.whitelisted_categories = (cls.help_channel.category_id,)
+        cls.whitelisted_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 = (
+            # Commands issued in whitelisted channels by members without whitelisted roles
+            WhitelistedContextTestCase(
+                kwargs={"whitelisted_channels": self.whitelisted_channels},
+                ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member)
+            ),
+            # `redirect_channel` should be added implicitly to the `whitelisted_channels`
+            WhitelistedContextTestCase(
+                kwargs={"redirect_channel": self.bot_commands.id},
+                ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member)
+            ),
+
+            # Commands issued in a whitelisted category by members without whitelisted roles
+            WhitelistedContextTestCase(
+                kwargs={"whitelisted_categories": self.whitelisted_categories},
+                ctx=helpers.MockContext(channel=self.help_channel, author=self.non_staff_member)
+            ),
+
+            # Command issued by a staff member in a non-whitelisted channel/category
+            WhitelistedContextTestCase(
+                kwargs={"whitelisted_roles": self.whitelisted_roles},
+                ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.staff_member)
+            ),
+
+            # With all kwargs provided
+            WhitelistedContextTestCase(
+                kwargs={
+                    "whitelisted_channels": self.whitelisted_channels,
+                    "whitelisted_categories": self.whitelisted_categories,
+                    "whitelisted_roles": self.whitelisted_roles,
+                    "redirect_channel": self.bot_commands,
+                },
+                ctx=helpers.MockContext(channel=self.help_channel, author=self.staff_member)
+            ),
+        )
+
+        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_whitelisted_context(**test_case.kwargs)
+
+            with self.subTest(test_case=test_case):
+                self.assertTrue(predicate(test_case.ctx))
+
+    def test_predicate_raises_exception_for_non_whitelisted_context(self):
+        """The predicate should raise `InWhitelistedContextCheckFailure` for a non-whitelisted context."""
+        test_cases = (
+            # Failing check with `redirect_channel`
+            WhitelistedContextTestCase(
+                kwargs={
+                    "whitelisted_categories": self.whitelisted_categories,
+                    "whitelisted_channels": self.whitelisted_channels,
+                    "whitelisted_roles": self.whitelisted_roles,
+                    "redirect_channel": self.bot_commands.id,
+                },
+                ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member)
+            ),
+
+            # Failing check without `redirect_channel`
+            WhitelistedContextTestCase(
+                kwargs={
+                    "whitelisted_categories": self.whitelisted_categories,
+                    "whitelisted_channels": self.whitelisted_channels,
+                    "whitelisted_roles": self.whitelisted_roles,
+                },
+                ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member)
+            ),
+        )
+
+        for test_case in test_cases:
+            # Create expected exception message based on whether or not a redirect channel was provided
+            expected_message = "Sorry, but you are not allowed to use that command here."
+            if test_case.kwargs.get("redirect_channel"):
+                expected_message += f" Please use the <#{test_case.kwargs['redirect_channel']}> channel instead."
+
+            # 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_whitelisted_context(**test_case.kwargs)
+
+            with self.subTest(test_case=test_case):
+                with self.assertRaises(InWhitelistedContextCheckFailure, msg=expected_message):
+                    predicate(test_case.ctx)
-- 
cgit v1.2.3


From b20bb7471b8d1d01f217f0620f8597bf1bae4456 Mon Sep 17 00:00:00 2001
From: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
Date: Thu, 23 Apr 2020 15:51:58 +0200
Subject: Simplify `in_whitelisted_context` decorator API

The API of the `in_whitelisted_context` decorator was a bit clunky:

- The long parameter names frequently required multiline decorators

- Despite `#bot-commands` being the defacto default, it needed to be passed

- The name of the function, `in_whitelisted_context` is fairly long in itself

To shorten the call length of the decorator, the parameter names were shortened by dropping the `whitelisted_` prefix. This means that the parameter names are now just `channels`, `categories`, and `roles`. This already means that all current usages of the decorator are reduced to one line.

In addition, `#bot-commands` has now been made the default redirect channel for the decorator. This means that if no `redirect` was passed, users will be redirected to `bot-commands` to use the command. If needed, `None` (or any falsey value) can be passed to disable redirection. Passing another channel id will trigger that channel to be used as the redirection target instead of bot-commands.

Finally, the name of the decorator was shortened to `in_whitelist`, which already communicates what it is supposed to do.
---
 bot/cogs/error_handler.py          |  6 ++--
 bot/cogs/information.py            | 10 ++-----
 bot/cogs/snekbox.py                |  9 ++----
 bot/cogs/utils.py                  |  8 ++---
 bot/cogs/verification.py           | 18 ++++--------
 bot/decorators.py                  | 49 +++++++++++++++----------------
 tests/bot/cogs/test_information.py |  4 +--
 tests/bot/test_decorators.py       | 60 +++++++++++++++++++-------------------
 8 files changed, 72 insertions(+), 92 deletions(-)

(limited to 'tests')

diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py
index 3f56a9798..b2f4c59f6 100644
--- a/bot/cogs/error_handler.py
+++ b/bot/cogs/error_handler.py
@@ -9,7 +9,7 @@ from bot.api import ResponseCodeError
 from bot.bot import Bot
 from bot.constants import Channels
 from bot.converters import TagNameConverter
-from bot.decorators import InWhitelistedContextCheckFailure
+from bot.decorators import InWhitelistCheckFailure
 
 log = logging.getLogger(__name__)
 
@@ -202,7 +202,7 @@ class ErrorHandler(Cog):
         * BotMissingRole
         * BotMissingAnyRole
         * NoPrivateMessage
-        * InWhitelistedContextCheckFailure
+        * InWhitelistCheckFailure
         """
         bot_missing_errors = (
             errors.BotMissingPermissions,
@@ -215,7 +215,7 @@ class ErrorHandler(Cog):
             await ctx.send(
                 f"Sorry, it looks like I don't have the permissions or roles I need to do that."
             )
-        elif isinstance(e, (InWhitelistedContextCheckFailure, errors.NoPrivateMessage)):
+        elif isinstance(e, (InWhitelistCheckFailure, errors.NoPrivateMessage)):
             ctx.bot.stats.incr("errors.wrong_channel_or_dm_error")
             await ctx.send(e)
 
diff --git a/bot/cogs/information.py b/bot/cogs/information.py
index 6b3fc0c96..4eb36c340 100644
--- a/bot/cogs/information.py
+++ b/bot/cogs/information.py
@@ -12,7 +12,7 @@ from discord.utils import escape_markdown
 
 from bot import constants
 from bot.bot import Bot
-from bot.decorators import InWhitelistedContextCheckFailure, in_whitelisted_context, with_role
+from bot.decorators import InWhitelistCheckFailure, in_whitelist, with_role
 from bot.pagination import LinePaginator
 from bot.utils.checks import cooldown_with_role_bypass, with_role_check
 from bot.utils.time import time_since
@@ -152,7 +152,7 @@ class Information(Cog):
         # Non-staff may only do this in #bot-commands
         if not with_role_check(ctx, *constants.STAFF_ROLES):
             if not ctx.channel.id == constants.Channels.bot_commands:
-                raise InWhitelistedContextCheckFailure(constants.Channels.bot_commands)
+                raise InWhitelistCheckFailure(constants.Channels.bot_commands)
 
         embed = await self.create_user_embed(ctx, user)
 
@@ -331,11 +331,7 @@ class Information(Cog):
 
     @cooldown_with_role_bypass(2, 60 * 3, BucketType.member, bypass_roles=constants.STAFF_ROLES)
     @group(invoke_without_command=True)
-    @in_whitelisted_context(
-        whitelisted_channels=(constants.Channels.bot_commands,),
-        whitelisted_roles=constants.STAFF_ROLES,
-        redirect_channel=constants.Channels.bot_commands,
-    )
+    @in_whitelist(channels=(constants.Channels.bot_commands,), roles=constants.STAFF_ROLES)
     async def raw(self, ctx: Context, *, message: Message, json: bool = False) -> None:
         """Shows information about the raw API response."""
         # I *guess* it could be deleted right as the command is invoked but I felt like it wasn't worth handling
diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py
index 4999074b6..8d4688114 100644
--- a/bot/cogs/snekbox.py
+++ b/bot/cogs/snekbox.py
@@ -13,7 +13,7 @@ from discord.ext.commands import Cog, Context, command, guild_only
 
 from bot.bot import Bot
 from bot.constants import Categories, Channels, Roles, URLs
-from bot.decorators import in_whitelisted_context
+from bot.decorators import in_whitelist
 from bot.utils.messages import wait_for_deletion
 
 log = logging.getLogger(__name__)
@@ -269,12 +269,7 @@ class Snekbox(Cog):
 
     @command(name="eval", aliases=("e",))
     @guild_only()
-    @in_whitelisted_context(
-        whitelisted_channels=EVAL_CHANNELS,
-        whitelisted_categories=EVAL_CATEGORIES,
-        whitelisted_roles=EVAL_ROLES,
-        redirect_channel=Channels.bot_commands,
-    )
+    @in_whitelist(channels=EVAL_CHANNELS, categories=EVAL_CATEGORIES, roles=EVAL_ROLES)
     async def eval_command(self, ctx: Context, *, code: str = None) -> None:
         """
         Run Python code and get the results.
diff --git a/bot/cogs/utils.py b/bot/cogs/utils.py
index 234ec514d..8023eb962 100644
--- a/bot/cogs/utils.py
+++ b/bot/cogs/utils.py
@@ -13,7 +13,7 @@ from discord.ext.commands import BadArgument, Cog, Context, command
 
 from bot.bot import Bot
 from bot.constants import Channels, MODERATION_ROLES, Mention, STAFF_ROLES
-from bot.decorators import in_whitelisted_context, with_role
+from bot.decorators import in_whitelist, with_role
 from bot.utils.time import humanize_delta
 
 log = logging.getLogger(__name__)
@@ -118,11 +118,7 @@ class Utils(Cog):
         await ctx.message.channel.send(embed=pep_embed)
 
     @command()
-    @in_whitelisted_context(
-        whitelisted_channels=(Channels.bot_commands,),
-        whitelisted_roles=STAFF_ROLES,
-        redirect_channel=Channels.bot_commands,
-    )
+    @in_whitelist(channels=(Channels.bot_commands,), roles=STAFF_ROLES)
     async def charinfo(self, ctx: Context, *, characters: str) -> None:
         """Shows you information on up to 25 unicode characters."""
         match = re.match(r"<(a?):(\w+):(\d+)>", characters)
diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py
index 040f52fbf..388b7a338 100644
--- a/bot/cogs/verification.py
+++ b/bot/cogs/verification.py
@@ -9,7 +9,7 @@ from discord.ext.commands import Cog, Context, command
 from bot import constants
 from bot.bot import Bot
 from bot.cogs.moderation import ModLog
-from bot.decorators import InWhitelistedContextCheckFailure, in_whitelisted_context, without_role
+from bot.decorators import InWhitelistCheckFailure, in_whitelist, without_role
 from bot.utils.checks import without_role_check
 
 log = logging.getLogger(__name__)
@@ -122,7 +122,7 @@ class Verification(Cog):
 
     @command(name='accept', aliases=('verify', 'verified', 'accepted'), hidden=True)
     @without_role(constants.Roles.verified)
-    @in_whitelisted_context(whitelisted_channels=(constants.Channels.verification,))
+    @in_whitelist(channels=(constants.Channels.verification,))
     async def accept_command(self, ctx: Context, *_) -> None:  # We don't actually care about the args
         """Accept our rules and gain access to the rest of the server."""
         log.debug(f"{ctx.author} called !accept. Assigning the 'Developer' role.")
@@ -138,10 +138,7 @@ class Verification(Cog):
                 await ctx.message.delete()
 
     @command(name='subscribe')
-    @in_whitelisted_context(
-        whitelisted_channels=(constants.Channels.bot_commands,),
-        redirect_channel=constants.Channels.bot_commands,
-    )
+    @in_whitelist(channels=(constants.Channels.bot_commands,))
     async def subscribe_command(self, ctx: Context, *_) -> None:  # We don't actually care about the args
         """Subscribe to announcement notifications by assigning yourself the role."""
         has_role = False
@@ -165,10 +162,7 @@ class Verification(Cog):
         )
 
     @command(name='unsubscribe')
-    @in_whitelisted_context(
-        whitelisted_channels=(constants.Channels.bot_commands,),
-        redirect_channel=constants.Channels.bot_commands,
-    )
+    @in_whitelist(channels=(constants.Channels.bot_commands,))
     async def unsubscribe_command(self, ctx: Context, *_) -> None:  # We don't actually care about the args
         """Unsubscribe from announcement notifications by removing the role from yourself."""
         has_role = False
@@ -193,8 +187,8 @@ class Verification(Cog):
 
     # This cannot be static (must have a __func__ attribute).
     async def cog_command_error(self, ctx: Context, error: Exception) -> None:
-        """Check for & ignore any InWhitelistedContextCheckFailure."""
-        if isinstance(error, InWhitelistedContextCheckFailure):
+        """Check for & ignore any InWhitelistCheckFailure."""
+        if isinstance(error, InWhitelistCheckFailure):
             error.handled = True
 
     @staticmethod
diff --git a/bot/decorators.py b/bot/decorators.py
index 149564d18..2ee5879f2 100644
--- a/bot/decorators.py
+++ b/bot/decorators.py
@@ -11,30 +11,34 @@ from discord.errors import NotFound
 from discord.ext import commands
 from discord.ext.commands import CheckFailure, Cog, Context
 
-from bot.constants import ERROR_REPLIES, RedirectOutput
+from bot.constants import Channels, ERROR_REPLIES, RedirectOutput
 from bot.utils.checks import with_role_check, without_role_check
 
 log = logging.getLogger(__name__)
 
 
-class InWhitelistedContextCheckFailure(CheckFailure):
+class InWhitelistCheckFailure(CheckFailure):
     """Raised when the `in_whitelist` check fails."""
 
-    def __init__(self, redirect_channel: Optional[int] = None):
-        error_message = "Sorry, but you are not allowed to use that command here."
+    def __init__(self, redirect_channel: Optional[int]) -> None:
+        self.redirect_channel = redirect_channel
 
         if redirect_channel:
-            error_message += f" Please use the <#{redirect_channel}> channel instead."
+            redirect_message = f" here. Please use the <#{redirect_channel}> channel instead"
+        else:
+            redirect_message = ""
+
+        error_message = f"You are not allowed to use that command{redirect_message}."
 
         super().__init__(error_message)
 
 
-def in_whitelisted_context(
+def in_whitelist(
     *,
-    whitelisted_channels: Container[int] = (),
-    whitelisted_categories: Container[int] = (),
-    whitelisted_roles: Container[int] = (),
-    redirect_channel: Optional[int] = None,
+    channels: Container[int] = (),
+    categories: Container[int] = (),
+    roles: Container[int] = (),
+    redirect: Optional[int] = Channels.bot_commands,
 
 ) -> Callable:
     """
@@ -46,45 +50,40 @@ def in_whitelisted_context(
     - `categories`: a container with category ids for whitelisted categories
     - `roles`: a container with with role ids for whitelisted roles
 
-    An optional `redirect_channel` can be provided to redirect users that are not
-    authorized to use the command in the current context. If no such channel is
-    provided, the users are simply told that they are not authorized to use the
-    command.
+    If the command was invoked in a context that was not whitelisted, the member is either
+    redirected to the `redirect` channel that was passed (default: #bot-commands) or simply
+    told that they're not allowed to use this particular command (if `None` was passed).
     """
-    if redirect_channel and redirect_channel not in whitelisted_channels:
+    if redirect and redirect not in channels:
         # It does not make sense for the channel whitelist to not contain the redirection
-        # channel (if provided). That's why we add the redirection channel to the `channels`
+        # channel (if applicable). That's why we add the redirection channel to the `channels`
         # container if it's not already in it. As we allow any container type to be passed,
         # we first create a tuple in order to safely add the redirection channel.
         #
         # Note: It's possible for the redirect channel to be in a whitelisted category, but
         # there's no easy way to check that and as a channel can easily be moved in and out of
         # categories, it's probably not wise to rely on its category in any case.
-        whitelisted_channels = tuple(whitelisted_channels) + (redirect_channel,)
+        channels = tuple(channels) + (redirect,)
 
     def predicate(ctx: Context) -> bool:
         """Check if a command was issued in a whitelisted context."""
-        if whitelisted_channels and ctx.channel.id in whitelisted_channels:
+        if channels and ctx.channel.id in channels:
             log.trace(f"{ctx.author} may use the `{ctx.command.name}` command as they are in a whitelisted channel.")
             return True
 
         # Only check the category id if we have a category whitelist and the channel has a `category_id`
-        if (
-            whitelisted_categories
-            and hasattr(ctx.channel, "category_id")
-            and ctx.channel.category_id in whitelisted_categories
-        ):
+        if categories and hasattr(ctx.channel, "category_id") and ctx.channel.category_id in categories:
             log.trace(f"{ctx.author} may use the `{ctx.command.name}` command as they are in a whitelisted category.")
             return True
 
         # Only check the roles whitelist if we have one and ensure the author's roles attribute returns
         # an iterable to prevent breakage in DM channels (for if we ever decide to enable commands there).
-        if whitelisted_roles and any(r.id in whitelisted_roles for r in getattr(ctx.author, "roles", ())):
+        if roles and any(r.id in roles for r in getattr(ctx.author, "roles", ())):
             log.trace(f"{ctx.author} may use the `{ctx.command.name}` command as they have a whitelisted role.")
             return True
 
         log.trace(f"{ctx.author} may not use the `{ctx.command.name}` command within this context.")
-        raise InWhitelistedContextCheckFailure(redirect_channel)
+        raise InWhitelistCheckFailure(redirect)
 
     return commands.check(predicate)
 
diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py
index 4a36fe030..6dace1080 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 InWhitelistedContextCheckFailure
+from bot.decorators import InWhitelistCheckFailure
 from tests import helpers
 
 
@@ -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(InWhitelistedContextCheckFailure, 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/test_decorators.py b/tests/bot/test_decorators.py
index fae7c0c52..645051fec 100644
--- a/tests/bot/test_decorators.py
+++ b/tests/bot/test_decorators.py
@@ -2,15 +2,15 @@ import collections
 import unittest
 import unittest.mock
 
-from bot.decorators import InWhitelistedContextCheckFailure, in_whitelisted_context
+from bot.decorators import InWhitelistCheckFailure, in_whitelist
 from tests import helpers
 
 
 WhitelistedContextTestCase = collections.namedtuple("WhitelistedContextTestCase", ("kwargs", "ctx"))
 
 
-class InWhitelistedContextTests(unittest.TestCase):
-    """Tests for the `in_whitelisted_context` check."""
+class InWhitelistTests(unittest.TestCase):
+    """Tests for the `in_whitelist` check."""
 
     @classmethod
     def setUpClass(cls):
@@ -23,43 +23,43 @@ class InWhitelistedContextTests(unittest.TestCase):
         cls.staff_role = helpers.MockRole(id=121212)
         cls.staff_member = helpers.MockMember(roles=(cls.staff_role,))
 
-        cls.whitelisted_channels = (cls.bot_commands.id,)
-        cls.whitelisted_categories = (cls.help_channel.category_id,)
-        cls.whitelisted_roles = (cls.staff_role.id,)
+        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 = (
             # Commands issued in whitelisted channels by members without whitelisted roles
             WhitelistedContextTestCase(
-                kwargs={"whitelisted_channels": self.whitelisted_channels},
+                kwargs={"channels": self.channels},
                 ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member)
             ),
-            # `redirect_channel` should be added implicitly to the `whitelisted_channels`
+            # `redirect` should be added implicitly to the `channels`
             WhitelistedContextTestCase(
-                kwargs={"redirect_channel": self.bot_commands.id},
+                kwargs={"redirect": self.bot_commands.id},
                 ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member)
             ),
 
             # Commands issued in a whitelisted category by members without whitelisted roles
             WhitelistedContextTestCase(
-                kwargs={"whitelisted_categories": self.whitelisted_categories},
+                kwargs={"categories": self.categories},
                 ctx=helpers.MockContext(channel=self.help_channel, author=self.non_staff_member)
             ),
 
             # Command issued by a staff member in a non-whitelisted channel/category
             WhitelistedContextTestCase(
-                kwargs={"whitelisted_roles": self.whitelisted_roles},
+                kwargs={"roles": self.roles},
                 ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.staff_member)
             ),
 
             # With all kwargs provided
             WhitelistedContextTestCase(
                 kwargs={
-                    "whitelisted_channels": self.whitelisted_channels,
-                    "whitelisted_categories": self.whitelisted_categories,
-                    "whitelisted_roles": self.whitelisted_roles,
-                    "redirect_channel": self.bot_commands,
+                    "channels": self.channels,
+                    "categories": self.categories,
+                    "roles": self.roles,
+                    "redirect": self.bot_commands,
                 },
                 ctx=helpers.MockContext(channel=self.help_channel, author=self.staff_member)
             ),
@@ -69,31 +69,31 @@ class InWhitelistedContextTests(unittest.TestCase):
             # 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_whitelisted_context(**test_case.kwargs)
+                predicate = in_whitelist(**test_case.kwargs)
 
             with self.subTest(test_case=test_case):
                 self.assertTrue(predicate(test_case.ctx))
 
     def test_predicate_raises_exception_for_non_whitelisted_context(self):
-        """The predicate should raise `InWhitelistedContextCheckFailure` for a non-whitelisted context."""
+        """The predicate should raise `InWhitelistCheckFailure` for a non-whitelisted context."""
         test_cases = (
-            # Failing check with `redirect_channel`
+            # Failing check with `redirect`
             WhitelistedContextTestCase(
                 kwargs={
-                    "whitelisted_categories": self.whitelisted_categories,
-                    "whitelisted_channels": self.whitelisted_channels,
-                    "whitelisted_roles": self.whitelisted_roles,
-                    "redirect_channel": self.bot_commands.id,
+                    "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)
             ),
 
-            # Failing check without `redirect_channel`
+            # Failing check without `redirect`
             WhitelistedContextTestCase(
                 kwargs={
-                    "whitelisted_categories": self.whitelisted_categories,
-                    "whitelisted_channels": self.whitelisted_channels,
-                    "whitelisted_roles": self.whitelisted_roles,
+                    "categories": self.categories,
+                    "channels": self.channels,
+                    "roles": self.roles,
                 },
                 ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.non_staff_member)
             ),
@@ -102,14 +102,14 @@ class InWhitelistedContextTests(unittest.TestCase):
         for test_case in test_cases:
             # Create expected exception message based on whether or not a redirect channel was provided
             expected_message = "Sorry, but you are not allowed to use that command here."
-            if test_case.kwargs.get("redirect_channel"):
-                expected_message += f" Please use the <#{test_case.kwargs['redirect_channel']}> channel instead."
+            if test_case.kwargs.get("redirect"):
+                expected_message += f" Please use the <#{test_case.kwargs['redirect']}> channel instead."
 
             # 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_whitelisted_context(**test_case.kwargs)
+                predicate = in_whitelist(**test_case.kwargs)
 
             with self.subTest(test_case=test_case):
-                with self.assertRaises(InWhitelistedContextCheckFailure, msg=expected_message):
+                with self.assertRaises(InWhitelistCheckFailure, msg=expected_message):
                     predicate(test_case.ctx)
-- 
cgit v1.2.3


From f5bb251bbfd92bfe67ee9638f2bf6d054eb30502 Mon Sep 17 00:00:00 2001
From: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
Date: Mon, 27 Apr 2020 16:01:45 +0200
Subject: Exclude never-run lines from coverage

---
 tests/bot/cogs/test_cogs.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'tests')

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}."
-- 
cgit v1.2.3


From 167f57b9cc78708b7c6b48f64442d7bddce2f75c Mon Sep 17 00:00:00 2001
From: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
Date: Mon, 27 Apr 2020 16:02:15 +0200
Subject: Add mock for discord.DMChannels

---
 tests/helpers.py | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

(limited to 'tests')

diff --git a/tests/helpers.py b/tests/helpers.py
index 9001deedf..2b79a6c2a 100644
--- a/tests/helpers.py
+++ b/tests/helpers.py
@@ -323,6 +323,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,
-- 
cgit v1.2.3


From d21e5962be961a267cef6ffef4f7d4aaf1114a08 Mon Sep 17 00:00:00 2001
From: Sebastiaan Zeeff <sebastiaan.zeeff@gmail.com>
Date: Mon, 27 Apr 2020 16:03:12 +0200
Subject: Add DMChannel tests for in_whitelist decorator

The `in_whitelist` decorator should not fail when a decorated command was called in a DMChannel; it should simply conclude that the user is not allowed to use the command. I've added a test case that uses a DMChannel context with User, not Member, objects.

In addition, I've opted to display a test case description in the `subTest`: Simply printing the actual arguments and context is messy and does not actually show you the information you'd like. This description is enough to figure out which test is failing and what the gist of the test is.
---
 tests/bot/test_decorators.py | 94 +++++++++++++++++++++++++++++---------------
 1 file changed, 63 insertions(+), 31 deletions(-)

(limited to 'tests')

diff --git a/tests/bot/test_decorators.py b/tests/bot/test_decorators.py
index 645051fec..a17dd3e16 100644
--- a/tests/bot/test_decorators.py
+++ b/tests/bot/test_decorators.py
@@ -2,11 +2,12 @@ import collections
 import unittest
 import unittest.mock
 
+from bot import constants
 from bot.decorators import InWhitelistCheckFailure, in_whitelist
 from tests import helpers
 
 
-WhitelistedContextTestCase = collections.namedtuple("WhitelistedContextTestCase", ("kwargs", "ctx"))
+InWhitelistTestCase = collections.namedtuple("WhitelistedContextTestCase", ("kwargs", "ctx", "description"))
 
 
 class InWhitelistTests(unittest.TestCase):
@@ -18,6 +19,7 @@ class InWhitelistTests(unittest.TestCase):
         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)
@@ -30,38 +32,35 @@ class InWhitelistTests(unittest.TestCase):
     def test_predicate_returns_true_for_whitelisted_context(self):
         """The predicate should return `True` if a whitelisted context was passed to it."""
         test_cases = (
-            # Commands issued in whitelisted channels by members without whitelisted roles
-            WhitelistedContextTestCase(
+            InWhitelistTestCase(
                 kwargs={"channels": self.channels},
-                ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member)
+                ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member),
+                description="In whitelisted channels by members without whitelisted roles",
             ),
-            # `redirect` should be added implicitly to the `channels`
-            WhitelistedContextTestCase(
+            InWhitelistTestCase(
                 kwargs={"redirect": self.bot_commands.id},
-                ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member)
+                ctx=helpers.MockContext(channel=self.bot_commands, author=self.non_staff_member),
+                description="`redirect` should be implicitly added to `channels`",
             ),
-
-            # Commands issued in a whitelisted category by members without whitelisted roles
-            WhitelistedContextTestCase(
+            InWhitelistTestCase(
                 kwargs={"categories": self.categories},
-                ctx=helpers.MockContext(channel=self.help_channel, author=self.non_staff_member)
+                ctx=helpers.MockContext(channel=self.help_channel, author=self.non_staff_member),
+                description="Whitelisted category without whitelisted role",
             ),
-
-            # Command issued by a staff member in a non-whitelisted channel/category
-            WhitelistedContextTestCase(
+            InWhitelistTestCase(
                 kwargs={"roles": self.roles},
-                ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.staff_member)
+                ctx=helpers.MockContext(channel=self.non_whitelisted_channel, author=self.staff_member),
+                description="Whitelisted role outside of whitelisted channel/category"
             ),
-
-            # With all kwargs provided
-            WhitelistedContextTestCase(
+            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)
+                ctx=helpers.MockContext(channel=self.help_channel, author=self.staff_member),
+                description="Case with all whitelist kwargs used",
             ),
         )
 
@@ -71,45 +70,78 @@ class InWhitelistTests(unittest.TestCase):
             with unittest.mock.patch("bot.decorators.commands.check", new=lambda predicate: predicate):
                 predicate = in_whitelist(**test_case.kwargs)
 
-            with self.subTest(test_case=test_case):
+            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 `redirect`
-            WhitelistedContextTestCase(
+            # 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)
+                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`
-            WhitelistedContextTestCase(
+            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.non_whitelisted_channel, author=self.non_staff_member)
+                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:
-            # Create expected exception message based on whether or not a redirect channel was provided
-            expected_message = "Sorry, but you are not allowed to use that command here."
-            if test_case.kwargs.get("redirect"):
-                expected_message += f" Please use the <#{test_case.kwargs['redirect']}> channel instead."
+            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_case=test_case):
-                with self.assertRaises(InWhitelistCheckFailure, msg=expected_message):
+            with self.subTest(test_description=test_case.description):
+                with self.assertRaisesRegex(InWhitelistCheckFailure, exception_message):
                     predicate(test_case.ctx)
-- 
cgit v1.2.3


From b43379d663a86680f762d20a7bd27a20927d4bfc Mon Sep 17 00:00:00 2001
From: MarkKoz <KozlovMark@gmail.com>
Date: Thu, 30 Apr 2020 18:35:03 -0700
Subject: Tests: change avatar_url_as assertion to use static_format

---
 tests/bot/cogs/test_information.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'tests')

diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py
index 6dace1080..b5f928dd6 100644
--- a/tests/bot/cogs/test_information.py
+++ b/tests/bot/cogs/test_information.py
@@ -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")
 
 
-- 
cgit v1.2.3