From fc2224fc047fbbefdc17e3624ebc2854342c59c1 Mon Sep 17 00:00:00 2001 From: "Karlis. S" Date: Sun, 1 Mar 2020 09:35:34 +0200 Subject: !roles Command Test: Applied !roles command changes --- tests/bot/cogs/test_information.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 8443cfe71..bb4ebd9d0 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -45,10 +45,9 @@ class InformationCogTests(unittest.TestCase): _, kwargs = self.ctx.send.call_args embed = kwargs.pop('embed') - self.assertEqual(embed.title, "Role information") + self.assertEqual(embed.title, "Role information (Total 1 roles)") self.assertEqual(embed.colour, discord.Colour.blurple()) - self.assertEqual(embed.description, f"`{self.moderator_role.id}` - {self.moderator_role.mention}\n") - self.assertEqual(embed.footer.text, "Total roles: 1") + self.assertEqual(embed.description, f"\n`{self.moderator_role.id}` - {self.moderator_role.mention}\n\n") def test_role_info_command(self): """Tests the `role info` command.""" -- cgit v1.2.3 From 28bcbf334eb08dfcd35b898b7cb803338664ee61 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 2 Mar 2020 09:42:00 -0800 Subject: Add more pre-commit hooks * Remove trailing whitespaces * Specify error code for a noqa in the free command --- .pre-commit-config.yaml | 23 ++++++++++++++++++++--- CONTRIBUTING.md | 2 +- bot/cogs/free.py | 2 +- tests/README.md | 10 +++++----- 4 files changed, 27 insertions(+), 10 deletions(-) (limited to 'tests') diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 860357868..4bb5e7e1c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,10 +1,27 @@ +exclude: ^\.cache/|\.venv/|\.git/|htmlcov/|logs/ repos: -- repo: local + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v2.5.0 hooks: - - id: flake8 + - id: check-merge-conflict + - id: check-toml + - id: check-yaml + args: [--unsafe] # Required due to custom constructors (e.g. !ENV) + - id: end-of-file-fixer + - id: mixed-line-ending + args: [--fix=lf] + - id: trailing-whitespace + args: [--markdown-linebreak-ext=md] + - repo: https://github.com/pre-commit/pygrep-hooks + rev: v1.5.1 + hooks: + - id: python-check-blanket-noqa + - repo: local + hooks: + - id: flake8 name: Flake8 description: This hook runs flake8 within our project's pipenv environment. entry: pipenv run lint language: python types: [python] - require_serial: true \ No newline at end of file + require_serial: true diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 39f76c7b4..61d11f844 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -43,7 +43,7 @@ To provide a standalone development environment for this project, docker compose When pulling down changes from GitHub, remember to sync your environment using `pipenv sync --dev` to ensure you're using the most up-to-date versions the project's dependencies. ### Type Hinting -[PEP 484](https://www.python.org/dev/peps/pep-0484/) formally specifies type hints for Python functions, added to the Python Standard Library in version 3.5. Type hints are recognized by most modern code editing tools and provide useful insight into both the input and output types of a function, preventing the user from having to go through the codebase to determine these types. +[PEP 484](https://www.python.org/dev/peps/pep-0484/) formally specifies type hints for Python functions, added to the Python Standard Library in version 3.5. Type hints are recognized by most modern code editing tools and provide useful insight into both the input and output types of a function, preventing the user from having to go through the codebase to determine these types. For example: diff --git a/bot/cogs/free.py b/bot/cogs/free.py index 02c02d067..33b55e79a 100644 --- a/bot/cogs/free.py +++ b/bot/cogs/free.py @@ -55,7 +55,7 @@ class Free(Cog): msg = messages[seek - 1] # Otherwise get last message else: - msg = await channel.history(limit=1).next() # noqa (False positive) + msg = await channel.history(limit=1).next() # noqa: B305 inactive = (datetime.utcnow() - msg.created_at).seconds if inactive > TIMEOUT: diff --git a/tests/README.md b/tests/README.md index be78821bf..4f62edd68 100644 --- a/tests/README.md +++ b/tests/README.md @@ -83,7 +83,7 @@ TagContentConverter should return correct values for valid input. As we are trying to test our "units" of code independently, we want to make sure that we do not rely objects and data generated by "external" code. If we we did, then we wouldn't know if the failure we're observing was caused by the code we are actually trying to test or something external to it. -However, the features that we are trying to test often depend on those objects generated by external pieces of code. It would be difficult to test a bot command without having access to a `Context` instance. Fortunately, there's a solution for that: we use fake objects that act like the true object. We call these fake objects "mocks". +However, the features that we are trying to test often depend on those objects generated by external pieces of code. It would be difficult to test a bot command without having access to a `Context` instance. Fortunately, there's a solution for that: we use fake objects that act like the true object. We call these fake objects "mocks". To create these mock object, we mainly use the [`unittest.mock`](https://docs.python.org/3/library/unittest.mock.html) module. In addition, we have also defined a couple of specialized mock objects that mock specific `discord.py` types (see the section on the below.). @@ -114,13 +114,13 @@ class BotCogTests(unittest.TestCase): ### Mocking coroutines -By default, the `unittest.mock.Mock` and `unittest.mock.MagicMock` classes cannot mock coroutines, since the `__call__` method they provide is synchronous. In anticipation of the `AsyncMock` that will be [introduced in Python 3.8](https://docs.python.org/3.9/whatsnew/3.8.html#unittest), we have added an `AsyncMock` helper to [`helpers.py`](/tests/helpers.py). Do note that this drop-in replacement only implements an asynchronous `__call__` method, not the additional assertions that will come with the new `AsyncMock` type in Python 3.8. +By default, the `unittest.mock.Mock` and `unittest.mock.MagicMock` classes cannot mock coroutines, since the `__call__` method they provide is synchronous. In anticipation of the `AsyncMock` that will be [introduced in Python 3.8](https://docs.python.org/3.9/whatsnew/3.8.html#unittest), we have added an `AsyncMock` helper to [`helpers.py`](/tests/helpers.py). Do note that this drop-in replacement only implements an asynchronous `__call__` method, not the additional assertions that will come with the new `AsyncMock` type in Python 3.8. ### Special mocks for some `discord.py` types To quote Ned Batchelder, Mock objects are "automatic chameleons". This means that they will happily allow the access to any attribute or method and provide a mocked value in return. One downside to this is that if the code you are testing gets the name of the attribute wrong, your mock object will not complain and the test may still pass. -In order to avoid that, we have defined a number of Mock types in [`helpers.py`](/tests/helpers.py) that follow the specifications of the actual Discord types they are mocking. This means that trying to access an attribute or method on a mocked object that does not exist on the equivalent `discord.py` object will result in an `AttributeError`. In addition, these mocks have some sensible defaults and **pass `isinstance` checks for the types they are mocking**. +In order to avoid that, we have defined a number of Mock types in [`helpers.py`](/tests/helpers.py) that follow the specifications of the actual Discord types they are mocking. This means that trying to access an attribute or method on a mocked object that does not exist on the equivalent `discord.py` object will result in an `AttributeError`. In addition, these mocks have some sensible defaults and **pass `isinstance` checks for the types they are mocking**. These special mocks are added when they are needed, so if you think it would be sensible to add another one, feel free to propose one in your PR. @@ -144,7 +144,7 @@ Finally, there are some considerations to make when writing tests, both for writ ### Test coverage is a starting point -Having test coverage is a good starting point for unit testing: If a part of your code was not covered by a test, we know that we have not tested it properly. The reverse is unfortunately not true: Even if the code we are testing has 100% branch coverage, it does not mean it's fully tested or guaranteed to work. +Having test coverage is a good starting point for unit testing: If a part of your code was not covered by a test, we know that we have not tested it properly. The reverse is unfortunately not true: Even if the code we are testing has 100% branch coverage, it does not mean it's fully tested or guaranteed to work. One problem is that 100% branch coverage may be misleading if we haven't tested our code against all the realistic input it may get in production. For instance, take a look at the following `member_information` function and the test we've written for it: @@ -169,7 +169,7 @@ class FunctionsTests(unittest.TestCase): If you were to run this test, not only would the function pass the test, `coverage.py` will also tell us that the test provides 100% branch coverage for the function. Can you spot the bug the test suite did not catch? -The problem here is that we have only tested our function with a member object that had `None` for the `member.joined` attribute. This means that `member.joined.stfptime("%d-%m-%Y")` was never executed during our test, leading to us missing the spelling mistake in `stfptime` (it should be `strftime`). +The problem here is that we have only tested our function with a member object that had `None` for the `member.joined` attribute. This means that `member.joined.stfptime("%d-%m-%Y")` was never executed during our test, leading to us missing the spelling mistake in `stfptime` (it should be `strftime`). Adding another test would not increase the test coverage we have, but it does ensure that we'll notice that this function can fail with realistic data: -- cgit v1.2.3 From aae928ebc06e7e7a6ed5b5b848464ce95e4ea9d8 Mon Sep 17 00:00:00 2001 From: "S. Co1" Date: Tue, 3 Mar 2020 22:53:19 -0500 Subject: Remove CaseInsensitiveDict This was added by the now-removed Snake cog & is not used elsewhere on bot. --- bot/utils/__init__.py | 57 ------------------------------------------------- tests/bot/test_utils.py | 37 -------------------------------- 2 files changed, 94 deletions(-) delete mode 100644 tests/bot/test_utils.py (limited to 'tests') diff --git a/bot/utils/__init__.py b/bot/utils/__init__.py index 3e4b15ce4..9b32e515d 100644 --- a/bot/utils/__init__.py +++ b/bot/utils/__init__.py @@ -1,5 +1,4 @@ from abc import ABCMeta -from typing import Any, Hashable from discord.ext.commands import CogMeta @@ -8,59 +7,3 @@ class CogABCMeta(CogMeta, ABCMeta): """Metaclass for ABCs meant to be implemented as Cogs.""" pass - - -class CaseInsensitiveDict(dict): - """ - We found this class on StackOverflow. Thanks to m000 for writing it! - - https://stackoverflow.com/a/32888599/4022104 - """ - - @classmethod - def _k(cls, key: Hashable) -> Hashable: - """Return lowered key if a string-like is passed, otherwise pass key straight through.""" - return key.lower() if isinstance(key, str) else key - - def __init__(self, *args, **kwargs): - super(CaseInsensitiveDict, self).__init__(*args, **kwargs) - self._convert_keys() - - def __getitem__(self, key: Hashable) -> Any: - """Case insensitive __setitem__.""" - return super(CaseInsensitiveDict, self).__getitem__(self.__class__._k(key)) - - def __setitem__(self, key: Hashable, value: Any): - """Case insensitive __setitem__.""" - super(CaseInsensitiveDict, self).__setitem__(self.__class__._k(key), value) - - def __delitem__(self, key: Hashable) -> Any: - """Case insensitive __delitem__.""" - return super(CaseInsensitiveDict, self).__delitem__(self.__class__._k(key)) - - def __contains__(self, key: Hashable) -> bool: - """Case insensitive __contains__.""" - return super(CaseInsensitiveDict, self).__contains__(self.__class__._k(key)) - - def pop(self, key: Hashable, *args, **kwargs) -> Any: - """Case insensitive pop.""" - return super(CaseInsensitiveDict, self).pop(self.__class__._k(key), *args, **kwargs) - - def get(self, key: Hashable, *args, **kwargs) -> Any: - """Case insensitive get.""" - return super(CaseInsensitiveDict, self).get(self.__class__._k(key), *args, **kwargs) - - def setdefault(self, key: Hashable, *args, **kwargs) -> Any: - """Case insensitive setdefault.""" - return super(CaseInsensitiveDict, self).setdefault(self.__class__._k(key), *args, **kwargs) - - def update(self, E: Any = None, **F) -> None: - """Case insensitive update.""" - super(CaseInsensitiveDict, self).update(self.__class__(E)) - super(CaseInsensitiveDict, self).update(self.__class__(**F)) - - def _convert_keys(self) -> None: - """Helper method to lowercase all existing string-like keys.""" - for k in list(self.keys()): - v = super(CaseInsensitiveDict, self).pop(k) - self.__setitem__(k, v) diff --git a/tests/bot/test_utils.py b/tests/bot/test_utils.py deleted file mode 100644 index d7bcc3ba6..000000000 --- a/tests/bot/test_utils.py +++ /dev/null @@ -1,37 +0,0 @@ -import unittest - -from bot import utils - - -class CaseInsensitiveDictTests(unittest.TestCase): - """Tests for the `CaseInsensitiveDict` container.""" - - def test_case_insensitive_key_access(self): - """Tests case insensitive key access and storage.""" - instance = utils.CaseInsensitiveDict() - - key = 'LEMON' - value = 'trees' - - instance[key] = value - self.assertIn(key, instance) - self.assertEqual(instance.get(key), value) - self.assertEqual(instance.get(key.casefold()), value) - self.assertEqual(instance.pop(key.casefold()), value) - self.assertNotIn(key, instance) - self.assertNotIn(key.casefold(), instance) - - instance.setdefault(key, value) - del instance[key] - self.assertNotIn(key, instance) - - def test_initialization_from_kwargs(self): - """Tests creating the dictionary from keyword arguments.""" - instance = utils.CaseInsensitiveDict({'FOO': 'bar'}) - self.assertEqual(instance['foo'], 'bar') - - def test_update_from_other_mapping(self): - """Tests updating the dictionary from another mapping.""" - instance = utils.CaseInsensitiveDict() - instance.update({'FOO': 'bar'}) - self.assertEqual(instance['foo'], 'bar') -- cgit v1.2.3 From 2c85b2241bd8a1e7ca8290cd385cded97c54f9bb Mon Sep 17 00:00:00 2001 From: "S. Co1" Date: Tue, 3 Mar 2020 22:59:07 -0500 Subject: Update code for pep8-naming compliance --- tests/base.py | 4 ++-- tests/bot/cogs/sync/test_base.py | 2 +- tests/bot/cogs/test_snekbox.py | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/base.py b/tests/base.py index 42174e911..d99b9ac31 100644 --- a/tests/base.py +++ b/tests/base.py @@ -31,7 +31,7 @@ class LoggingTestsMixin: """ @contextmanager - def assertNotLogs(self, logger=None, level=None, msg=None): + def assertNotLogs(self, logger=None, level=None, msg=None): # noqa: N802 """ Asserts that no logs of `level` and higher were emitted by `logger`. @@ -81,7 +81,7 @@ class LoggingTestsMixin: class CommandTestCase(unittest.IsolatedAsyncioTestCase): """TestCase with additional assertions that are useful for testing Discord commands.""" - async def assertHasPermissionsCheck( + async def assertHasPermissionsCheck( # noqa: N802 self, cmd: commands.Command, permissions: Dict[str, bool], diff --git a/tests/bot/cogs/sync/test_base.py b/tests/bot/cogs/sync/test_base.py index fe0594efe..6ee9dfda6 100644 --- a/tests/bot/cogs/sync/test_base.py +++ b/tests/bot/cogs/sync/test_base.py @@ -84,7 +84,7 @@ class SyncerSendPromptTests(unittest.IsolatedAsyncioTestCase): method.assert_called_once_with(constants.Channels.dev_core) - async def test_send_prompt_returns_None_if_channel_fetch_fails(self): + async def test_send_prompt_returns_none_if_channel_fetch_fails(self): """None should be returned if there's an HTTPException when fetching the channel.""" self.bot.get_channel.return_value = None self.bot.fetch_channel.side_effect = discord.HTTPException(mock.MagicMock(), "test error!") diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index 9cd7f0154..fd9468829 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -89,15 +89,15 @@ class SnekboxTests(unittest.IsolatedAsyncioTestCase): self.assertEqual(actual, expected) @patch('bot.cogs.snekbox.Signals', side_effect=ValueError) - def test_get_results_message_invalid_signal(self, mock_Signals: Mock): + def test_get_results_message_invalid_signal(self, mock_signals: Mock): self.assertEqual( self.cog.get_results_message({'stdout': '', 'returncode': 127}), ('Your eval job has completed with return code 127', '') ) @patch('bot.cogs.snekbox.Signals') - def test_get_results_message_valid_signal(self, mock_Signals: Mock): - mock_Signals.return_value.name = 'SIGTEST' + def test_get_results_message_valid_signal(self, mock_signals: Mock): + mock_signals.return_value.name = 'SIGTEST' self.assertEqual( self.cog.get_results_message({'stdout': '', 'returncode': 127}), ('Your eval job has completed with return code 127 (SIGTEST)', '') -- cgit v1.2.3 From 5579f2d32d5faadad778d64c50cf6fbefccf4f28 Mon Sep 17 00:00:00 2001 From: ks123 Date: Thu, 5 Mar 2020 09:05:06 +0200 Subject: (Information Cog, !roles command test): Applied empty parameter change. --- 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 c6fd937b8..7c265bba8 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -47,7 +47,7 @@ class InformationCogTests(unittest.TestCase): self.assertEqual(embed.title, "Role information (Total 1 roles)") self.assertEqual(embed.colour, discord.Colour.blurple()) - self.assertEqual(embed.description, f"\n`{self.moderator_role.id}` - {self.moderator_role.mention}\n\n") + self.assertEqual(embed.description, f"\n`{self.moderator_role.id}` - {self.moderator_role.mention}\n") def test_role_info_command(self): """Tests the `role info` command.""" -- cgit v1.2.3 From 0b75d3f5e717f99f53522d4224abea6223ef6c84 Mon Sep 17 00:00:00 2001 From: ks123 Date: Thu, 5 Mar 2020 09:08:10 +0200 Subject: (Information Cog, !roles command test): Removed 's' at end of "Total 1 role(s)" due changes in command. --- 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 7c265bba8..3c26374f5 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -45,7 +45,7 @@ class InformationCogTests(unittest.TestCase): _, kwargs = self.ctx.send.call_args embed = kwargs.pop('embed') - self.assertEqual(embed.title, "Role information (Total 1 roles)") + self.assertEqual(embed.title, "Role information (Total 1 role)") self.assertEqual(embed.colour, discord.Colour.blurple()) self.assertEqual(embed.description, f"\n`{self.moderator_role.id}` - {self.moderator_role.mention}\n") -- cgit v1.2.3 From d4253e106771f90a983717a994349d52337b2de9 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 8 Mar 2020 19:40:34 +0100 Subject: Add tests for FirstHash class. --- tests/bot/cogs/moderation/__init__.py | 0 tests/bot/cogs/moderation/test_silence.py | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 tests/bot/cogs/moderation/__init__.py create mode 100644 tests/bot/cogs/moderation/test_silence.py (limited to 'tests') diff --git a/tests/bot/cogs/moderation/__init__.py b/tests/bot/cogs/moderation/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py new file mode 100644 index 000000000..2a06f5944 --- /dev/null +++ b/tests/bot/cogs/moderation/test_silence.py @@ -0,0 +1,25 @@ +import unittest + +from bot.cogs.moderation.silence import FirstHash + + +class FirstHashTests(unittest.TestCase): + def setUp(self) -> None: + self.test_cases = ( + (FirstHash(0, 4), FirstHash(0, 5)), + (FirstHash("string", None), FirstHash("string", True)) + ) + + def test_hashes_equal(self): + """Check hashes equal with same first item.""" + + for tuple1, tuple2 in self.test_cases: + with self.subTest(tuple1=tuple1, tuple2=tuple2): + self.assertEqual(hash(tuple1), hash(tuple2)) + + def test_eq(self): + """Check objects are equal with same first item.""" + + for tuple1, tuple2 in self.test_cases: + with self.subTest(tuple1=tuple1, tuple2=tuple2): + self.assertTrue(tuple1 == tuple2) -- cgit v1.2.3 From e872176b452ceca1b639ef42d640e18656c7c0c9 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 8 Mar 2020 19:42:18 +0100 Subject: Add test case for Silence cog. --- tests/bot/cogs/moderation/test_silence.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 2a06f5944..1db2b6eec 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -1,6 +1,7 @@ import unittest -from bot.cogs.moderation.silence import FirstHash +from bot.cogs.moderation.silence import FirstHash, Silence +from tests.helpers import MockBot, MockContext class FirstHashTests(unittest.TestCase): @@ -23,3 +24,11 @@ class FirstHashTests(unittest.TestCase): for tuple1, tuple2 in self.test_cases: with self.subTest(tuple1=tuple1, tuple2=tuple2): self.assertTrue(tuple1 == tuple2) + + +class SilenceTests(unittest.TestCase): + def setUp(self) -> None: + + self.bot = MockBot() + self.cog = Silence(self.bot) + self.ctx = MockContext() -- cgit v1.2.3 From 1d83a5752aae483224129ee798e529f3d7d8e132 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 8 Mar 2020 19:42:51 +0100 Subject: Add test for `silence` discord output. --- tests/bot/cogs/moderation/test_silence.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 1db2b6eec..088410bee 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -1,6 +1,10 @@ +import asyncio import unittest +from functools import partial +from unittest import mock from bot.cogs.moderation.silence import FirstHash, Silence +from bot.constants import Emojis from tests.helpers import MockBot, MockContext @@ -32,3 +36,23 @@ class SilenceTests(unittest.TestCase): self.bot = MockBot() self.cog = Silence(self.bot) self.ctx = MockContext() + + def test_silence_sent_correct_discord_message(self): + """Check if proper message was sent when called with duration in channel with previous state.""" + test_cases = ( + ((self.cog, self.ctx, 0.0001), f"{Emojis.check_mark} #channel silenced for 0.0001 minute(s).", True,), + ((self.cog, self.ctx, None), f"{Emojis.check_mark} #channel silenced indefinitely.", True,), + ((self.cog, self.ctx, 5), f"{Emojis.cross_mark} #channel is already silenced.", False,), + ) + for silence_call_args, result_message, _silence_patch_return in test_cases: + with self.subTest( + silence_duration=silence_call_args[-1], + result_message=result_message, + starting_unsilenced_state=_silence_patch_return + ): + with mock.patch( + "bot.cogs.moderation.silence.Silence._silence", + new_callable=partial(mock.AsyncMock, return_value=_silence_patch_return) + ): + asyncio.run(self.cog.silence.callback(*silence_call_args)) + self.ctx.send.call_args.assert_called_once_with(result_message) -- cgit v1.2.3 From cfbe3b9742b5531bdced1d5b099739f01033a6bb Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 8 Mar 2020 22:20:00 +0100 Subject: Add test for `unsilence` discord output. --- tests/bot/cogs/moderation/test_silence.py | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 088410bee..17420ce7d 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -56,3 +56,12 @@ class SilenceTests(unittest.TestCase): ): asyncio.run(self.cog.silence.callback(*silence_call_args)) self.ctx.send.call_args.assert_called_once_with(result_message) + + def test_unsilence_sent_correct_discord_message(self): + """Check if proper message was sent to `alert_chanel`.""" + with mock.patch( + "bot.cogs.moderation.silence.Silence._unsilence", + new_callable=partial(mock.AsyncMock, return_value=True) + ): + asyncio.run(self.cog.unsilence.callback(self.cog, self.ctx)) + self.ctx.channel.send.call_args.assert_called_once_with(f"{Emojis.check_mark} Unsilenced #channel.") -- cgit v1.2.3 From ee94c38063981ee6770c1d263eab9c0d2e178380 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Mon, 9 Mar 2020 20:41:55 +0100 Subject: Use `patch.object` instead of patch with direct `return_value`. --- tests/bot/cogs/moderation/test_silence.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 17420ce7d..53b3fd388 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -1,6 +1,5 @@ import asyncio import unittest -from functools import partial from unittest import mock from bot.cogs.moderation.silence import FirstHash, Silence @@ -50,18 +49,12 @@ class SilenceTests(unittest.TestCase): result_message=result_message, starting_unsilenced_state=_silence_patch_return ): - with mock.patch( - "bot.cogs.moderation.silence.Silence._silence", - new_callable=partial(mock.AsyncMock, return_value=_silence_patch_return) - ): + with mock.patch.object(self.cog, "_silence", return_value=_silence_patch_return): asyncio.run(self.cog.silence.callback(*silence_call_args)) self.ctx.send.call_args.assert_called_once_with(result_message) def test_unsilence_sent_correct_discord_message(self): """Check if proper message was sent to `alert_chanel`.""" - with mock.patch( - "bot.cogs.moderation.silence.Silence._unsilence", - new_callable=partial(mock.AsyncMock, return_value=True) - ): + with mock.patch.object(self.cog, "_unsilence", return_value=True): asyncio.run(self.cog.unsilence.callback(self.cog, self.ctx)) self.ctx.channel.send.call_args.assert_called_once_with(f"{Emojis.check_mark} Unsilenced #channel.") -- cgit v1.2.3 From 60814ee9270d4c550047478bf8d4a179d7351696 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 9 Mar 2020 13:09:08 -0700 Subject: Cog tests: create boilerplate for command name tests --- tests/bot/cogs/test_cogs.py | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 tests/bot/cogs/test_cogs.py (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py new file mode 100644 index 000000000..6f5d07030 --- /dev/null +++ b/tests/bot/cogs/test_cogs.py @@ -0,0 +1,7 @@ +"""Test suite for general tests which apply to all cogs.""" + +import unittest + + +class CommandNameTests(unittest.TestCase): + """Tests for shadowing command names and aliases.""" -- cgit v1.2.3 From d31f7e3f4a4876d51119d5875afa9221b14b285e Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 9 Mar 2020 13:10:21 -0700 Subject: Cog tests: add a function to get all commands For tests, ideally creating instances of cogs should be avoided to avoid extra code execution. This function was copied over from discord.py because their function is not a static method, though it still works as one. It was probably just a design decision on their part to not make it static. --- tests/bot/cogs/test_cogs.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py index 6f5d07030..b128ca123 100644 --- a/tests/bot/cogs/test_cogs.py +++ b/tests/bot/cogs/test_cogs.py @@ -1,7 +1,19 @@ """Test suite for general tests which apply to all cogs.""" +import typing as t import unittest +from discord.ext import commands + class CommandNameTests(unittest.TestCase): """Tests for shadowing command names and aliases.""" + + @staticmethod + def walk_commands(cog: commands.Cog) -> t.Iterator[commands.Command]: + """An iterator that recursively walks through `cog`'s commands and subcommands.""" + for command in cog.__cog_commands__: + if command.parent is None: + yield command + if isinstance(command, commands.GroupMixin): + yield from command.walk_commands() -- cgit v1.2.3 From 85c439fbf78f59ff314f4f9daef1467d486709c3 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 00:01:58 +0100 Subject: Remove unnecessary args from test cases. Needless call args which were constant were kept in the test cases, resulting in redundant code, the args were moved directly into the function call. --- tests/bot/cogs/moderation/test_silence.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 53b3fd388..1341911d5 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -39,18 +39,18 @@ class SilenceTests(unittest.TestCase): def test_silence_sent_correct_discord_message(self): """Check if proper message was sent when called with duration in channel with previous state.""" test_cases = ( - ((self.cog, self.ctx, 0.0001), f"{Emojis.check_mark} #channel silenced for 0.0001 minute(s).", True,), - ((self.cog, self.ctx, None), f"{Emojis.check_mark} #channel silenced indefinitely.", True,), - ((self.cog, self.ctx, 5), f"{Emojis.cross_mark} #channel is already silenced.", False,), + (0.0001, f"{Emojis.check_mark} #channel silenced for 0.0001 minute(s).", True,), + (None, f"{Emojis.check_mark} #channel silenced indefinitely.", True,), + (5, f"{Emojis.cross_mark} #channel is already silenced.", False,), ) - for silence_call_args, result_message, _silence_patch_return in test_cases: + for duration, result_message, _silence_patch_return in test_cases: with self.subTest( - silence_duration=silence_call_args[-1], + silence_duration=duration, result_message=result_message, starting_unsilenced_state=_silence_patch_return ): with mock.patch.object(self.cog, "_silence", return_value=_silence_patch_return): - asyncio.run(self.cog.silence.callback(*silence_call_args)) + asyncio.run(self.cog.silence.callback(self.cog, self.ctx, duration)) self.ctx.send.call_args.assert_called_once_with(result_message) def test_unsilence_sent_correct_discord_message(self): -- cgit v1.2.3 From adaf456607ba2f2724c6fd34308cd170c81aa651 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 00:56:31 +0100 Subject: Remove channel mentions from output discord messages. With the removal of the channel args, it's no longer necessary to mention the channel in the command output. Tests adjusted accordingly --- bot/cogs/moderation/silence.py | 8 ++++---- tests/bot/cogs/moderation/test_silence.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'tests') diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 76c5a171d..68cad4062 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -92,13 +92,13 @@ class Silence(commands.Cog): If duration is forever, start a notifier loop that triggers every 15 minutes. """ if not await self._silence(ctx.channel, persistent=(duration is None), duration=duration): - await ctx.send(f"{Emojis.cross_mark} {ctx.channel.mention} is already silenced.") + await ctx.send(f"{Emojis.cross_mark} current channel is already silenced.") return if duration is None: - await ctx.send(f"{Emojis.check_mark} {ctx.channel.mention} silenced indefinitely.") + await ctx.send(f"{Emojis.check_mark} silenced current channel indefinitely.") return - await ctx.send(f"{Emojis.check_mark} {ctx.channel.mention} silenced for {duration} minute(s).") + await ctx.send(f"{Emojis.check_mark} silenced current channel for {duration} minute(s).") await asyncio.sleep(duration*60) await ctx.invoke(self.unsilence) @@ -110,7 +110,7 @@ class Silence(commands.Cog): Unsilence a previously silenced `channel` and remove it from indefinitely muted channels notice if applicable. """ if await self._unsilence(ctx.channel): - await ctx.send(f"{Emojis.check_mark} Unsilenced {ctx.channel.mention}.") + await ctx.send(f"{Emojis.check_mark} unsilenced current channel.") async def _silence(self, channel: TextChannel, persistent: bool, duration: Optional[int]) -> bool: """ diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 1341911d5..6da374a8f 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -39,9 +39,9 @@ class SilenceTests(unittest.TestCase): def test_silence_sent_correct_discord_message(self): """Check if proper message was sent when called with duration in channel with previous state.""" test_cases = ( - (0.0001, f"{Emojis.check_mark} #channel silenced for 0.0001 minute(s).", True,), - (None, f"{Emojis.check_mark} #channel silenced indefinitely.", True,), - (5, f"{Emojis.cross_mark} #channel is already silenced.", False,), + (0.0001, f"{Emojis.check_mark} silenced current channel for 0.0001 minute(s).", True,), + (None, f"{Emojis.check_mark} silenced current channel indefinitely.", True,), + (5, f"{Emojis.cross_mark} current channel is already silenced.", False,), ) for duration, result_message, _silence_patch_return in test_cases: with self.subTest( @@ -57,4 +57,4 @@ class SilenceTests(unittest.TestCase): """Check if proper message was sent to `alert_chanel`.""" with mock.patch.object(self.cog, "_unsilence", return_value=True): asyncio.run(self.cog.unsilence.callback(self.cog, self.ctx)) - self.ctx.channel.send.call_args.assert_called_once_with(f"{Emojis.check_mark} Unsilenced #channel.") + self.ctx.send.call_args.assert_called_once_with(f"{Emojis.check_mark} unsilenced current channel.") -- cgit v1.2.3 From 68d43946d1dc6393a4f7b8b4812b5c4787842c12 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 01:28:26 +0100 Subject: Add test for `_silence` method. --- tests/bot/cogs/moderation/test_silence.py | 35 ++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 6da374a8f..6a75db2a0 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -1,10 +1,11 @@ import asyncio import unittest from unittest import mock +from unittest.mock import Mock from bot.cogs.moderation.silence import FirstHash, Silence from bot.constants import Emojis -from tests.helpers import MockBot, MockContext +from tests.helpers import MockBot, MockContext, MockTextChannel class FirstHashTests(unittest.TestCase): @@ -35,6 +36,7 @@ class SilenceTests(unittest.TestCase): self.bot = MockBot() self.cog = Silence(self.bot) self.ctx = MockContext() + self.cog._verified_role = None def test_silence_sent_correct_discord_message(self): """Check if proper message was sent when called with duration in channel with previous state.""" @@ -58,3 +60,34 @@ class SilenceTests(unittest.TestCase): with mock.patch.object(self.cog, "_unsilence", return_value=True): asyncio.run(self.cog.unsilence.callback(self.cog, self.ctx)) self.ctx.send.call_args.assert_called_once_with(f"{Emojis.check_mark} unsilenced current channel.") + + def test_silence_private_for_false(self): + """Permissions are not set and `False` is returned in an already silenced channel.""" + perm_overwrite = Mock(send_messages=False) + channel = Mock(overwrites_for=Mock(return_value=perm_overwrite)) + + self.assertFalse(asyncio.run(self.cog._silence(channel, True, None))) + channel.set_permissions.assert_not_called() + + def test_silence_private_silenced_channel(self): + """Channel had `send_message` permissions revoked and was added to `muted_channels`.""" + channel = MockTextChannel() + muted_channels = Mock() + with mock.patch.object(self.cog, "muted_channels", new=muted_channels, create=True): + self.assertTrue(asyncio.run(self.cog._silence(channel, False, None))) + channel.set_permissions.assert_called_once() + self.assertFalse(channel.set_permissions.call_args.kwargs['overwrite'].send_messages) + muted_channels.add.call_args.assert_called_once_with(channel) + + def test_silence_private_notifier(self): + """Channel should be added to notifier with `persistent` set to `True`, and the other way around.""" + channel = MockTextChannel() + with mock.patch.object(self.cog, "notifier", create=True): + with self.subTest(persistent=True): + asyncio.run(self.cog._silence(channel, True, None)) + self.cog.notifier.add_channel.assert_called_once() + + with mock.patch.object(self.cog, "notifier", create=True): + with self.subTest(persistent=False): + asyncio.run(self.cog._silence(channel, False, None)) + self.cog.notifier.add_channel.assert_not_called() -- cgit v1.2.3 From fef8c8e8504d8431ae7cad23128733d0b9039c7a Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 02:31:36 +0100 Subject: Use async test case. This allows us to use coroutines with await directly instead of asyncio.run --- tests/bot/cogs/moderation/test_silence.py | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 6a75db2a0..33ff78ca6 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -1,4 +1,3 @@ -import asyncio import unittest from unittest import mock from unittest.mock import Mock @@ -30,15 +29,14 @@ class FirstHashTests(unittest.TestCase): self.assertTrue(tuple1 == tuple2) -class SilenceTests(unittest.TestCase): +class SilenceTests(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: - self.bot = MockBot() self.cog = Silence(self.bot) self.ctx = MockContext() self.cog._verified_role = None - def test_silence_sent_correct_discord_message(self): + async def test_silence_sent_correct_discord_message(self): """Check if proper message was sent when called with duration in channel with previous state.""" test_cases = ( (0.0001, f"{Emojis.check_mark} silenced current channel for 0.0001 minute(s).", True,), @@ -52,42 +50,42 @@ class SilenceTests(unittest.TestCase): starting_unsilenced_state=_silence_patch_return ): with mock.patch.object(self.cog, "_silence", return_value=_silence_patch_return): - asyncio.run(self.cog.silence.callback(self.cog, self.ctx, duration)) + await self.cog.silence.callback(self.cog, self.ctx, duration) self.ctx.send.call_args.assert_called_once_with(result_message) - def test_unsilence_sent_correct_discord_message(self): + async def test_unsilence_sent_correct_discord_message(self): """Check if proper message was sent to `alert_chanel`.""" with mock.patch.object(self.cog, "_unsilence", return_value=True): - asyncio.run(self.cog.unsilence.callback(self.cog, self.ctx)) + await self.cog.unsilence.callback(self.cog, self.ctx) self.ctx.send.call_args.assert_called_once_with(f"{Emojis.check_mark} unsilenced current channel.") - def test_silence_private_for_false(self): + async def test_silence_private_for_false(self): """Permissions are not set and `False` is returned in an already silenced channel.""" perm_overwrite = Mock(send_messages=False) channel = Mock(overwrites_for=Mock(return_value=perm_overwrite)) - self.assertFalse(asyncio.run(self.cog._silence(channel, True, None))) + self.assertFalse(await self.cog._silence(channel, True, None)) channel.set_permissions.assert_not_called() - def test_silence_private_silenced_channel(self): + async def test_silence_private_silenced_channel(self): """Channel had `send_message` permissions revoked and was added to `muted_channels`.""" channel = MockTextChannel() muted_channels = Mock() with mock.patch.object(self.cog, "muted_channels", new=muted_channels, create=True): - self.assertTrue(asyncio.run(self.cog._silence(channel, False, None))) + self.assertTrue(await self.cog._silence(channel, False, None)) channel.set_permissions.assert_called_once() self.assertFalse(channel.set_permissions.call_args.kwargs['overwrite'].send_messages) muted_channels.add.call_args.assert_called_once_with(channel) - def test_silence_private_notifier(self): + async def test_silence_private_notifier(self): """Channel should be added to notifier with `persistent` set to `True`, and the other way around.""" channel = MockTextChannel() with mock.patch.object(self.cog, "notifier", create=True): with self.subTest(persistent=True): - asyncio.run(self.cog._silence(channel, True, None)) + await self.cog._silence(channel, True, None) self.cog.notifier.add_channel.assert_called_once() with mock.patch.object(self.cog, "notifier", create=True): with self.subTest(persistent=False): - asyncio.run(self.cog._silence(channel, False, None)) + await self.cog._silence(channel, False, None) self.cog.notifier.add_channel.assert_not_called() -- cgit v1.2.3 From c575beccdbe5e4e715a4b11b378dd969a0327191 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 02:47:07 +0100 Subject: Add tests for `_unsilence` --- tests/bot/cogs/moderation/test_silence.py | 34 ++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 33ff78ca6..acfa3ffb8 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -1,6 +1,6 @@ import unittest from unittest import mock -from unittest.mock import Mock +from unittest.mock import MagicMock, Mock from bot.cogs.moderation.silence import FirstHash, Silence from bot.constants import Emojis @@ -89,3 +89,35 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): with self.subTest(persistent=False): await self.cog._silence(channel, False, None) self.cog.notifier.add_channel.assert_not_called() + + async def test_unsilence_private_for_false(self): + """Permissions are not set and `False` is returned in an unsilenced channel.""" + channel = Mock() + self.assertFalse(await self.cog._unsilence(channel)) + channel.set_permissions.assert_not_called() + + async def test_unsilence_private_unsilenced_channel(self): + """Channel had `send_message` permissions restored""" + perm_overwrite = MagicMock(send_messages=False) + channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) + with mock.patch.object(self.cog, "notifier", create=True): + self.assertTrue(await self.cog._unsilence(channel)) + channel.set_permissions.assert_called_once() + self.assertTrue(channel.set_permissions.call_args.kwargs['overwrite'].send_messages) + + async def test_unsilence_private_removed_notifier(self): + """Channel was removed from `notifier` on unsilence.""" + perm_overwrite = MagicMock(send_messages=False) + channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) + with mock.patch.object(self.cog, "notifier", create=True): + await self.cog._unsilence(channel) + self.cog.notifier.remove_channel.call_args.assert_called_once_with(channel) + + async def test_unsilence_private_removed_muted_channel(self): + """Channel was removed from `muted_channels` on unsilence.""" + perm_overwrite = MagicMock(send_messages=False) + channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) + with mock.patch.object(self.cog, "muted_channels", create=True),\ + mock.patch.object(self.cog, "notifier", create=True): # noqa E127 + await self.cog._unsilence(channel) + self.cog.muted_channels.remove.call_args.assert_called_once_with(channel) -- cgit v1.2.3 From a3f07589b215317d6a0fc16d982c3b645fe96151 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 02:54:35 +0100 Subject: Separate tests for permissions and `muted_channels.add` on `_silence`. --- tests/bot/cogs/moderation/test_silence.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index acfa3ffb8..3a513f3a7 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -68,14 +68,11 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel.set_permissions.assert_not_called() async def test_silence_private_silenced_channel(self): - """Channel had `send_message` permissions revoked and was added to `muted_channels`.""" + """Channel had `send_message` permissions revoked.""" channel = MockTextChannel() - muted_channels = Mock() - with mock.patch.object(self.cog, "muted_channels", new=muted_channels, create=True): - self.assertTrue(await self.cog._silence(channel, False, None)) + self.assertTrue(await self.cog._silence(channel, False, None)) channel.set_permissions.assert_called_once() self.assertFalse(channel.set_permissions.call_args.kwargs['overwrite'].send_messages) - muted_channels.add.call_args.assert_called_once_with(channel) async def test_silence_private_notifier(self): """Channel should be added to notifier with `persistent` set to `True`, and the other way around.""" @@ -90,6 +87,12 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._silence(channel, False, None) self.cog.notifier.add_channel.assert_not_called() + async def test_silence_private_removed_muted_channel(self): + channel = MockTextChannel() + with mock.patch.object(self.cog, "muted_channels") as muted_channels: + await self.cog._silence(MockTextChannel(), False, None) + muted_channels.add.call_args.assert_called_once_with(channel) + async def test_unsilence_private_for_false(self): """Permissions are not set and `False` is returned in an unsilenced channel.""" channel = Mock() -- cgit v1.2.3 From c72d31f717ac5e755fe3848c99ebf426fcdf6d8b Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 02:58:09 +0100 Subject: Use patch decorators and assign names from `with` patches. --- tests/bot/cogs/moderation/test_silence.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 3a513f3a7..027508661 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -99,28 +99,28 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.assertFalse(await self.cog._unsilence(channel)) channel.set_permissions.assert_not_called() - async def test_unsilence_private_unsilenced_channel(self): + @mock.patch.object(Silence, "notifier", create=True) + async def test_unsilence_private_unsilenced_channel(self, _): """Channel had `send_message` permissions restored""" perm_overwrite = MagicMock(send_messages=False) channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) - with mock.patch.object(self.cog, "notifier", create=True): - self.assertTrue(await self.cog._unsilence(channel)) + self.assertTrue(await self.cog._unsilence(channel)) channel.set_permissions.assert_called_once() self.assertTrue(channel.set_permissions.call_args.kwargs['overwrite'].send_messages) - async def test_unsilence_private_removed_notifier(self): + @mock.patch.object(Silence, "notifier", create=True) + async def test_unsilence_private_removed_notifier(self, notifier): """Channel was removed from `notifier` on unsilence.""" perm_overwrite = MagicMock(send_messages=False) channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) - with mock.patch.object(self.cog, "notifier", create=True): - await self.cog._unsilence(channel) - self.cog.notifier.remove_channel.call_args.assert_called_once_with(channel) + await self.cog._unsilence(channel) + notifier.remove_channel.call_args.assert_called_once_with(channel) - async def test_unsilence_private_removed_muted_channel(self): + @mock.patch.object(Silence, "notifier", create=True) + async def test_unsilence_private_removed_muted_channel(self, _): """Channel was removed from `muted_channels` on unsilence.""" perm_overwrite = MagicMock(send_messages=False) channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) - with mock.patch.object(self.cog, "muted_channels", create=True),\ - mock.patch.object(self.cog, "notifier", create=True): # noqa E127 + with mock.patch.object(self.cog, "muted_channels") as muted_channels: await self.cog._unsilence(channel) - self.cog.muted_channels.remove.call_args.assert_called_once_with(channel) + muted_channels.remove.call_args.assert_called_once_with(channel) -- cgit v1.2.3 From 44967038f39f4ecd1375fb9edff2b972becb5661 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 14:51:15 +0100 Subject: Add test for `cog_unload`. --- tests/bot/cogs/moderation/test_silence.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 027508661..fc2600f5c 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -3,7 +3,7 @@ from unittest import mock from unittest.mock import MagicMock, Mock from bot.cogs.moderation.silence import FirstHash, Silence -from bot.constants import Emojis +from bot.constants import Emojis, Roles from tests.helpers import MockBot, MockContext, MockTextChannel @@ -124,3 +124,19 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): with mock.patch.object(self.cog, "muted_channels") as muted_channels: await self.cog._unsilence(channel) muted_channels.remove.call_args.assert_called_once_with(channel) + + @mock.patch("bot.cogs.moderation.silence.asyncio") + @mock.patch.object(Silence, "_mod_alerts_channel", create=True) + def test_cog_unload(self, alert_channel, asyncio_mock): + """Task for sending an alert was created with present `muted_channels`.""" + with mock.patch.object(self.cog, "muted_channels"): + self.cog.cog_unload() + asyncio_mock.create_task.call_args.assert_called_once_with( + alert_channel.send(f"<@&{Roles.moderators}> channels left silenced on cog unload: ") + ) + + @mock.patch("bot.cogs.moderation.silence.asyncio") + def test_cog_unload1(self, asyncio_mock): + """No task created with no channels.""" + self.cog.cog_unload() + asyncio_mock.create_task.assert_not_called() -- cgit v1.2.3 From cb9397ba9ef311917629c8904087c1b3c38cc2d3 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 15:26:03 +0100 Subject: Add test for `cog_check`. --- tests/bot/cogs/moderation/test_silence.py | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index fc2600f5c..eaf897d1d 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -140,3 +140,10 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): """No task created with no channels.""" self.cog.cog_unload() asyncio_mock.create_task.assert_not_called() + + @mock.patch("bot.cogs.moderation.silence.with_role_check") + @mock.patch("bot.cogs.moderation.silence.MODERATION_ROLES", new=(1, 2, 3)) + def test_cog_check(self, role_check): + """Role check is called with `MODERATION_ROLES`""" + self.cog.cog_check(self.ctx) + role_check.assert_called_once_with(self.ctx, *(1, 2, 3)) -- cgit v1.2.3 From fbee48ee04dc6b44f97f229549c62cbfd5cef615 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 15:32:06 +0100 Subject: Fix erroneous `assert_called_once_with` calls. `assert_called_once_with` was being tested on call_args which always reported success.st. --- tests/bot/cogs/moderation/test_silence.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index eaf897d1d..4163a9af7 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -51,13 +51,13 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): ): with mock.patch.object(self.cog, "_silence", return_value=_silence_patch_return): await self.cog.silence.callback(self.cog, self.ctx, duration) - self.ctx.send.call_args.assert_called_once_with(result_message) + self.ctx.send.assert_called_once_with(result_message) async def test_unsilence_sent_correct_discord_message(self): """Check if proper message was sent to `alert_chanel`.""" with mock.patch.object(self.cog, "_unsilence", return_value=True): await self.cog.unsilence.callback(self.cog, self.ctx) - self.ctx.send.call_args.assert_called_once_with(f"{Emojis.check_mark} unsilenced current channel.") + self.ctx.send.assert_called_once_with(f"{Emojis.check_mark} unsilenced current channel.") async def test_silence_private_for_false(self): """Permissions are not set and `False` is returned in an already silenced channel.""" @@ -91,7 +91,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockTextChannel() with mock.patch.object(self.cog, "muted_channels") as muted_channels: await self.cog._silence(MockTextChannel(), False, None) - muted_channels.add.call_args.assert_called_once_with(channel) + muted_channels.add.assert_called_once_with(channel) async def test_unsilence_private_for_false(self): """Permissions are not set and `False` is returned in an unsilenced channel.""" @@ -114,7 +114,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): perm_overwrite = MagicMock(send_messages=False) channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) await self.cog._unsilence(channel) - notifier.remove_channel.call_args.assert_called_once_with(channel) + notifier.remove_channel.assert_called_once_with(channel) @mock.patch.object(Silence, "notifier", create=True) async def test_unsilence_private_removed_muted_channel(self, _): @@ -123,7 +123,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) with mock.patch.object(self.cog, "muted_channels") as muted_channels: await self.cog._unsilence(channel) - muted_channels.remove.call_args.assert_called_once_with(channel) + muted_channels.remove.assert_called_once_with(channel) @mock.patch("bot.cogs.moderation.silence.asyncio") @mock.patch.object(Silence, "_mod_alerts_channel", create=True) @@ -131,9 +131,8 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): """Task for sending an alert was created with present `muted_channels`.""" with mock.patch.object(self.cog, "muted_channels"): self.cog.cog_unload() - asyncio_mock.create_task.call_args.assert_called_once_with( - alert_channel.send(f"<@&{Roles.moderators}> channels left silenced on cog unload: ") - ) + asyncio_mock.create_task.assert_called_once_with(alert_channel.send()) + alert_channel.send.called_once_with(f"<@&{Roles.moderators}> chandnels left silenced on cog unload: ") @mock.patch("bot.cogs.moderation.silence.asyncio") def test_cog_unload1(self, asyncio_mock): -- cgit v1.2.3 From 64b27e557acf268a19246b2eb80ad6a743df95f4 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 15:32:24 +0100 Subject: Reset `self.ctx` call history after every subtest. --- tests/bot/cogs/moderation/test_silence.py | 1 + 1 file changed, 1 insertion(+) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 4163a9af7..ab2f091ec 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -52,6 +52,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): with mock.patch.object(self.cog, "_silence", return_value=_silence_patch_return): await self.cog.silence.callback(self.cog, self.ctx, duration) self.ctx.send.assert_called_once_with(result_message) + self.ctx.reset_mock() async def test_unsilence_sent_correct_discord_message(self): """Check if proper message was sent to `alert_chanel`.""" -- cgit v1.2.3 From 10428d9a456c7bce533cda53100e4c35930211d6 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 15:33:05 +0100 Subject: Pass created channel instead of new object. Creating a new object caused the assert to fail because different objects were used. --- tests/bot/cogs/moderation/test_silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index ab2f091ec..23f8a84ab 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -91,7 +91,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_silence_private_removed_muted_channel(self): channel = MockTextChannel() with mock.patch.object(self.cog, "muted_channels") as muted_channels: - await self.cog._silence(MockTextChannel(), False, None) + await self.cog._silence(channel, False, None) muted_channels.add.assert_called_once_with(channel) async def test_unsilence_private_for_false(self): -- cgit v1.2.3 From b2aa9af7f9f1485aa3ae8ed4d029fd2d72ea17ad Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 16:02:35 +0100 Subject: Add tests for `_get_instance_vars`. --- tests/bot/cogs/moderation/test_silence.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 23f8a84ab..c9aa7d84f 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -3,7 +3,7 @@ from unittest import mock from unittest.mock import MagicMock, Mock from bot.cogs.moderation.silence import FirstHash, Silence -from bot.constants import Emojis, Roles +from bot.constants import Channels, Emojis, Guild, Roles from tests.helpers import MockBot, MockContext, MockTextChannel @@ -36,6 +36,33 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.ctx = MockContext() self.cog._verified_role = None + async def test_instance_vars_got_guild(self): + """Bot got guild after it became available.""" + await self.cog._get_instance_vars() + self.bot.wait_until_guild_available.assert_called_once() + self.bot.get_guild.assert_called_once_with(Guild.id) + + async def test_instance_vars_got_role(self): + """Got `Roles.verified` role from guild.""" + await self.cog._get_instance_vars() + guild = self.bot.get_guild() + guild.get_role.assert_called_once_with(Roles.verified) + + async def test_instance_vars_got_channels(self): + """Got channels from bot.""" + await self.cog._get_instance_vars() + self.bot.get_channel.called_once_with(Channels.mod_alerts) + self.bot.get_channel.called_once_with(Channels.mod_log) + + @mock.patch("bot.cogs.moderation.silence.SilenceNotifier") + async def test_instance_vars_got_notifier(self, notifier): + """Notifier was started with channel.""" + mod_log = MockTextChannel() + self.bot.get_channel.side_effect = (None, mod_log) + await self.cog._get_instance_vars() + notifier.assert_called_once_with(mod_log) + self.bot.get_channel.side_effect = None + async def test_silence_sent_correct_discord_message(self): """Check if proper message was sent when called with duration in channel with previous state.""" test_cases = ( -- cgit v1.2.3 From 8ee70ffe645621a6b97172176afe1ac63261df31 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 16:10:38 +0100 Subject: Create test case for `SilenceNotifier` --- tests/bot/cogs/moderation/test_silence.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index c9aa7d84f..fc7734d45 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -2,7 +2,7 @@ import unittest from unittest import mock from unittest.mock import MagicMock, Mock -from bot.cogs.moderation.silence import FirstHash, Silence +from bot.cogs.moderation.silence import FirstHash, Silence, SilenceNotifier from bot.constants import Channels, Emojis, Guild, Roles from tests.helpers import MockBot, MockContext, MockTextChannel @@ -29,6 +29,12 @@ class FirstHashTests(unittest.TestCase): self.assertTrue(tuple1 == tuple2) +class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): + def setUp(self) -> None: + self.alert_channel = MockTextChannel() + self.notifier = SilenceNotifier(self.alert_channel) + + class SilenceTests(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: self.bot = MockBot() -- cgit v1.2.3 From fd75f10f3c8a588bd1763873baad08b8f90d58a3 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 17:09:40 +0100 Subject: Add tests for `add_channel`. --- tests/bot/cogs/moderation/test_silence.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index fc7734d45..be5b8e550 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -33,6 +33,27 @@ class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: self.alert_channel = MockTextChannel() self.notifier = SilenceNotifier(self.alert_channel) + self.notifier.stop = self.notifier_stop_mock = Mock() + self.notifier.start = self.notifier_start_mock = Mock() + self.notifier._current_loop = self.current_loop_mock = Mock() + + def test_add_channel_adds_channel(self): + """Channel in FirstHash with current loop is added to internal set.""" + channel = Mock() + with mock.patch.object(self.notifier, "_silenced_channels") as silenced_channels: + self.notifier.add_channel(channel) + silenced_channels.add.assert_called_with(FirstHash(channel, self.current_loop_mock)) + + def test_add_channel_starts_loop(self): + """Loop is started if `_silenced_channels` was empty.""" + self.notifier.add_channel(Mock()) + self.notifier_start_mock.assert_called_once() + + def test_add_channel_skips_start_with_channels(self): + """Loop start is not called when `_silenced_channels` is not empty.""" + with mock.patch.object(self.notifier, "_silenced_channels"): + self.notifier.add_channel(Mock()) + self.notifier_start_mock.assert_not_called() class SilenceTests(unittest.IsolatedAsyncioTestCase): -- cgit v1.2.3 From d9c904164a9e54750ce8ee36535bceacfc4800f5 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 18:06:53 +0100 Subject: Remove `_current_loop` from setup. --- tests/bot/cogs/moderation/test_silence.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index be5b8e550..2e04dc407 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -35,14 +35,13 @@ class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): self.notifier = SilenceNotifier(self.alert_channel) self.notifier.stop = self.notifier_stop_mock = Mock() self.notifier.start = self.notifier_start_mock = Mock() - self.notifier._current_loop = self.current_loop_mock = Mock() def test_add_channel_adds_channel(self): """Channel in FirstHash with current loop is added to internal set.""" channel = Mock() with mock.patch.object(self.notifier, "_silenced_channels") as silenced_channels: self.notifier.add_channel(channel) - silenced_channels.add.assert_called_with(FirstHash(channel, self.current_loop_mock)) + silenced_channels.add.assert_called_with(FirstHash(channel, self.notifier._current_loop)) def test_add_channel_starts_loop(self): """Loop is started if `_silenced_channels` was empty.""" -- cgit v1.2.3 From 4740c0fcdc6da6f164963fb34715e78c5d586cec Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 18:07:14 +0100 Subject: Add tests for `remove_channel`. --- tests/bot/cogs/moderation/test_silence.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 2e04dc407..c52ca2a2a 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -54,6 +54,24 @@ class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): self.notifier.add_channel(Mock()) self.notifier_start_mock.assert_not_called() + def test_remove_channel_removes_channel(self): + """Channel in FirstHash is removed from `_silenced_channels`.""" + channel = Mock() + with mock.patch.object(self.notifier, "_silenced_channels") as silenced_channels: + self.notifier.remove_channel(channel) + silenced_channels.remove.assert_called_with(FirstHash(channel)) + + def test_remove_channel_stops_loop(self): + """Notifier loop is stopped if `_silenced_channels` is empty after remove.""" + with mock.patch.object(self.notifier, "_silenced_channels", __bool__=lambda _: False): + self.notifier.remove_channel(Mock()) + self.notifier_stop_mock.assert_called_once() + + def test_remove_channel_skips_stop_with_channels(self): + """Notifier loop is not stopped if `_silenced_channels` is not empty after remove.""" + self.notifier.remove_channel(Mock()) + self.notifier_stop_mock.assert_not_called() + class SilenceTests(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: -- cgit v1.2.3 From 28cf22bcd98d94fa27e80dde4c86c9054b33c538 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 11 Mar 2020 21:55:09 +0100 Subject: Add tests for `_notifier`. --- tests/bot/cogs/moderation/test_silence.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index c52ca2a2a..d4719159e 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -72,6 +72,25 @@ class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): self.notifier.remove_channel(Mock()) self.notifier_stop_mock.assert_not_called() + async def test_notifier_private_sends_alert(self): + """Alert is sent on 15 min intervals.""" + test_cases = (900, 1800, 2700) + for current_loop in test_cases: + with self.subTest(current_loop=current_loop): + with mock.patch.object(self.notifier, "_current_loop", new=current_loop): + await self.notifier._notifier() + self.alert_channel.send.assert_called_once_with(f"<@&{Roles.moderators}> currently silenced channels: ") + self.alert_channel.send.reset_mock() + + async def test_notifier_skips_alert(self): + """Alert is skipped on first loop or not an increment of 900.""" + test_cases = (0, 15, 5000) + for current_loop in test_cases: + with self.subTest(current_loop=current_loop): + with mock.patch.object(self.notifier, "_current_loop", new=current_loop): + await self.notifier._notifier() + self.alert_channel.send.assert_not_called() + class SilenceTests(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: -- cgit v1.2.3 From d9ed24922f6daa17d625b345cb195e7fae7758cc Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 9 Mar 2020 13:31:44 -0700 Subject: Cog tests: add a function to get all extensions --- tests/bot/cogs/test_cogs.py | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py index b128ca123..386299fb1 100644 --- a/tests/bot/cogs/test_cogs.py +++ b/tests/bot/cogs/test_cogs.py @@ -1,10 +1,15 @@ """Test suite for general tests which apply to all cogs.""" +import importlib +import pkgutil import typing as t import unittest +from types import ModuleType from discord.ext import commands +from bot import cogs + class CommandNameTests(unittest.TestCase): """Tests for shadowing command names and aliases.""" @@ -17,3 +22,9 @@ class CommandNameTests(unittest.TestCase): yield command if isinstance(command, commands.GroupMixin): yield from command.walk_commands() + + @staticmethod + def walk_extensions() -> t.Iterator[ModuleType]: + """Yield imported extensions (modules) from the bot.cogs subpackage.""" + for module in pkgutil.iter_modules(cogs.__path__, "bot.cogs."): + yield importlib.import_module(module.name) -- cgit v1.2.3 From b923c0f844f65275d90e3807aa8e3eadf3920252 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 9 Mar 2020 13:37:56 -0700 Subject: Cog tests: add a function to get all cogs --- tests/bot/cogs/test_cogs.py | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py index 386299fb1..4290c279c 100644 --- a/tests/bot/cogs/test_cogs.py +++ b/tests/bot/cogs/test_cogs.py @@ -28,3 +28,10 @@ class CommandNameTests(unittest.TestCase): """Yield imported extensions (modules) from the bot.cogs subpackage.""" for module in pkgutil.iter_modules(cogs.__path__, "bot.cogs."): yield importlib.import_module(module.name) + + @staticmethod + def walk_cogs(extension: ModuleType) -> t.Iterator[commands.Cog]: + """Yield all cogs defined in an extension.""" + for name, cls in extension.__dict__.items(): + if isinstance(cls, commands.Cog): + yield getattr(extension, name) -- cgit v1.2.3 From 0358121687988159cb6754e249eed1ee2d40a783 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 9 Mar 2020 13:56:58 -0700 Subject: Cog tests: add a function to get all qualified names for a cmd --- tests/bot/cogs/test_cogs.py | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py index 4290c279c..e28717756 100644 --- a/tests/bot/cogs/test_cogs.py +++ b/tests/bot/cogs/test_cogs.py @@ -35,3 +35,11 @@ class CommandNameTests(unittest.TestCase): for name, cls in extension.__dict__.items(): if isinstance(cls, commands.Cog): yield getattr(extension, name) + + @staticmethod + def get_qualified_names(command: commands.Command) -> t.List[str]: + """Return a list of all qualified names, including aliases, for the `command`.""" + names = [f"{command.full_parent_name} {alias}" for alias in command.aliases] + names.append(command.qualified_name) + + return names -- cgit v1.2.3 From 5419b3e9599e8bb2f519949aa268eb3a4b3adbcc Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 9 Mar 2020 14:17:23 -0700 Subject: Cog tests: add a function to yield all commands This will help reduce nesting in the actual test. --- tests/bot/cogs/test_cogs.py | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py index e28717756..d260b46a7 100644 --- a/tests/bot/cogs/test_cogs.py +++ b/tests/bot/cogs/test_cogs.py @@ -43,3 +43,10 @@ class CommandNameTests(unittest.TestCase): names.append(command.qualified_name) return names + + def get_all_commands(self) -> t.Iterator[commands.Command]: + """Yield all commands for all cogs in all extensions.""" + for extension in self.walk_extensions(): + for cog in self.walk_cogs(extension): + for cmd in self.walk_commands(cog): + yield cmd -- cgit v1.2.3 From 1b4def2c8c0d82fc9738c1e969404e305c91cac9 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 9 Mar 2020 14:31:56 -0700 Subject: Cog tests: fix Cog type check in `walk_cogs` --- tests/bot/cogs/test_cogs.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py index d260b46a7..75aa1dbf6 100644 --- a/tests/bot/cogs/test_cogs.py +++ b/tests/bot/cogs/test_cogs.py @@ -32,9 +32,9 @@ class CommandNameTests(unittest.TestCase): @staticmethod def walk_cogs(extension: ModuleType) -> t.Iterator[commands.Cog]: """Yield all cogs defined in an extension.""" - for name, cls in extension.__dict__.items(): - if isinstance(cls, commands.Cog): - yield getattr(extension, name) + for obj in extension.__dict__.values(): + if isinstance(obj, type) and issubclass(obj, commands.Cog): + yield obj @staticmethod def get_qualified_names(command: commands.Command) -> t.List[str]: -- cgit v1.2.3 From 78327b9fa7c64a04d527fce582b93210356451fe Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 9 Mar 2020 14:49:08 -0700 Subject: Cog tests: fix duplicate cogs being yielded Have to check the modules are equal to prevent yielding imported cogs. --- tests/bot/cogs/test_cogs.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py index 75aa1dbf6..de0982c93 100644 --- a/tests/bot/cogs/test_cogs.py +++ b/tests/bot/cogs/test_cogs.py @@ -33,7 +33,8 @@ class CommandNameTests(unittest.TestCase): def walk_cogs(extension: ModuleType) -> t.Iterator[commands.Cog]: """Yield all cogs defined in an extension.""" for obj in extension.__dict__.values(): - if isinstance(obj, type) and issubclass(obj, commands.Cog): + is_cog = isinstance(obj, type) and issubclass(obj, commands.Cog) + if is_cog and obj.__module__ == extension.__name__: yield obj @staticmethod -- cgit v1.2.3 From bbcdf24a4b5d4f84834bbc8a8da7db2da627541f Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 9 Mar 2020 15:02:22 -0700 Subject: Cog tests: fix nested modules not being found * Rename `walk_extensions` to `walk_modules` because some extensions don't consist of a single module --- tests/bot/cogs/test_cogs.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py index de0982c93..3a9f07db6 100644 --- a/tests/bot/cogs/test_cogs.py +++ b/tests/bot/cogs/test_cogs.py @@ -24,17 +24,21 @@ class CommandNameTests(unittest.TestCase): yield from command.walk_commands() @staticmethod - def walk_extensions() -> t.Iterator[ModuleType]: - """Yield imported extensions (modules) from the bot.cogs subpackage.""" - for module in pkgutil.iter_modules(cogs.__path__, "bot.cogs."): - yield importlib.import_module(module.name) + 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) + + for module in pkgutil.walk_packages(cogs.__path__, "bot.cogs.", onerror=on_error): + if not module.ispkg: + yield importlib.import_module(module.name) @staticmethod - def walk_cogs(extension: ModuleType) -> t.Iterator[commands.Cog]: + def walk_cogs(module: ModuleType) -> t.Iterator[commands.Cog]: """Yield all cogs defined in an extension.""" - for obj in extension.__dict__.values(): + for obj in module.__dict__.values(): is_cog = isinstance(obj, type) and issubclass(obj, commands.Cog) - if is_cog and obj.__module__ == extension.__name__: + if is_cog and obj.__module__ == module.__name__: yield obj @staticmethod @@ -47,7 +51,7 @@ class CommandNameTests(unittest.TestCase): def get_all_commands(self) -> t.Iterator[commands.Command]: """Yield all commands for all cogs in all extensions.""" - for extension in self.walk_extensions(): - for cog in self.walk_cogs(extension): + for module in self.walk_modules(): + for cog in self.walk_cogs(module): for cmd in self.walk_commands(cog): yield cmd -- cgit v1.2.3 From 02fe32879be51b3f202501ea8cdc5314ca3b77b2 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 9 Mar 2020 15:29:19 -0700 Subject: Cog tests: fix duplicate commands being yielded discord.py yields duplicate Command objects for each alias a command has, so the duplicates need to be removed on our end. --- tests/bot/cogs/test_cogs.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py index 3a9f07db6..9d1d4ebea 100644 --- a/tests/bot/cogs/test_cogs.py +++ b/tests/bot/cogs/test_cogs.py @@ -21,7 +21,8 @@ class CommandNameTests(unittest.TestCase): if command.parent is None: yield command if isinstance(command, commands.GroupMixin): - yield from command.walk_commands() + # Annoyingly it returns duplicates for each alias so use a set to fix that + yield from set(command.walk_commands()) @staticmethod def walk_modules() -> t.Iterator[ModuleType]: -- cgit v1.2.3 From 7e7c538435c899f45ff277e05fb59d139f401954 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 9 Mar 2020 15:34:12 -0700 Subject: Cog tests: add a test for duplicate command names & aliases --- tests/bot/cogs/test_cogs.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py index 9d1d4ebea..616f5f44a 100644 --- a/tests/bot/cogs/test_cogs.py +++ b/tests/bot/cogs/test_cogs.py @@ -4,6 +4,7 @@ import importlib import pkgutil import typing as t import unittest +from collections import defaultdict from types import ModuleType from discord.ext import commands @@ -56,3 +57,19 @@ class CommandNameTests(unittest.TestCase): for cog in self.walk_cogs(module): for cmd in self.walk_commands(cog): yield cmd + + def test_names_dont_shadow(self): + """Names and aliases of commands should be unique.""" + all_names = defaultdict(list) + for cmd in self.get_all_commands(): + func_name = f"{cmd.module}.{cmd.callback.__qualname__}" + + for name in self.get_qualified_names(cmd): + with self.subTest(cmd=func_name, name=name): + if name in all_names: + conflicts = ", ".join(all_names.get(name, "")) + self.fail( + f"Name '{name}' of the command {func_name} conflicts with {conflicts}." + ) + + all_names[name].append(func_name) -- cgit v1.2.3 From f105ae75a98ae3e0295352d7debbc4fe04c73afd Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 13 Mar 2020 17:32:16 -0700 Subject: Cog tests: fix leading space in aliases without parents --- tests/bot/cogs/test_cogs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py index 616f5f44a..cbd203786 100644 --- a/tests/bot/cogs/test_cogs.py +++ b/tests/bot/cogs/test_cogs.py @@ -46,7 +46,7 @@ class CommandNameTests(unittest.TestCase): @staticmethod def get_qualified_names(command: commands.Command) -> t.List[str]: """Return a list of all qualified names, including aliases, for the `command`.""" - names = [f"{command.full_parent_name} {alias}" for alias in command.aliases] + names = [f"{command.full_parent_name} {alias}".strip() for alias in command.aliases] names.append(command.qualified_name) return names -- cgit v1.2.3 From 4d4975544ffec249aa6cd43d14987c00794caf99 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 13 Mar 2020 17:42:24 -0700 Subject: Cog tests: fix error on import due to discord.ext.tasks.loop The tasks extensions loop requires an event loop to exist. To work around this, it's been mocked. --- tests/bot/cogs/test_cogs.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py index cbd203786..db559ded6 100644 --- a/tests/bot/cogs/test_cogs.py +++ b/tests/bot/cogs/test_cogs.py @@ -6,6 +6,7 @@ import typing as t import unittest from collections import defaultdict from types import ModuleType +from unittest import mock from discord.ext import commands @@ -31,9 +32,10 @@ class CommandNameTests(unittest.TestCase): def on_error(name: str) -> t.NoReturn: raise ImportError(name=name) - for module in pkgutil.walk_packages(cogs.__path__, "bot.cogs.", onerror=on_error): - if not module.ispkg: - yield importlib.import_module(module.name) + with mock.patch("discord.ext.tasks.loop"): + for module in pkgutil.walk_packages(cogs.__path__, "bot.cogs.", onerror=on_error): + if not module.ispkg: + yield importlib.import_module(module.name) @staticmethod def walk_cogs(module: ModuleType) -> t.Iterator[commands.Cog]: -- cgit v1.2.3 From 252b385e46ef542203e69f4f6d147dadbcec8f0f Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 16:11:35 +0100 Subject: Use dict instead of a set and custom class. The FirstHash class is no longer necessary with only channels and the current loop in tuples. FirstHash was removed, along with its tests and tests were adjusted for new dict behaviour. --- bot/cogs/moderation/silence.py | 24 +++++------------------- tests/bot/cogs/moderation/test_silence.py | 28 +++------------------------- 2 files changed, 8 insertions(+), 44 deletions(-) (limited to 'tests') diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 8ed1cb28b..5df1fbbc0 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -15,26 +15,12 @@ from bot.utils.checks import with_role_check log = logging.getLogger(__name__) -class FirstHash(tuple): - """Tuple with only first item used for hash and eq.""" - - def __new__(cls, *args): - """Construct tuple from `args`.""" - return super().__new__(cls, args) - - def __hash__(self): - return hash((self[0],)) - - def __eq__(self, other: "FirstHash"): - return self[0] == other[0] - - class SilenceNotifier(tasks.Loop): """Loop notifier for posting notices to `alert_channel` containing added channels.""" def __init__(self, alert_channel: TextChannel): super().__init__(self._notifier, seconds=1, minutes=0, hours=0, count=None, reconnect=True, loop=None) - self._silenced_channels = set() + self._silenced_channels = {} self._alert_channel = alert_channel def add_channel(self, channel: TextChannel) -> None: @@ -42,12 +28,12 @@ class SilenceNotifier(tasks.Loop): if not self._silenced_channels: self.start() log.info("Starting notifier loop.") - self._silenced_channels.add(FirstHash(channel, self._current_loop)) + self._silenced_channels[channel] = self._current_loop def remove_channel(self, channel: TextChannel) -> None: """Remove channel from `_silenced_channels` and stop loop if no channels remain.""" with suppress(KeyError): - self._silenced_channels.remove(FirstHash(channel)) + del self._silenced_channels[channel] if not self._silenced_channels: self.stop() log.info("Stopping notifier loop.") @@ -58,11 +44,11 @@ class SilenceNotifier(tasks.Loop): if self._current_loop and not self._current_loop/60 % 15: log.debug( f"Sending notice with channels: " - f"{', '.join(f'#{channel} ({channel.id})' for channel, _ in self._silenced_channels)}." + f"{', '.join(f'#{channel} ({channel.id})' for channel in self._silenced_channels)}." ) channels_text = ', '.join( f"{channel.mention} for {(self._current_loop-start)//60} min" - for channel, start in self._silenced_channels + for channel, start in self._silenced_channels.items() ) await self._alert_channel.send(f"<@&{Roles.moderators}> currently silenced channels: {channels_text}") diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index d4719159e..6114fee21 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -2,33 +2,11 @@ import unittest from unittest import mock from unittest.mock import MagicMock, Mock -from bot.cogs.moderation.silence import FirstHash, Silence, SilenceNotifier +from bot.cogs.moderation.silence import Silence, SilenceNotifier from bot.constants import Channels, Emojis, Guild, Roles from tests.helpers import MockBot, MockContext, MockTextChannel -class FirstHashTests(unittest.TestCase): - def setUp(self) -> None: - self.test_cases = ( - (FirstHash(0, 4), FirstHash(0, 5)), - (FirstHash("string", None), FirstHash("string", True)) - ) - - def test_hashes_equal(self): - """Check hashes equal with same first item.""" - - for tuple1, tuple2 in self.test_cases: - with self.subTest(tuple1=tuple1, tuple2=tuple2): - self.assertEqual(hash(tuple1), hash(tuple2)) - - def test_eq(self): - """Check objects are equal with same first item.""" - - for tuple1, tuple2 in self.test_cases: - with self.subTest(tuple1=tuple1, tuple2=tuple2): - self.assertTrue(tuple1 == tuple2) - - class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: self.alert_channel = MockTextChannel() @@ -41,7 +19,7 @@ class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): channel = Mock() with mock.patch.object(self.notifier, "_silenced_channels") as silenced_channels: self.notifier.add_channel(channel) - silenced_channels.add.assert_called_with(FirstHash(channel, self.notifier._current_loop)) + silenced_channels.__setitem__.assert_called_with(channel, self.notifier._current_loop) def test_add_channel_starts_loop(self): """Loop is started if `_silenced_channels` was empty.""" @@ -59,7 +37,7 @@ class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): channel = Mock() with mock.patch.object(self.notifier, "_silenced_channels") as silenced_channels: self.notifier.remove_channel(channel) - silenced_channels.remove.assert_called_with(FirstHash(channel)) + silenced_channels.__delitem__.assert_called_with(channel) def test_remove_channel_stops_loop(self): """Notifier loop is stopped if `_silenced_channels` is empty after remove.""" -- cgit v1.2.3 From 36c57c6f89a070fbb77a641182e37c788b6de7a0 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 18:21:49 +0100 Subject: Adjust tests for new calling behaviour. `.set_permissions` calls were changed to use kwargs directly instead of an overwrite, this reflects the changes in tests. --- tests/bot/cogs/moderation/test_silence.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 6114fee21..b09426fde 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -141,7 +141,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockTextChannel() self.assertTrue(await self.cog._silence(channel, False, None)) channel.set_permissions.assert_called_once() - self.assertFalse(channel.set_permissions.call_args.kwargs['overwrite'].send_messages) + self.assertFalse(channel.set_permissions.call_args.kwargs['send_messages']) async def test_silence_private_notifier(self): """Channel should be added to notifier with `persistent` set to `True`, and the other way around.""" @@ -175,7 +175,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) self.assertTrue(await self.cog._unsilence(channel)) channel.set_permissions.assert_called_once() - self.assertTrue(channel.set_permissions.call_args.kwargs['overwrite'].send_messages) + self.assertTrue(channel.set_permissions.call_args.kwargs['send_messages']) @mock.patch.object(Silence, "notifier", create=True) async def test_unsilence_private_removed_notifier(self, notifier): -- cgit v1.2.3 From 0a2774fadddd18a86822a47599ebc4b76f1e5a7e Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Sun, 15 Mar 2020 18:23:11 +0100 Subject: Set `_get_instance_vars_event` in test's `setUp`. --- tests/bot/cogs/moderation/test_silence.py | 2 ++ 1 file changed, 2 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index b09426fde..c6f1fc1da 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -76,6 +76,8 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.cog = Silence(self.bot) self.ctx = MockContext() self.cog._verified_role = None + # Set event so command callbacks can continue. + self.cog._get_instance_vars_event.set() async def test_instance_vars_got_guild(self): """Bot got guild after it became available.""" -- cgit v1.2.3 From b8559cc12fa75dd4b4a52697cf5aa313d3c397d0 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Mon, 16 Mar 2020 10:27:21 -0700 Subject: Cog tests: comment some code for clarification --- tests/bot/cogs/test_cogs.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/test_cogs.py b/tests/bot/cogs/test_cogs.py index db559ded6..39f6492cb 100644 --- a/tests/bot/cogs/test_cogs.py +++ b/tests/bot/cogs/test_cogs.py @@ -19,6 +19,7 @@ class CommandNameTests(unittest.TestCase): @staticmethod def walk_commands(cog: commands.Cog) -> t.Iterator[commands.Command]: """An iterator that recursively walks through `cog`'s commands and subcommands.""" + # Can't use Bot.walk_commands() or Cog.get_commands() cause those are instance methods. for command in cog.__cog_commands__: if command.parent is None: yield command @@ -32,6 +33,7 @@ class CommandNameTests(unittest.TestCase): def on_error(name: str) -> t.NoReturn: raise ImportError(name=name) + # The mock prevents asyncio.get_event_loop() from being called. with mock.patch("discord.ext.tasks.loop"): for module in pkgutil.walk_packages(cogs.__path__, "bot.cogs.", onerror=on_error): if not module.ispkg: @@ -41,6 +43,7 @@ class CommandNameTests(unittest.TestCase): def walk_cogs(module: ModuleType) -> t.Iterator[commands.Cog]: """Yield all cogs defined in an extension.""" for obj in module.__dict__.values(): + # Check if it's a class type cause otherwise issubclass() may raise a TypeError. is_cog = isinstance(obj, type) and issubclass(obj, commands.Cog) if is_cog and obj.__module__ == module.__name__: yield obj -- cgit v1.2.3 From 039a04462be58e9d345e32efcae13c8c999776db Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 13:23:44 +0100 Subject: Fix `test_cog_unload` passing tests with invalid values. The first assert - `asyncio_mock.create_task.assert_called_once_with` called `alert_channel`'s send resulting in an extra call. `send` on `alert_channel` was not tested properly because of a typo and a missing assert in the method call. --- tests/bot/cogs/moderation/test_silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index c6f1fc1da..febfd584b 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -202,8 +202,8 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): """Task for sending an alert was created with present `muted_channels`.""" with mock.patch.object(self.cog, "muted_channels"): self.cog.cog_unload() + alert_channel.send.assert_called_once_with(f"<@&{Roles.moderators}> channels left silenced on cog unload: ") asyncio_mock.create_task.assert_called_once_with(alert_channel.send()) - alert_channel.send.called_once_with(f"<@&{Roles.moderators}> chandnels left silenced on cog unload: ") @mock.patch("bot.cogs.moderation.silence.asyncio") def test_cog_unload1(self, asyncio_mock): -- cgit v1.2.3 From 2803c13c477634ceefe3501ad9cb7c76cfecf450 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 17:10:16 +0100 Subject: Rename `cog_unload` tests. Previous names were undescriptive from testing phases. --- tests/bot/cogs/moderation/test_silence.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index febfd584b..07a70e7dc 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -198,7 +198,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): @mock.patch("bot.cogs.moderation.silence.asyncio") @mock.patch.object(Silence, "_mod_alerts_channel", create=True) - def test_cog_unload(self, alert_channel, asyncio_mock): + def test_cog_unload_starts_task(self, alert_channel, asyncio_mock): """Task for sending an alert was created with present `muted_channels`.""" with mock.patch.object(self.cog, "muted_channels"): self.cog.cog_unload() @@ -206,7 +206,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): asyncio_mock.create_task.assert_called_once_with(alert_channel.send()) @mock.patch("bot.cogs.moderation.silence.asyncio") - def test_cog_unload1(self, asyncio_mock): + def test_cog_unload_skips_task_start(self, asyncio_mock): """No task created with no channels.""" self.cog.cog_unload() asyncio_mock.create_task.assert_not_called() -- cgit v1.2.3 From 20c41f2c5af6fd716c3e7f15de412f7f16f5ff1e Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 17:15:10 +0100 Subject: Remove one indentation level. Co-authored-by: MarkKoz --- tests/bot/cogs/moderation/test_silence.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 07a70e7dc..8b9e30cfe 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -115,9 +115,9 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): ) for duration, result_message, _silence_patch_return in test_cases: with self.subTest( - silence_duration=duration, - result_message=result_message, - starting_unsilenced_state=_silence_patch_return + silence_duration=duration, + result_message=result_message, + starting_unsilenced_state=_silence_patch_return ): with mock.patch.object(self.cog, "_silence", return_value=_silence_patch_return): await self.cog.silence.callback(self.cog, self.ctx, duration) -- cgit v1.2.3 From d456e40ac97a38ee99561546bcafb6aa94117cb7 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 17:16:31 +0100 Subject: Remove `alert_channel` mention from docstring. After removing the optional channel arg and changing output message channels we're only testing `ctx`'s `send`. --- tests/bot/cogs/moderation/test_silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 8b9e30cfe..b4a34bbc7 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -125,7 +125,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.ctx.reset_mock() async def test_unsilence_sent_correct_discord_message(self): - """Check if proper message was sent to `alert_chanel`.""" + """Proper reply after a successful unsilence.""" with mock.patch.object(self.cog, "_unsilence", return_value=True): await self.cog.unsilence.callback(self.cog, self.ctx) self.ctx.send.assert_called_once_with(f"{Emojis.check_mark} unsilenced current channel.") -- cgit v1.2.3 From 386c93a6f18adbf84691f17b13f5113800a353ae Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 19:40:27 +0100 Subject: Fix test name. `removed` was describing the opposite behaviour. --- tests/bot/cogs/moderation/test_silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index b4a34bbc7..55193e2f8 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -158,7 +158,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._silence(channel, False, None) self.cog.notifier.add_channel.assert_not_called() - async def test_silence_private_removed_muted_channel(self): + async def test_silence_private_added_muted_channel(self): channel = MockTextChannel() with mock.patch.object(self.cog, "muted_channels") as muted_channels: await self.cog._silence(channel, False, None) -- cgit v1.2.3 From dced6fdf5f571b82bc975dd3159af57c6f9a12b3 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 19:41:13 +0100 Subject: Add docstring to test. --- tests/bot/cogs/moderation/test_silence.py | 1 + 1 file changed, 1 insertion(+) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 55193e2f8..71541086d 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -159,6 +159,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.cog.notifier.add_channel.assert_not_called() async def test_silence_private_added_muted_channel(self): + """Channel was added to `muted_channels` on silence.""" channel = MockTextChannel() with mock.patch.object(self.cog, "muted_channels") as muted_channels: await self.cog._silence(channel, False, None) -- cgit v1.2.3 From c68b943708eaca110ddfa6121872513a422bbef4 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 20:10:50 +0100 Subject: Use set `discard` instead of `remove`. Discard ignores non present values, allowing us to skip the KeyError suppress. --- bot/cogs/moderation/silence.py | 3 +-- tests/bot/cogs/moderation/test_silence.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 1523baf11..a1446089e 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -141,8 +141,7 @@ class Silence(commands.Cog): await channel.set_permissions(self._verified_role, **dict(current_overwrite, send_messages=True)) log.info(f"Unsilenced channel #{channel} ({channel.id}).") self.notifier.remove_channel(channel) - with suppress(KeyError): - self.muted_channels.remove(channel) + self.muted_channels.discard(channel) return True log.info(f"Tried to unsilence channel #{channel} ({channel.id}) but the channel was not silenced.") return False diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 71541086d..eee020455 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -195,7 +195,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) with mock.patch.object(self.cog, "muted_channels") as muted_channels: await self.cog._unsilence(channel) - muted_channels.remove.assert_called_once_with(channel) + muted_channels.discard.assert_called_once_with(channel) @mock.patch("bot.cogs.moderation.silence.asyncio") @mock.patch.object(Silence, "_mod_alerts_channel", create=True) -- cgit v1.2.3 From cd429230fcb18c7101afd931317d37ad142bfe4b Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 20:11:44 +0100 Subject: Add tests ensuring permissions get preserved. --- tests/bot/cogs/moderation/test_silence.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'tests') diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index eee020455..44682a1bd 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -2,6 +2,8 @@ import unittest from unittest import mock from unittest.mock import MagicMock, Mock +from discord import PermissionOverwrite + from bot.cogs.moderation.silence import Silence, SilenceNotifier from bot.constants import Channels, Emojis, Guild, Roles from tests.helpers import MockBot, MockContext, MockTextChannel @@ -145,6 +147,20 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel.set_permissions.assert_called_once() self.assertFalse(channel.set_permissions.call_args.kwargs['send_messages']) + async def test_silence_private_preserves_permissions(self): + """Previous permissions were preserved when channel was silenced.""" + channel = MockTextChannel() + # Set up mock channel permission state. + mock_permissions = PermissionOverwrite() + mock_permissions_dict = dict(mock_permissions) + channel.overwrites_for.return_value = mock_permissions + await self.cog._silence(channel, False, None) + new_permissions = channel.set_permissions.call_args.kwargs + # Remove 'send_messages' key because it got changed in the method. + del new_permissions['send_messages'] + del mock_permissions_dict['send_messages'] + self.assertDictEqual(mock_permissions_dict, new_permissions) + async def test_silence_private_notifier(self): """Channel should be added to notifier with `persistent` set to `True`, and the other way around.""" channel = MockTextChannel() @@ -197,6 +213,21 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._unsilence(channel) muted_channels.discard.assert_called_once_with(channel) + @mock.patch.object(Silence, "notifier", create=True) + async def test_unsilence_private_preserves_permissions(self, _): + """Previous permissions were preserved when channel was unsilenced.""" + channel = MockTextChannel() + # Set up mock channel permission state. + mock_permissions = PermissionOverwrite(send_messages=False) + mock_permissions_dict = dict(mock_permissions) + channel.overwrites_for.return_value = mock_permissions + await self.cog._unsilence(channel) + new_permissions = channel.set_permissions.call_args.kwargs + # Remove 'send_messages' key because it got changed in the method. + del new_permissions['send_messages'] + del mock_permissions_dict['send_messages'] + self.assertDictEqual(mock_permissions_dict, new_permissions) + @mock.patch("bot.cogs.moderation.silence.asyncio") @mock.patch.object(Silence, "_mod_alerts_channel", create=True) def test_cog_unload_starts_task(self, alert_channel, asyncio_mock): -- cgit v1.2.3 From cefcc575b6faa94fb18f1985f039125d023b2580 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 17 Mar 2020 22:18:58 +0100 Subject: Add tests for `HushDurationConverter`. --- tests/bot/test_converters.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'tests') diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py index 1e5ca62ae..ca8cb6825 100644 --- a/tests/bot/test_converters.py +++ b/tests/bot/test_converters.py @@ -8,6 +8,7 @@ from discord.ext.commands import BadArgument from bot.converters import ( Duration, + HushDurationConverter, ISODateTime, TagContentConverter, TagNameConverter, @@ -271,3 +272,32 @@ class ConverterTests(unittest.TestCase): exception_message = f"`{datetime_string}` is not a valid ISO-8601 datetime string" with self.assertRaises(BadArgument, msg=exception_message): asyncio.run(converter.convert(self.context, datetime_string)) + + def test_hush_duration_converter_for_valid(self): + """HushDurationConverter returns correct value for minutes duration or `"forever"` strings.""" + test_values = ( + ("0", 0), + ("15", 15), + ("10", 10), + ("5m", 5), + ("5M", 5), + ("forever", None), + ) + converter = HushDurationConverter() + for minutes_string, expected_minutes in test_values: + with self.subTest(minutes_string=minutes_string, expected_minutes=expected_minutes): + converted = asyncio.run(converter.convert(self.context, minutes_string)) + self.assertEqual(expected_minutes, converted) + + def test_hush_duration_converter_for_invalid(self): + """HushDurationConverter raises correct exception for invalid minutes duration strings.""" + test_values = ( + ("16", "Duration must be at most 15 minutes."), + ("10d", "10d is not a valid minutes duration."), + ("-1", "-1 is not a valid minutes duration."), + ) + converter = HushDurationConverter() + for invalid_minutes_string, exception_message in test_values: + with self.subTest(invalid_minutes_string=invalid_minutes_string, exception_message=exception_message): + with self.assertRaisesRegex(BadArgument, exception_message): + asyncio.run(converter.convert(self.context, invalid_minutes_string)) -- cgit v1.2.3 From 430c616ec4ec60a5ddb1e66d3aacc622c9a78ae6 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Wed, 25 Mar 2020 12:21:57 -0700 Subject: Snekbox tests: test `get_code` Should return 1st arg (or None) if eval cmd in message, otherwise return full content. --- tests/bot/cogs/test_snekbox.py | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index fd9468829..1fad6904b 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -3,9 +3,11 @@ import logging import unittest from unittest.mock import AsyncMock, MagicMock, Mock, call, patch +from discord.ext import commands + +from bot import constants from bot.cogs import snekbox from bot.cogs.snekbox import Snekbox -from bot.constants import URLs from tests.helpers import MockBot, MockContext, MockMessage, MockReaction, MockUser @@ -23,7 +25,7 @@ class SnekboxTests(unittest.IsolatedAsyncioTestCase): self.assertEqual(await self.cog.post_eval("import random"), "return") self.bot.http_session.post.assert_called_with( - URLs.snekbox_eval_api, + constants.URLs.snekbox_eval_api, json={"input": "import random"}, raise_for_status=True ) @@ -43,10 +45,10 @@ class SnekboxTests(unittest.IsolatedAsyncioTestCase): self.assertEqual( await self.cog.upload_output("My awesome output"), - URLs.paste_service.format(key=key) + constants.URLs.paste_service.format(key=key) ) self.bot.http_session.post.assert_called_with( - URLs.paste_service.format(key="documents"), + constants.URLs.paste_service.format(key="documents"), data="My awesome output", raise_for_status=True ) @@ -302,6 +304,32 @@ class SnekboxTests(unittest.IsolatedAsyncioTestCase): self.assertEqual(actual, None) ctx.message.clear_reactions.assert_called_once() + async def test_get_code(self): + """Should return 1st arg (or None) if eval cmd in message, otherwise return full content.""" + prefix = constants.Bot.prefix + subtests = ( + (self.cog.eval_command, f"{prefix}{self.cog.eval_command.name} print(1)", "print(1)"), + (self.cog.eval_command, f"{prefix}{self.cog.eval_command.name}", None), + (MagicMock(spec=commands.Command), f"{prefix}tags get foo"), + (None, "print(123)") + ) + + for command, content, *expected_code in subtests: + if not expected_code: + expected_code = content + else: + [expected_code] = expected_code + + with self.subTest(content=content, expected_code=expected_code): + self.bot.get_context.reset_mock() + self.bot.get_context.return_value = MockContext(command=command) + message = MockMessage(content=content) + + actual_code = await self.cog.get_code(message) + + self.bot.get_context.assert_awaited_once_with(message) + self.assertEqual(actual_code, expected_code) + def test_predicate_eval_message_edit(self): """Test the predicate_eval_message_edit function.""" msg0 = MockMessage(id=1, content='abc') -- cgit v1.2.3 From c3e9a290a93c978a4dfec3ab121a0e45147855c8 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Wed, 25 Mar 2020 14:08:34 -0700 Subject: Snekbox tests: use `get_code` in `test_continue_eval_does_continue` --- tests/bot/cogs/test_snekbox.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index 1fad6904b..1dec0ccaf 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -1,7 +1,7 @@ import asyncio import logging import unittest -from unittest.mock import AsyncMock, MagicMock, Mock, call, patch +from unittest.mock import AsyncMock, MagicMock, Mock, call, create_autospec, patch from discord.ext import commands @@ -281,11 +281,14 @@ class SnekboxTests(unittest.IsolatedAsyncioTestCase): """Test that the continue_eval function does continue if required conditions are met.""" ctx = MockContext(message=MockMessage(add_reaction=AsyncMock(), clear_reactions=AsyncMock())) response = MockMessage(delete=AsyncMock()) - new_msg = MockMessage(content='!e NewCode') + new_msg = MockMessage() self.bot.wait_for.side_effect = ((None, new_msg), None) + expected = "NewCode" + self.cog.get_code = create_autospec(self.cog.get_code, spec_set=True, return_value=expected) actual = await self.cog.continue_eval(ctx, response) - self.assertEqual(actual, 'NewCode') + self.cog.get_code.assert_awaited_once_with(new_msg) + 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), -- cgit v1.2.3 From 582ddbb1ca8bab2cb883781911f5f35962330995 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Mon, 30 Mar 2020 13:36:34 +0200 Subject: Set unsilence permissions to inherit instead of true The "unsilence" action of the silence/hush command used `send_messages=True` when unsilencing a hushed channel. This had the side effect of also enabling send messages permissions for those with the Muted rule, as an explicit True permission apparently overwrites an explicit False permission, even if the latter was set for a higher top-role. The solution is to revert back to the `Inherit` permission by assigning `None`. This is what we normally use when Developers are allowed to send messages to a channel. --- bot/cogs/moderation/silence.py | 2 +- tests/bot/cogs/moderation/test_silence.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index a1446089e..1ef3967a9 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -138,7 +138,7 @@ class Silence(commands.Cog): """ current_overwrite = channel.overwrites_for(self._verified_role) if current_overwrite.send_messages is False: - await channel.set_permissions(self._verified_role, **dict(current_overwrite, send_messages=True)) + await channel.set_permissions(self._verified_role, **dict(current_overwrite, send_messages=None)) log.info(f"Unsilenced channel #{channel} ({channel.id}).") self.notifier.remove_channel(channel) self.muted_channels.discard(channel) diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 44682a1bd..3fd149f04 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -194,7 +194,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockTextChannel(overwrites_for=Mock(return_value=perm_overwrite)) self.assertTrue(await self.cog._unsilence(channel)) channel.set_permissions.assert_called_once() - self.assertTrue(channel.set_permissions.call_args.kwargs['send_messages']) + self.assertIsNone(channel.set_permissions.call_args.kwargs['send_messages']) @mock.patch.object(Silence, "notifier", create=True) async def test_unsilence_private_removed_notifier(self, notifier): -- cgit v1.2.3 From 7434ed3152e6d3f89babe2fef332983925d04434 Mon Sep 17 00:00:00 2001 From: ks129 <45097959+ks129@users.noreply.github.com> Date: Thu, 9 Apr 2020 14:45:32 +0300 Subject: (Syncer Tests): Replaced wrong side effect Replaced `TimeoutError` with `asyncio.TimeoutError`. --- tests/bot/cogs/sync/test_base.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'tests') 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: -- cgit v1.2.3 From 1140e9690644e46196a1c8cad900272ffb3ae09a Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff 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. --- bot/cogs/error_handler.py | 6 +-- bot/cogs/information.py | 10 +++-- bot/cogs/snekbox.py | 11 ++++- bot/cogs/utils.py | 8 +++- bot/cogs/verification.py | 18 +++++--- bot/decorators.py | 84 +++++++++++++++++++++++++------------- tests/bot/cogs/test_information.py | 4 +- 7 files changed, 94 insertions(+), 47 deletions(-) (limited to 'tests') diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index dae283c6a..3f56a9798 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 InChannelCheckFailure +from bot.decorators import InWhitelistedContextCheckFailure log = logging.getLogger(__name__) @@ -202,7 +202,7 @@ class ErrorHandler(Cog): * BotMissingRole * BotMissingAnyRole * NoPrivateMessage - * InChannelCheckFailure + * InWhitelistedContextCheckFailure """ 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, (InChannelCheckFailure, errors.NoPrivateMessage)): + elif isinstance(e, (InWhitelistedContextCheckFailure, 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 7921a4932..6b3fc0c96 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 InChannelCheckFailure, in_channel, with_role +from bot.decorators import InWhitelistedContextCheckFailure, in_whitelisted_context, 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 InChannelCheckFailure(constants.Channels.bot_commands) + raise InWhitelistedContextCheckFailure(constants.Channels.bot_commands) embed = await self.create_user_embed(ctx, user) @@ -331,7 +331,11 @@ class Information(Cog): @cooldown_with_role_bypass(2, 60 * 3, BucketType.member, bypass_roles=constants.STAFF_ROLES) @group(invoke_without_command=True) - @in_channel(constants.Channels.bot_commands, bypass_roles=constants.STAFF_ROLES) + @in_whitelisted_context( + whitelisted_channels=(constants.Channels.bot_commands,), + whitelisted_roles=constants.STAFF_ROLES, + redirect_channel=constants.Channels.bot_commands, + ) 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 315383b12..8827cb585 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 Channels, Roles, URLs -from bot.decorators import in_channel +from bot.decorators import in_whitelisted_context from bot.utils.messages import wait_for_deletion log = logging.getLogger(__name__) @@ -38,6 +38,9 @@ RAW_CODE_REGEX = re.compile( ) MAX_PASTE_LEN = 1000 + +# `!eval` command whitelists +EVAL_CHANNELS = (Channels.bot_commands, Channels.esoteric) EVAL_ROLES = (Roles.helpers, Roles.moderators, Roles.admins, Roles.owners, Roles.python_community, Roles.partners) SIGKILL = 9 @@ -265,7 +268,11 @@ class Snekbox(Cog): @command(name="eval", aliases=("e",)) @guild_only() - @in_channel(Channels.bot_commands, hidden_channels=(Channels.esoteric,), bypass_roles=EVAL_ROLES) + @in_whitelisted_context( + whitelisted_channels=EVAL_CHANNELS, + whitelisted_roles=EVAL_ROLES, + redirect_channel=Channels.bot_commands, + ) 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 3ed471bbf..234ec514d 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_channel, with_role +from bot.decorators import in_whitelisted_context, with_role from bot.utils.time import humanize_delta log = logging.getLogger(__name__) @@ -118,7 +118,11 @@ class Utils(Cog): await ctx.message.channel.send(embed=pep_embed) @command() - @in_channel(Channels.bot_commands, bypass_roles=STAFF_ROLES) + @in_whitelisted_context( + whitelisted_channels=(Channels.bot_commands,), + whitelisted_roles=STAFF_ROLES, + redirect_channel=Channels.bot_commands, + ) 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 b0a493e68..040f52fbf 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 InChannelCheckFailure, in_channel, without_role +from bot.decorators import InWhitelistedContextCheckFailure, in_whitelisted_context, 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_channel(constants.Channels.verification) + @in_whitelisted_context(whitelisted_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,7 +138,10 @@ class Verification(Cog): await ctx.message.delete() @command(name='subscribe') - @in_channel(constants.Channels.bot_commands) + @in_whitelisted_context( + whitelisted_channels=(constants.Channels.bot_commands,), + redirect_channel=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 @@ -162,7 +165,10 @@ class Verification(Cog): ) @command(name='unsubscribe') - @in_channel(constants.Channels.bot_commands) + @in_whitelisted_context( + whitelisted_channels=(constants.Channels.bot_commands,), + redirect_channel=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 @@ -187,8 +193,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 InChannelCheckFailure.""" - if isinstance(error, InChannelCheckFailure): + """Check for & ignore any InWhitelistedContextCheckFailure.""" + if isinstance(error, InWhitelistedContextCheckFailure): error.handled = True @staticmethod diff --git a/bot/decorators.py b/bot/decorators.py index 2d18eaa6a..149564d18 100644 --- a/bot/decorators.py +++ b/bot/decorators.py @@ -3,7 +3,7 @@ import random from asyncio import Lock, sleep from contextlib import suppress from functools import wraps -from typing import Callable, Container, Union +from typing import Callable, Container, Optional, Union from weakref import WeakValueDictionary from discord import Colour, Embed, Member @@ -17,48 +17,74 @@ from bot.utils.checks import with_role_check, without_role_check log = logging.getLogger(__name__) -class InChannelCheckFailure(CheckFailure): - """Raised when a check fails for a message being sent in a whitelisted channel.""" +class InWhitelistedContextCheckFailure(CheckFailure): + """Raised when the `in_whitelist` check fails.""" - def __init__(self, *channels: int): - self.channels = channels - channels_str = ', '.join(f"<#{c_id}>" for c_id in channels) + def __init__(self, redirect_channel: Optional[int] = None): + error_message = "Sorry, but you are not allowed to use that command here." - super().__init__(f"Sorry, but you may only use this command within {channels_str}.") + if redirect_channel: + error_message += f" Please use the <#{redirect_channel}> channel instead." + super().__init__(error_message) + + +def in_whitelisted_context( + *, + whitelisted_channels: Container[int] = (), + whitelisted_categories: Container[int] = (), + whitelisted_roles: Container[int] = (), + redirect_channel: Optional[int] = None, -def in_channel( - *channels: int, - hidden_channels: Container[int] = None, - bypass_roles: Container[int] = None ) -> Callable: """ - Checks that the message is in a whitelisted channel or optionally has a bypass role. + Check if a command was issued in a whitelisted context. + + The whitelists that can be provided are: - Hidden channels are channels which will not be displayed in the InChannelCheckFailure error - message. + - `channels`: a container with channel ids for whitelisted channels + - `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. """ - hidden_channels = hidden_channels or [] - bypass_roles = bypass_roles or [] + if redirect_channel and redirect_channel not in whitelisted_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` + # 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,) def predicate(ctx: Context) -> bool: - """In-channel checker predicate.""" - if ctx.channel.id in channels or ctx.channel.id in hidden_channels: - log.debug(f"{ctx.author} tried to call the '{ctx.command.name}' command. " - f"The command was used in a whitelisted channel.") + """Check if a command was issued in a whitelisted context.""" + if whitelisted_channels and ctx.channel.id in whitelisted_channels: + log.trace(f"{ctx.author} may use the `{ctx.command.name}` command as they are in a whitelisted channel.") return True - if bypass_roles: - if any(r.id in bypass_roles for r in ctx.author.roles): - log.debug(f"{ctx.author} tried to call the '{ctx.command.name}' command. " - f"The command was not used in a whitelisted channel, " - f"but the author had a role to bypass the in_channel check.") - 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 + ): + log.trace(f"{ctx.author} may use the `{ctx.command.name}` command as they are in a whitelisted category.") + return True - log.debug(f"{ctx.author} tried to call the '{ctx.command.name}' command. " - f"The in_channel check failed.") + # 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", ())): + log.trace(f"{ctx.author} may use the `{ctx.command.name}` command as they have a whitelisted role.") + return True - raise InChannelCheckFailure(*channels) + log.trace(f"{ctx.author} may not use the `{ctx.command.name}` command within this context.") + raise InWhitelistedContextCheckFailure(redirect_channel) return commands.check(predicate) 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 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 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 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 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 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 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 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