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 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(+) 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 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(+) 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(+) 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(+) 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(+) 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(-) 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(-) 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(-) 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(-) 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(+) 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(-) 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(-) 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 590c26355eb9b490a738afe936820c0e12c34873 Mon Sep 17 00:00:00 2001 From: ks123 Date: Mon, 16 Mar 2020 13:35:31 +0200 Subject: (Mod Log): Fixed case when `on_guild_channel_update` old or new value is empty and with this message formatting go wrong. --- bot/cogs/moderation/modlog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index 81d95298d..5d7c91ac4 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -215,7 +215,7 @@ class ModLog(Cog, name="ModLog"): new = value["new_value"] old = value["old_value"] - changes.append(f"**{key.title()}:** `{old}` **→** `{new}`") + changes.append(f"**{key.title()}:** `{old or 'None'}` **→** `{new or 'None'}`") done.append(key) -- cgit v1.2.3 From 8e60f04048cf9272daf2a2e08eab76a69af97bf4 Mon Sep 17 00:00:00 2001 From: Karlis S <45097959+ks129@users.noreply.github.com> Date: Mon, 16 Mar 2020 18:32:55 +0200 Subject: (Mod Log): Added comment about channel update formatting change. --- bot/cogs/moderation/modlog.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index 5d7c91ac4..21eded6e6 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -215,6 +215,8 @@ class ModLog(Cog, name="ModLog"): new = value["new_value"] old = value["old_value"] + # `or` is required here on `old` and `new` due otherwise, when one of them is empty, + # formatting in Discord will break. changes.append(f"**{key.title()}:** `{old or 'None'}` **→** `{new or 'None'}`") done.append(key) -- cgit v1.2.3 From 88d2d85ec114eac2b9e3be9b18e075302f73509e Mon Sep 17 00:00:00 2001 From: "S. Co1" Date: Mon, 16 Mar 2020 13:23:29 -0400 Subject: Update explanation comment so it explains what happens --- bot/cogs/moderation/modlog.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index 21eded6e6..5f9bc0c6c 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -215,8 +215,9 @@ class ModLog(Cog, name="ModLog"): new = value["new_value"] old = value["old_value"] - # `or` is required here on `old` and `new` due otherwise, when one of them is empty, - # formatting in Discord will break. + # Discord does not treat consecutive backticks ("``") as an empty inline code block, so the markdown + # formatting is broken when `new` and/or `old` are empty values. "None" is used for these cases so + # formatting is preserved. changes.append(f"**{key.title()}:** `{old or 'None'}` **→** `{new or 'None'}`") done.append(key) -- 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(+) 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