From 6e57f41d727c9cde51d36adc04393c3723d62472 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Mon, 28 Oct 2019 15:54:31 +0100 Subject: Enhance the output of the user command https://github.com/python-discord/bot/issues/628 https://github.com/python-discord/bot/issues/339 This commit introduces several changes to the output of the `!user` command for moderation staff. The output for regular users has not changed. Changes: - When issued in a moderation channel, the infraction count of the user will now be broken down by type as described in #339. This allows moderators to get a quicker overview of someone's history by providing more information. The command will display the total number of infractions per type, with the number of active infractions in parentheses behind it if there are any. This change also means that there no longer a need for the `hidden` parameter: When issued in a moderation channel, hidden infractions are included by default; when issued outside of a mod channel, the command will be equal to what a regular user would get. In addition to broken-down infraction info, the command now also shows information about the nominations of a user when it's issued inside of a moderation channel. - The code has been refactored to smaller units that take care of a single action to make unit testing easier. I have included tests that cover the command and all of the new helper methods. Tests for the other methods/commands in the cog will be added in the specific issue calling for tests for this cog (#581) This commit closes #628 and closes #339 --- tests/bot/cogs/test_information.py | 444 +++++++++++++++++++++++++++++++++++-- 1 file changed, 431 insertions(+), 13 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 9bbd35a91..5c34541d8 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -7,7 +7,11 @@ import discord from bot import constants from bot.cogs import information -from tests.helpers import AsyncMock, MockBot, MockContext, MockGuild, MockMember, MockRole +from bot.decorators import InChannelCheckFailure +from tests import helpers + + +COG_PATH = "bot.cogs.information.Information" class InformationCogTests(unittest.TestCase): @@ -15,22 +19,22 @@ class InformationCogTests(unittest.TestCase): @classmethod def setUpClass(cls): - cls.moderator_role = MockRole(name="Moderator", role_id=constants.Roles.moderator) + cls.moderator_role = helpers.MockRole(name="Moderator", role_id=constants.Roles.moderator) def setUp(self): """Sets up fresh objects for each test.""" - self.bot = MockBot() + self.bot = helpers.MockBot() self.cog = information.Information(self.bot) - self.ctx = MockContext() + self.ctx = helpers.MockContext() self.ctx.author.roles.append(self.moderator_role) def test_roles_command_command(self): """Test if the `role_info` command correctly returns the `moderator_role`.""" self.ctx.guild.roles.append(self.moderator_role) - self.cog.roles_info.can_run = AsyncMock() + self.cog.roles_info.can_run = helpers.AsyncMock() self.cog.roles_info.can_run.return_value = True coroutine = self.cog.roles_info.callback(self.cog, self.ctx) @@ -48,7 +52,7 @@ class InformationCogTests(unittest.TestCase): def test_role_info_command(self): """Tests the `role info` command.""" - dummy_role = MockRole( + dummy_role = helpers.MockRole( name="Dummy", role_id=112233445566778899, colour=discord.Colour.blurple(), @@ -57,7 +61,7 @@ class InformationCogTests(unittest.TestCase): permissions=discord.Permissions(0) ) - admin_role = MockRole( + admin_role = helpers.MockRole( name="Admins", role_id=998877665544332211, colour=discord.Colour.red(), @@ -68,7 +72,7 @@ class InformationCogTests(unittest.TestCase): self.ctx.guild.roles.append([dummy_role, admin_role]) - self.cog.role_info.can_run = AsyncMock() + self.cog.role_info.can_run = helpers.AsyncMock() self.cog.role_info.can_run.return_value = True coroutine = self.cog.role_info.callback(self.cog, self.ctx, dummy_role, admin_role) @@ -99,7 +103,7 @@ class InformationCogTests(unittest.TestCase): def test_server_info_command(self, time_since_patch): time_since_patch.return_value = '2 days ago' - self.ctx.guild = MockGuild( + self.ctx.guild = helpers.MockGuild( features=('lemons', 'apples'), region="The Moon", roles=[self.moderator_role], @@ -121,10 +125,10 @@ class InformationCogTests(unittest.TestCase): ) ], members=[ - *(MockMember(status='online') for _ in range(2)), - *(MockMember(status='idle') for _ in range(1)), - *(MockMember(status='dnd') for _ in range(4)), - *(MockMember(status='offline') for _ in range(3)), + *(helpers.MockMember(status='online') for _ in range(2)), + *(helpers.MockMember(status='idle') for _ in range(1)), + *(helpers.MockMember(status='dnd') for _ in range(4)), + *(helpers.MockMember(status='offline') for _ in range(3)), ], member_count=1_234, icon_url='a-lemon.jpg', @@ -162,3 +166,417 @@ class InformationCogTests(unittest.TestCase): ) ) self.assertEqual(embed.thumbnail.url, 'a-lemon.jpg') + + +class UserInfractionHelperMethodTests(unittest.TestCase): + """Tests for the helper methods of the `!user` command.""" + + def setUp(self): + """Common set-up steps done before for each test.""" + self.bot = helpers.MockBot() + self.bot.api_client.get = helpers.AsyncMock() + self.cog = information.Information(self.bot) + self.member = helpers.MockMember(user_id=1234) + + def test_user_command_helper_method_get_requests(self): + """The helper methods should form the correct get requests.""" + test_values = ( + { + "helper_method": self.cog.basic_user_infraction_counts, + "expected_args": ("bot/infractions", {'hidden': 'False', 'user__id': str(self.member.id)}), + }, + { + "helper_method": self.cog.expanded_user_infraction_counts, + "expected_args": ("bot/infractions", {'user__id': str(self.member.id)}), + }, + { + "helper_method": self.cog.user_nomination_counts, + "expected_args": ("bot/nominations", {'user__id': str(self.member.id)}), + }, + ) + + for test_value in test_values: + helper_method = test_value["helper_method"] + endpoint, params = test_value["expected_args"] + + with self.subTest(method=helper_method, endpoint=endpoint, params=params): + asyncio.run(helper_method(self.member)) + self.bot.api_client.get.assert_called_once_with(endpoint, params=params) + self.bot.api_client.get.reset_mock() + + def _method_subtests(self, method, test_values, default_header): + """Helper method that runs the subtests for the different helper methods.""" + for test_value in test_values: + api_response = test_value["api response"] + expected_lines = test_value["expected_lines"] + + with self.subTest(method=method, api_response=api_response, expected_lines=expected_lines): + self.bot.api_client.get.return_value = api_response + + expected_output = "\n".join(default_header + expected_lines) + actual_output = asyncio.run(method(self.member)) + + self.assertEqual(expected_output, actual_output) + + def test_basic_user_infraction_counts_returns_correct_strings(self): + """The method should correctly list both the total and active number of non-hidden infractions.""" + test_values = ( + # No infractions means zero counts + { + "api response": [], + "expected_lines": ["Total: 0", "Active: 0"], + }, + # Simple, single-infraction dictionaries + { + "api response": [{"type": "ban", "active": True}], + "expected_lines": ["Total: 1", "Active: 1"], + }, + { + "api response": [{"type": "ban", "active": False}], + "expected_lines": ["Total: 1", "Active: 0"], + }, + # Multiple infractions with various `active` status + { + "api response": [ + {"type": "ban", "active": True}, + {"type": "kick", "active": False}, + {"type": "ban", "active": True}, + {"type": "ban", "active": False}, + ], + "expected_lines": ["Total: 4", "Active: 2"], + }, + ) + + header = ["**Infractions**"] + + self._method_subtests(self.cog.basic_user_infraction_counts, test_values, header) + + def test_expanded_user_infraction_counts_returns_correct_strings(self): + """The method should correctly list the total and active number of all infractions split by infraction type.""" + test_values = ( + { + "api response": [], + "expected_lines": ["This user has never received an infraction."], + }, + # Shows non-hidden inactive infraction as expected + { + "api response": [{"type": "kick", "active": False, "hidden": False}], + "expected_lines": ["Kicks: 1"], + }, + # Shows non-hidden active infraction as expected + { + "api response": [{"type": "mute", "active": True, "hidden": False}], + "expected_lines": ["Mutes: 1 (1 active)"], + }, + # Shows hidden inactive infraction as expected + { + "api response": [{"type": "superstar", "active": False, "hidden": True}], + "expected_lines": ["Superstars: 1"], + }, + # Shows hidden active infraction as expected + { + "api response": [{"type": "ban", "active": True, "hidden": True}], + "expected_lines": ["Bans: 1 (1 active)"], + }, + # Correctly displays tally of multiple infractions of mixed properties in alphabetical order + { + "api response": [ + {"type": "kick", "active": False, "hidden": True}, + {"type": "ban", "active": True, "hidden": True}, + {"type": "superstar", "active": True, "hidden": True}, + {"type": "mute", "active": True, "hidden": True}, + {"type": "ban", "active": False, "hidden": False}, + {"type": "note", "active": False, "hidden": True}, + {"type": "note", "active": False, "hidden": True}, + {"type": "warn", "active": False, "hidden": False}, + {"type": "note", "active": False, "hidden": True}, + ], + "expected_lines": [ + "Bans: 2 (1 active)", + "Kicks: 1", + "Mutes: 1 (1 active)", + "Notes: 3", + "Superstars: 1 (1 active)", + "Warns: 1", + ], + }, + ) + + header = ["**Infractions**"] + + self._method_subtests(self.cog.expanded_user_infraction_counts, test_values, header) + + def test_user_nomination_counts_returns_correct_strings(self): + """The method should list the number of active and historical nominations for the user.""" + test_values = ( + { + "api response": [], + "expected_lines": ["This user has never been nominated."], + }, + { + "api response": [{'active': True}], + "expected_lines": ["This user is **currently** nominated (1 nomination in total)."], + }, + { + "api response": [{'active': True}, {'active': False}], + "expected_lines": ["This user is **currently** nominated (2 nominations in total)."], + }, + { + "api response": [{'active': False}], + "expected_lines": ["This user has 1 historical nomination, but is currently not nominated."], + }, + { + "api response": [{'active': False}, {'active': False}], + "expected_lines": ["This user has 2 historical nominations, but is currently not nominated."], + }, + + ) + + header = ["**Nominations**"] + + self._method_subtests(self.cog.user_nomination_counts, test_values, header) + + +@unittest.mock.patch("bot.cogs.information.time_since", new=unittest.mock.MagicMock(return_value="1 year ago")) +@unittest.mock.patch("bot.cogs.information.constants.MODERATION_CHANNELS", new=[50]) +class UserEmbedTests(unittest.TestCase): + """Tests for the creation of the `!user` embed.""" + + def setUp(self): + """Common set-up steps done before for each test.""" + self.bot = helpers.MockBot() + self.bot.api_client.get = helpers.AsyncMock() + self.cog = information.Information(self.bot) + + @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) + def test_create_user_embed_uses_string_representation_of_user_in_title_if_nick_is_not_available(self): + """The embed should use the string representation of the user if they don't have a nick.""" + ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) + user = helpers.MockMember() + user.nick = None + user.__str__ = unittest.mock.Mock(return_value="Mr. Hemlock") + + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + self.assertEqual(embed.title, "Mr. Hemlock") + + @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) + def test_create_user_embed_uses_nick_in_title_if_available(self): + """The embed should use the nick if it's available.""" + ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) + user = helpers.MockMember() + user.nick = "Cat lover" + user.__str__ = unittest.mock.Mock(return_value="Mr. Hemlock") + + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + self.assertEqual(embed.title, "Cat lover (Mr. Hemlock)") + + @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) + def test_create_user_embed_ignores_everyone_role(self): + """Created `!user` embeds should not contain mention of the @everyone-role.""" + ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) + admins_role = helpers.MockRole('Admins') + admins_role.colour = 100 + + # A `MockMember` has the @Everyone role by default; we add the Admins to that. + user = helpers.MockMember(roles=[admins_role], top_role=admins_role) + + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + self.assertIn("&Admins", embed.description) + self.assertNotIn("&Everyone", embed.description) + + @unittest.mock.patch(f"{COG_PATH}.expanded_user_infraction_counts", new_callable=helpers.AsyncMock) + @unittest.mock.patch(f"{COG_PATH}.user_nomination_counts", new_callable=helpers.AsyncMock) + def test_create_user_embed_expanded_information_in_moderation_channels(self, nomination_counts, infraction_counts): + """The embed should contain expanded infractions and nomination info in mod channels.""" + ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=50)) + + moderators_role = helpers.MockRole('Moderators') + moderators_role.colour = 100 + + infraction_counts.return_value = "expanded infractions info" + nomination_counts.return_value = "nomination info" + + user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + infraction_counts.assert_called_once_with(user) + nomination_counts.assert_called_once_with(user) + + self.assertEqual( + textwrap.dedent(f""" + **User Information** + Created: {"1 year ago"} + Profile: {user.mention} + ID: {user.id} + + **Member Information** + Joined: {"1 year ago"} + Roles: &Moderators + + expanded infractions info + + nomination info + """).strip(), + embed.description + ) + + @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new_callable=helpers.AsyncMock) + def test_create_user_embed_basic_information_outside_of_moderation_channels(self, infraction_counts): + """The embed should contain only basic infraction data outside of mod channels.""" + ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=100)) + + moderators_role = helpers.MockRole('Moderators') + moderators_role.colour = 100 + + infraction_counts.return_value = "basic infractions info" + + user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + infraction_counts.assert_called_once_with(user) + + self.assertEqual( + textwrap.dedent(f""" + **User Information** + Created: {"1 year ago"} + Profile: {user.mention} + ID: {user.id} + + **Member Information** + Joined: {"1 year ago"} + Roles: &Moderators + + basic infractions info + """).strip(), + embed.description + ) + + @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) + def test_create_user_embed_uses_top_role_colour_when_user_has_roles(self): + """The embed should be created with the colour of the top role, if a top role is available.""" + ctx = helpers.MockContext() + + moderators_role = helpers.MockRole('Moderators') + moderators_role.colour = 100 + + user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + self.assertEqual(embed.colour, discord.Colour(moderators_role.colour)) + + @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) + def test_create_user_embed_uses_blurple_colour_when_user_has_no_roles(self): + """The embed should be created with a blurple colour if the user has no assigned roles.""" + ctx = helpers.MockContext() + + user = helpers.MockMember(user_id=217) + embed = asyncio.run(self.cog.create_user_embed(ctx, user)) + + self.assertEqual(embed.colour, discord.Colour.blurple()) + + @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) + def test_create_user_embed_uses_png_format_of_user_avatar_as_thumbnail(self): + """The embed thumbnail should be set to the user's avatar in `png` format.""" + ctx = helpers.MockContext() + + user = helpers.MockMember(user_id=217) + 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") + self.assertEqual(embed.thumbnail.url, "avatar url") + + +@unittest.mock.patch("bot.cogs.information.constants") +class UserCommandTests(unittest.TestCase): + """Tests for the `!user` command.""" + + def setUp(self): + """Set up steps executed before each test is run.""" + self.bot = helpers.MockBot() + self.cog = information.Information(self.bot) + + self.moderator_role = helpers.MockRole("Moderators", role_id=2, position=10) + self.flautist_role = helpers.MockRole("Flautists", role_id=3, position=2) + self.bassist_role = helpers.MockRole("Bassists", role_id=4, position=3) + + self.author = helpers.MockMember(user_id=1, name="syntaxaire") + self.moderator = helpers.MockMember(user_id=2, name="riffautae", roles=[self.moderator_role]) + self.target = helpers.MockMember(user_id=3, name="__fluzz__") + + def test_regular_member_cannot_target_another_member(self, constants): + """A regular user should not be able to use `!user` targeting another user.""" + constants.MODERATION_ROLES = [self.moderator_role.id] + + ctx = helpers.MockContext(author=self.author) + + asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.target)) + + ctx.send.assert_called_once_with("You may not use this command on users other than yourself.") + + def test_regular_member_cannot_use_command_outside_of_bot_commands(self, constants): + """A regular user should not be able to use this command outside of bot-commands.""" + constants.MODERATION_ROLES = [self.moderator_role.id] + constants.STAFF_ROLES = [self.moderator_role.id] + constants.Channels.bot = 50 + + ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=100)) + + msg = "Sorry, but you may only use this command within <#50>." + with self.assertRaises(InChannelCheckFailure, msg=msg): + asyncio.run(self.cog.user_info.callback(self.cog, ctx)) + + @unittest.mock.patch("bot.cogs.information.Information.create_user_embed", new_callable=helpers.AsyncMock) + def test_regular_user_may_use_command_in_bot_commands_channel(self, create_embed, constants): + """A regular user should be allowed to use `!user` targeting themselves in bot-commands.""" + constants.STAFF_ROLES = [self.moderator_role.id] + constants.Channels.bot = 50 + + ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=50)) + + asyncio.run(self.cog.user_info.callback(self.cog, ctx)) + + create_embed.assert_called_once_with(ctx, self.author) + ctx.send.assert_called_once() + + @unittest.mock.patch("bot.cogs.information.Information.create_user_embed", new_callable=helpers.AsyncMock) + def test_regular_user_can_explicitly_target_themselves(self, create_embed, constants): + """A user should target itself with `!user` when a `user` argument was not provided.""" + constants.STAFF_ROLES = [self.moderator_role.id] + constants.Channels.bot = 50 + + ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=50)) + + asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.author)) + + create_embed.assert_called_once_with(ctx, self.author) + ctx.send.assert_called_once() + + @unittest.mock.patch("bot.cogs.information.Information.create_user_embed", new_callable=helpers.AsyncMock) + def test_staff_members_can_bypass_channel_restriction(self, create_embed, constants): + """Staff members should be able to bypass the bot-commands channel restriction.""" + constants.STAFF_ROLES = [self.moderator_role.id] + constants.Channels.bot = 50 + + ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(channel_id=200)) + + asyncio.run(self.cog.user_info.callback(self.cog, ctx)) + + create_embed.assert_called_once_with(ctx, self.moderator) + ctx.send.assert_called_once() + + @unittest.mock.patch("bot.cogs.information.Information.create_user_embed", new_callable=helpers.AsyncMock) + def test_moderators_can_target_another_member(self, create_embed, constants): + """A moderator should be able to use `!user` targeting another user.""" + constants.MODERATION_ROLES = [self.moderator_role.id] + constants.STAFF_ROLES = [self.moderator_role.id] + + ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(channel_id=50)) + + asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.target)) + + create_embed.assert_called_once_with(ctx, self.target) + ctx.send.assert_called_once() -- cgit v1.2.3 From 38d9e705c77003008f1562188cba079a4245061b Mon Sep 17 00:00:00 2001 From: kwzrd Date: Fri, 1 Nov 2019 23:01:45 +0100 Subject: Add unit test for links antispam rule --- tests/bot/rules/test_links.py | 93 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 tests/bot/rules/test_links.py (limited to 'tests') diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py new file mode 100644 index 000000000..f71a8e6bb --- /dev/null +++ b/tests/bot/rules/test_links.py @@ -0,0 +1,93 @@ +import unittest +from typing import List, NamedTuple, Tuple + +from bot.rules import links +from tests.helpers import async_test + + +class FakeMessage(NamedTuple): + author: str + content: str + + +class Case(NamedTuple): + recent_messages: List[FakeMessage] + relevant_messages: Tuple[FakeMessage] + culprit: Tuple[str] + total_links: int + + +def msg(author: str, total_links: int) -> FakeMessage: + """Makes a message with `total_links` links.""" + content = " ".join(["https://pydis.com"] * total_links) + return FakeMessage(author=author, content=content) + + +class LinksTests(unittest.TestCase): + """Tests applying the `links` rule.""" + + def setUp(self): + self.config = { + "max": 2, + "interval": 10 + } + + @async_test + async def test_links_within_limit(self): + """Messages with an allowed amount of links.""" + cases = ( + [msg("bob", 0)], + [msg("bob", 2)], + [msg("bob", 3)], + [msg("bob", 3), msg("alice", 3)] + ) + + for recent_messages in cases: + last_message = recent_messages[0] + with self.subTest( + last_message=last_message, + recent_messages=recent_messages, + config=self.config + ): + self.assertIsNone( + await links.apply(last_message, recent_messages, self.config) + ) + + @async_test + async def test_links_exceeding_limit(self): + """Messages with a a higher than allowed amount of links.""" + cases = ( + Case( + [msg("bob", 1), msg("bob", 2)], + (msg("bob", 1), msg("bob", 2)), + ("bob",), + 3 + ), + Case( + [msg("alice", 2), msg("bob", 3), msg("alice", 1)], + (msg("alice", 2), msg("alice", 1)), + ("alice",), + 3 + ) + ) + + for recent_messages, relevant_messages, culprit, total_links in cases: + last_message = recent_messages[0] + + with self.subTest( + last_message=last_message, + recent_messages=recent_messages, + relevant_messages=relevant_messages, + culprit=culprit, + total_links=total_links, + config=self.config + ): + desired_output = ( + f"sent {total_links} links in {self.config['interval']}s", + culprit, + relevant_messages + ) + self.assertTupleEqual( + await links.apply(last_message, recent_messages, self.config), + desired_output + ) -- cgit v1.2.3 From a21670459953599c8f13286595520e033da4199a Mon Sep 17 00:00:00 2001 From: kwzrd Date: Fri, 1 Nov 2019 23:24:20 +0100 Subject: Add two more test cases for links rule unit test --- tests/bot/rules/test_links.py | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'tests') diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index f71a8e6bb..f043495cf 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -39,6 +39,7 @@ class LinksTests(unittest.TestCase): [msg("bob", 0)], [msg("bob", 2)], [msg("bob", 3)], + [msg("bob", 1), msg("bob", 1)], [msg("bob", 3), msg("alice", 3)] ) @@ -63,6 +64,12 @@ class LinksTests(unittest.TestCase): ("bob",), 3 ), + Case( + [msg("alice", 1), msg("alice", 1), msg("alice", 1)], + (msg("alice", 1), msg("alice", 1), msg("alice", 1)), + ("alice",), + 3 + ), Case( [msg("alice", 2), msg("bob", 3), msg("alice", 1)], (msg("alice", 2), msg("alice", 1)), -- cgit v1.2.3 From 9e16b0cf17e93295167956d31ad9f92396ca3402 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 2 Nov 2019 01:19:07 +0100 Subject: Annotate unclear test cases with inline comments --- tests/bot/rules/test_links.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index f043495cf..a04b90c13 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -38,9 +38,9 @@ class LinksTests(unittest.TestCase): cases = ( [msg("bob", 0)], [msg("bob", 2)], - [msg("bob", 3)], + [msg("bob", 3)], # Filter only applies if len(messages_with_links) > 1 [msg("bob", 1), msg("bob", 1)], - [msg("bob", 3), msg("alice", 3)] + [msg("bob", 3), msg("alice", 3)] # Only messages from latest author count ) for recent_messages in cases: -- cgit v1.2.3 From f6ed29c8692759dc0c8b003ebf2d4ce3d5ca6ed9 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 2 Nov 2019 01:34:46 +0100 Subject: Adjust case to only test a single aspect --- tests/bot/rules/test_links.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index a04b90c13..95f107f91 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -40,7 +40,7 @@ class LinksTests(unittest.TestCase): [msg("bob", 2)], [msg("bob", 3)], # Filter only applies if len(messages_with_links) > 1 [msg("bob", 1), msg("bob", 1)], - [msg("bob", 3), msg("alice", 3)] # Only messages from latest author count + [msg("bob", 2), msg("alice", 2)] # Only messages from latest author count ) for recent_messages in cases: -- cgit v1.2.3 From 9000d92ab8ceaf855d23c0e6dfdb09d05a40e59a Mon Sep 17 00:00:00 2001 From: kwzrd Date: Fri, 8 Nov 2019 22:07:00 +0100 Subject: Add whitespace for readability, consistency & allure --- tests/bot/rules/test_links.py | 1 + 1 file changed, 1 insertion(+) (limited to 'tests') diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index 95f107f91..40336beb0 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -45,6 +45,7 @@ class LinksTests(unittest.TestCase): for recent_messages in cases: last_message = recent_messages[0] + with self.subTest( last_message=last_message, recent_messages=recent_messages, -- cgit v1.2.3 From 760f265339b1462b60ea7d5af1fd8d4476c134f0 Mon Sep 17 00:00:00 2001 From: kwzrd <44734341+kwzrd@users.noreply.github.com> Date: Fri, 8 Nov 2019 22:52:47 +0100 Subject: Update docstring to use asterisks when referring to argument names Co-Authored-By: Kyle Stanley --- tests/bot/rules/test_links.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index 40336beb0..be832843b 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -18,7 +18,7 @@ class Case(NamedTuple): def msg(author: str, total_links: int) -> FakeMessage: - """Makes a message with `total_links` links.""" + """Makes a message with *total_links* links.""" content = " ".join(["https://pydis.com"] * total_links) return FakeMessage(author=author, content=content) -- cgit v1.2.3 From dceafb83e829548638e8589c88f80364e8009821 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 13 Nov 2019 15:32:13 +0100 Subject: Prevent unwanted logging while running tests Previously, logging messages would output to std.out. when running individual test files (instead of running the entire suite). To prevent this, I've added a `for`-loop to `tests.helpers` that sets the level of all registered loggers to `CRITICAL`. The reason for adding this to `tests.helpers` is simple: It's the most common file to be imported in individual tests, increasing the chance of the code being run for individual test files. A small downside of this way of handling logging is that when we are trying to assert logging messages are being emitted, we need to set the logger explicitly in the `self.assertLogs` context manager. This is a small downside, though, and probably good practice anyway. There was one test in `tests.bot.test_api` that did not do this, so I have changed this to make the test compatible with the new set-up. --- tests/bot/test_api.py | 4 +++- tests/helpers.py | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/test_api.py b/tests/bot/test_api.py index e0ede0eb1..5a88adc5c 100644 --- a/tests/bot/test_api.py +++ b/tests/bot/test_api.py @@ -121,7 +121,9 @@ class LoggingHandlerTests(LoggingTestCase): def test_schedule_queued_tasks_for_nonempty_queue(self): """`APILoggingHandler` should schedule logs when the queue is not empty.""" - with self.assertLogs(level=logging.DEBUG) as logs, patch('asyncio.create_task') as create_task: + log = logging.getLogger("bot.api") + + with self.assertLogs(logger=log, level=logging.DEBUG) as logs, patch('asyncio.create_task') as create_task: self.log_handler.queue = [555] self.log_handler.schedule_queued_tasks() self.assertListEqual(self.log_handler.queue, []) diff --git a/tests/helpers.py b/tests/helpers.py index 8496ba031..8d661513d 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio import functools import inspect +import logging import unittest.mock from typing import Any, Iterable, Optional @@ -10,6 +11,16 @@ import discord from discord.ext.commands import Bot, Context +for logger in logging.Logger.manager.loggerDict.values(): + # Set all loggers to CRITICAL by default to prevent screen clutter during testing + + if not isinstance(logger, logging.Logger): + # There might be some logging.PlaceHolder objects in there + continue + + logger.setLevel(logging.CRITICAL) + + def async_test(wrapped): """ Run a test case via asyncio. -- cgit v1.2.3 From bf7720f16fa69716f15b16e3dcd0f20c186958b8 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 13 Nov 2019 16:06:51 +0100 Subject: Allow `name` attribute to be set during Mock init The `name` keyword argument has a special meaning for the default mockobjects provided by `unittest.mock`. This means that by default, the common d.py `name` attribute can't be set during initalization of one of our custom Mock-objects by passing it to the constructor. Since it's unlikely for us to make use of the special `name` feature of mocks and more likely to want to set the d.py `name` attribute, I added special handling of the `name` kwarg. --- tests/helpers.py | 3 +++ 1 file changed, 3 insertions(+) (limited to 'tests') diff --git a/tests/helpers.py b/tests/helpers.py index 8d661513d..5dc7a0d2f 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -74,7 +74,10 @@ class CustomMockMixin: child_mock_type = unittest.mock.MagicMock def __init__(self, spec: Any = None, **kwargs): + name = kwargs.pop('name', None) # `name` has special meaning for Mock classes, so we need to set it manually. super().__init__(spec=spec, **kwargs) + if name: + self.name = name if spec: self._extract_coroutine_methods_from_spec_instance(spec) -- cgit v1.2.3 From 36e9de480dcabb3c844090a4fd87561534536c04 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 13 Nov 2019 11:24:58 +0100 Subject: Prevent setting unknown attributes on d.py mocks Our custom `discord.py` now follow the specifications of the object they are mocking more strictly by using the `spec_set` instead of the `spec` kwarg to initialize the specifications. This means that trying to set an attribute that does not follow the specifications will now also result in an `AttributeError`. To make sure we are not trying to set illegal attributes during the default initialization of the mock objects, I've changed the way we handle default values of parameters. This does introduce a breaking change: Instead of passing a `suffix_id`, the `id` attribute should now be passed using the exact name. `id`. This commit also makes sure existing tests follow this change. --- tests/bot/cogs/test_information.py | 58 ++++++++++----------- tests/bot/cogs/test_token_remover.py | 2 +- tests/bot/utils/test_checks.py | 6 +-- tests/helpers.py | 94 +++++++++++++--------------------- tests/test_helpers.py | 99 ++++++++++++------------------------ 5 files changed, 101 insertions(+), 158 deletions(-) (limited to 'tests') diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 5c34541d8..4496a2ae0 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -19,7 +19,7 @@ class InformationCogTests(unittest.TestCase): @classmethod def setUpClass(cls): - cls.moderator_role = helpers.MockRole(name="Moderator", role_id=constants.Roles.moderator) + cls.moderator_role = helpers.MockRole(name="Moderator", id=constants.Roles.moderator) def setUp(self): """Sets up fresh objects for each test.""" @@ -54,7 +54,7 @@ class InformationCogTests(unittest.TestCase): """Tests the `role info` command.""" dummy_role = helpers.MockRole( name="Dummy", - role_id=112233445566778899, + id=112233445566778899, colour=discord.Colour.blurple(), position=10, members=[self.ctx.author], @@ -63,7 +63,7 @@ class InformationCogTests(unittest.TestCase): admin_role = helpers.MockRole( name="Admins", - role_id=998877665544332211, + id=998877665544332211, colour=discord.Colour.red(), position=3, members=[self.ctx.author], @@ -176,7 +176,7 @@ class UserInfractionHelperMethodTests(unittest.TestCase): self.bot = helpers.MockBot() self.bot.api_client.get = helpers.AsyncMock() self.cog = information.Information(self.bot) - self.member = helpers.MockMember(user_id=1234) + self.member = helpers.MockMember(id=1234) def test_user_command_helper_method_get_requests(self): """The helper methods should form the correct get requests.""" @@ -351,7 +351,7 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) def test_create_user_embed_uses_string_representation_of_user_in_title_if_nick_is_not_available(self): """The embed should use the string representation of the user if they don't have a nick.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=1)) user = helpers.MockMember() user.nick = None user.__str__ = unittest.mock.Mock(return_value="Mr. Hemlock") @@ -363,7 +363,7 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) def test_create_user_embed_uses_nick_in_title_if_available(self): """The embed should use the nick if it's available.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=1)) user = helpers.MockMember() user.nick = "Cat lover" user.__str__ = unittest.mock.Mock(return_value="Mr. Hemlock") @@ -375,8 +375,8 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new=helpers.AsyncMock(return_value="")) def test_create_user_embed_ignores_everyone_role(self): """Created `!user` embeds should not contain mention of the @everyone-role.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=1)) - admins_role = helpers.MockRole('Admins') + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=1)) + admins_role = helpers.MockRole(name='Admins') admins_role.colour = 100 # A `MockMember` has the @Everyone role by default; we add the Admins to that. @@ -391,15 +391,15 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.user_nomination_counts", new_callable=helpers.AsyncMock) def test_create_user_embed_expanded_information_in_moderation_channels(self, nomination_counts, infraction_counts): """The embed should contain expanded infractions and nomination info in mod channels.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=50)) + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=50)) - moderators_role = helpers.MockRole('Moderators') + moderators_role = helpers.MockRole(name='Moderators') moderators_role.colour = 100 infraction_counts.return_value = "expanded infractions info" nomination_counts.return_value = "nomination info" - user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + user = helpers.MockMember(id=314, roles=[moderators_role], top_role=moderators_role) embed = asyncio.run(self.cog.create_user_embed(ctx, user)) infraction_counts.assert_called_once_with(user) @@ -426,14 +426,14 @@ class UserEmbedTests(unittest.TestCase): @unittest.mock.patch(f"{COG_PATH}.basic_user_infraction_counts", new_callable=helpers.AsyncMock) def test_create_user_embed_basic_information_outside_of_moderation_channels(self, infraction_counts): """The embed should contain only basic infraction data outside of mod channels.""" - ctx = helpers.MockContext(channel=helpers.MockTextChannel(channel_id=100)) + ctx = helpers.MockContext(channel=helpers.MockTextChannel(id=100)) - moderators_role = helpers.MockRole('Moderators') + moderators_role = helpers.MockRole(name='Moderators') moderators_role.colour = 100 infraction_counts.return_value = "basic infractions info" - user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + user = helpers.MockMember(id=314, roles=[moderators_role], top_role=moderators_role) embed = asyncio.run(self.cog.create_user_embed(ctx, user)) infraction_counts.assert_called_once_with(user) @@ -459,10 +459,10 @@ class UserEmbedTests(unittest.TestCase): """The embed should be created with the colour of the top role, if a top role is available.""" ctx = helpers.MockContext() - moderators_role = helpers.MockRole('Moderators') + moderators_role = helpers.MockRole(name='Moderators') moderators_role.colour = 100 - user = helpers.MockMember(user_id=314, roles=[moderators_role], top_role=moderators_role) + user = helpers.MockMember(id=314, roles=[moderators_role], top_role=moderators_role) embed = asyncio.run(self.cog.create_user_embed(ctx, user)) self.assertEqual(embed.colour, discord.Colour(moderators_role.colour)) @@ -472,7 +472,7 @@ class UserEmbedTests(unittest.TestCase): """The embed should be created with a blurple colour if the user has no assigned roles.""" ctx = helpers.MockContext() - user = helpers.MockMember(user_id=217) + user = helpers.MockMember(id=217) embed = asyncio.run(self.cog.create_user_embed(ctx, user)) self.assertEqual(embed.colour, discord.Colour.blurple()) @@ -482,7 +482,7 @@ class UserEmbedTests(unittest.TestCase): """The embed thumbnail should be set to the user's avatar in `png` format.""" ctx = helpers.MockContext() - user = helpers.MockMember(user_id=217) + user = helpers.MockMember(id=217) user.avatar_url_as.return_value = "avatar url" embed = asyncio.run(self.cog.create_user_embed(ctx, user)) @@ -499,13 +499,13 @@ class UserCommandTests(unittest.TestCase): self.bot = helpers.MockBot() self.cog = information.Information(self.bot) - self.moderator_role = helpers.MockRole("Moderators", role_id=2, position=10) - self.flautist_role = helpers.MockRole("Flautists", role_id=3, position=2) - self.bassist_role = helpers.MockRole("Bassists", role_id=4, position=3) + self.moderator_role = helpers.MockRole(name="Moderators", id=2, position=10) + self.flautist_role = helpers.MockRole(name="Flautists", id=3, position=2) + self.bassist_role = helpers.MockRole(name="Bassists", id=4, position=3) - self.author = helpers.MockMember(user_id=1, name="syntaxaire") - self.moderator = helpers.MockMember(user_id=2, name="riffautae", roles=[self.moderator_role]) - self.target = helpers.MockMember(user_id=3, name="__fluzz__") + self.author = helpers.MockMember(id=1, name="syntaxaire") + self.moderator = helpers.MockMember(id=2, name="riffautae", roles=[self.moderator_role]) + self.target = helpers.MockMember(id=3, name="__fluzz__") def test_regular_member_cannot_target_another_member(self, constants): """A regular user should not be able to use `!user` targeting another user.""" @@ -523,7 +523,7 @@ class UserCommandTests(unittest.TestCase): constants.STAFF_ROLES = [self.moderator_role.id] constants.Channels.bot = 50 - ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=100)) + 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): @@ -535,7 +535,7 @@ class UserCommandTests(unittest.TestCase): constants.STAFF_ROLES = [self.moderator_role.id] constants.Channels.bot = 50 - ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=50)) + ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=50)) asyncio.run(self.cog.user_info.callback(self.cog, ctx)) @@ -548,7 +548,7 @@ class UserCommandTests(unittest.TestCase): constants.STAFF_ROLES = [self.moderator_role.id] constants.Channels.bot = 50 - ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(channel_id=50)) + ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=50)) asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.author)) @@ -561,7 +561,7 @@ class UserCommandTests(unittest.TestCase): constants.STAFF_ROLES = [self.moderator_role.id] constants.Channels.bot = 50 - ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(channel_id=200)) + ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(id=200)) asyncio.run(self.cog.user_info.callback(self.cog, ctx)) @@ -574,7 +574,7 @@ class UserCommandTests(unittest.TestCase): constants.MODERATION_ROLES = [self.moderator_role.id] constants.STAFF_ROLES = [self.moderator_role.id] - ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(channel_id=50)) + ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(id=50)) asyncio.run(self.cog.user_info.callback(self.cog, ctx, self.target)) diff --git a/tests/bot/cogs/test_token_remover.py b/tests/bot/cogs/test_token_remover.py index dfb1bafc9..3276cf5a5 100644 --- a/tests/bot/cogs/test_token_remover.py +++ b/tests/bot/cogs/test_token_remover.py @@ -24,7 +24,7 @@ class TokenRemoverTests(unittest.TestCase): self.bot.get_cog.return_value.send_log_message = AsyncMock() self.cog = TokenRemover(bot=self.bot) - self.msg = MockMessage(message_id=555, content='') + self.msg = MockMessage(id=555, content='') self.msg.author.__str__ = MagicMock() self.msg.author.__str__.return_value = 'lemon' self.msg.author.bot = False diff --git a/tests/bot/utils/test_checks.py b/tests/bot/utils/test_checks.py index 19b758336..9610771e5 100644 --- a/tests/bot/utils/test_checks.py +++ b/tests/bot/utils/test_checks.py @@ -22,7 +22,7 @@ class ChecksTests(unittest.TestCase): def test_with_role_check_with_guild_and_required_role(self): """`with_role_check` returns `True` if `Context.author` has the required role.""" - self.ctx.author.roles.append(MockRole(role_id=10)) + self.ctx.author.roles.append(MockRole(id=10)) self.assertTrue(checks.with_role_check(self.ctx, 10)) def test_without_role_check_without_guild(self): @@ -33,13 +33,13 @@ class ChecksTests(unittest.TestCase): def test_without_role_check_returns_false_with_unwanted_role(self): """`without_role_check` returns `False` if `Context.author` has unwanted role.""" role_id = 42 - self.ctx.author.roles.append(MockRole(role_id=role_id)) + self.ctx.author.roles.append(MockRole(id=role_id)) self.assertFalse(checks.without_role_check(self.ctx, role_id)) def test_without_role_check_returns_true_without_unwanted_role(self): """`without_role_check` returns `True` if `Context.author` does not have unwanted role.""" role_id = 42 - self.ctx.author.roles.append(MockRole(role_id=role_id)) + self.ctx.author.roles.append(MockRole(id=role_id)) self.assertTrue(checks.without_role_check(self.ctx, role_id + 10)) def test_in_channel_check_for_correct_channel(self): diff --git a/tests/helpers.py b/tests/helpers.py index 5dc7a0d2f..35f2c288c 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -1,8 +1,10 @@ from __future__ import annotations import asyncio +import collections import functools import inspect +import itertools import logging import unittest.mock from typing import Any, Iterable, Optional @@ -72,14 +74,16 @@ class CustomMockMixin: """ child_mock_type = unittest.mock.MagicMock + discord_id = itertools.count(0) - def __init__(self, spec: Any = None, **kwargs): + def __init__(self, spec_set: Any = None, **kwargs): name = kwargs.pop('name', None) # `name` has special meaning for Mock classes, so we need to set it manually. - super().__init__(spec=spec, **kwargs) + super().__init__(spec_set=spec_set, **kwargs) + if name: self.name = name - if spec: - self._extract_coroutine_methods_from_spec_instance(spec) + if spec_set: + self._extract_coroutine_methods_from_spec_instance(spec_set) def _get_child_mock(self, **kw): """ @@ -169,25 +173,14 @@ class MockGuild(CustomMockMixin, unittest.mock.Mock, HashableMixin): For more info, see the `Mocking` section in `tests/README.md`. """ - def __init__( - self, - guild_id: int = 1, - roles: Optional[Iterable[MockRole]] = None, - members: Optional[Iterable[MockMember]] = None, - **kwargs, - ) -> None: - super().__init__(spec=guild_instance, **kwargs) - - self.id = guild_id - - self.roles = [MockRole("@everyone", 1)] + def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None: + default_kwargs = {'id': next(self.discord_id), 'members': []} + super().__init__(spec_set=guild_instance, **collections.ChainMap(kwargs, default_kwargs)) + + self.roles = [MockRole(name="@everyone", position=1, id=0)] if roles: self.roles.extend(roles) - self.members = [] - if members: - self.members.extend(members) - # Create a Role instance to get a realistic Mock of `discord.Role` role_data = {'name': 'role', 'id': 1} @@ -201,13 +194,12 @@ class MockRole(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): Instances of this class will follow the specifications of `discord.Role` instances. For more information, see the `MockGuild` docstring. """ - def __init__(self, name: str = "role", role_id: int = 1, position: int = 1, **kwargs) -> None: - super().__init__(spec=role_instance, **kwargs) + def __init__(self, **kwargs) -> None: + default_kwargs = {'id': next(self.discord_id), 'name': 'role', 'position': 1} + super().__init__(spec_set=role_instance, **collections.ChainMap(kwargs, default_kwargs)) - self.name = name - self.id = role_id - self.position = position - self.mention = f'&{self.name}' + if 'mention' not in kwargs: + self.mention = f'&{self.name}' def __lt__(self, other): """Simplified position-based comparisons similar to those of `discord.Role`.""" @@ -227,19 +219,11 @@ class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin Instances of this class will follow the specifications of `discord.Member` instances. For more information, see the `MockGuild` docstring. """ - def __init__( - self, - name: str = "member", - user_id: int = 1, - roles: Optional[Iterable[MockRole]] = None, - **kwargs, - ) -> None: - super().__init__(spec=member_instance, **kwargs) - - self.name = name - self.id = user_id - - self.roles = [MockRole("@everyone", 1)] + def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None: + default_kwargs = {'name': 'member', 'id': next(self.discord_id)} + super().__init__(spec_set=member_instance, **collections.ChainMap(kwargs, default_kwargs)) + + self.roles = [MockRole(name="@everyone", position=1, id=0)] if roles: self.roles.extend(roles) @@ -248,6 +232,8 @@ class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin # Create a Bot instance to get a realistic MagicMock of `discord.ext.commands.Bot` bot_instance = Bot(command_prefix=unittest.mock.MagicMock()) +bot_instance.http_session = None +bot_instance.api_client = None class MockBot(CustomMockMixin, unittest.mock.MagicMock): @@ -258,11 +244,7 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): For more information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=bot_instance, **kwargs) - - # Our custom attributes and methods - self.http_session = unittest.mock.MagicMock() - self.api_client = unittest.mock.MagicMock() + super().__init__(spec_set=bot_instance, **kwargs) # self.wait_for is *not* a coroutine function, but returns a coroutine nonetheless and # and should therefore be awaited. (The documentation calls it a coroutine as well, which @@ -294,11 +276,11 @@ class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): more information, see the `MockGuild` docstring. """ def __init__(self, name: str = 'channel', channel_id: int = 1, **kwargs) -> None: - super().__init__(spec=channel_instance, **kwargs) - self.id = channel_id - self.name = name - self.guild = kwargs.get('guild', MockGuild()) - self.mention = f"#{self.name}" + default_kwargs = {'id': next(self.discord_id), 'name': 'channel', 'guild': MockGuild()} + super().__init__(spec_set=channel_instance, **collections.ChainMap(kwargs, default_kwargs)) + + if 'mention' not in kwargs: + self.mention = f"#{self.name}" # Create a Message instance to get a realistic MagicMock of `discord.Message` @@ -335,12 +317,11 @@ class MockContext(CustomMockMixin, unittest.mock.MagicMock): instances. For more information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=context_instance, **kwargs) + super().__init__(spec_set=context_instance, **kwargs) self.bot = kwargs.get('bot', MockBot()) self.guild = kwargs.get('guild', MockGuild()) self.author = kwargs.get('author', MockMember()) self.channel = kwargs.get('channel', MockTextChannel()) - self.command = kwargs.get('command', unittest.mock.MagicMock()) class MockMessage(CustomMockMixin, unittest.mock.MagicMock): @@ -351,7 +332,7 @@ class MockMessage(CustomMockMixin, unittest.mock.MagicMock): information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=message_instance, **kwargs) + super().__init__(spec_set=message_instance, **kwargs) self.author = kwargs.get('author', MockMember()) self.channel = kwargs.get('channel', MockTextChannel()) @@ -368,12 +349,9 @@ class MockEmoji(CustomMockMixin, unittest.mock.MagicMock): information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=emoji_instance, **kwargs) + super().__init__(spec_set=emoji_instance, **kwargs) self.guild = kwargs.get('guild', MockGuild()) - # Get all coroutine functions and set them as AsyncMock attributes - self._extract_coroutine_methods_from_spec_instance(emoji_instance) - partial_emoji_instance = discord.PartialEmoji(animated=False, name='guido') @@ -386,7 +364,7 @@ class MockPartialEmoji(CustomMockMixin, unittest.mock.MagicMock): more information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=partial_emoji_instance, **kwargs) + super().__init__(spec_set=partial_emoji_instance, **kwargs) reaction_instance = discord.Reaction(message=MockMessage(), data={'me': True}, emoji=MockEmoji()) @@ -400,6 +378,6 @@ class MockReaction(CustomMockMixin, unittest.mock.MagicMock): more information, see the `MockGuild` docstring. """ def __init__(self, **kwargs) -> None: - super().__init__(spec=reaction_instance, **kwargs) + super().__init__(spec_set=reaction_instance, **kwargs) self.emoji = kwargs.get('emoji', MockEmoji()) self.message = kwargs.get('message', MockMessage()) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 2b58634dd..e879ef97a 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -19,7 +19,6 @@ class DiscordMocksTests(unittest.TestCase): self.assertIsInstance(role, discord.Role) self.assertEqual(role.name, "role") - self.assertEqual(role.id, 1) self.assertEqual(role.position, 1) self.assertEqual(role.mention, "&role") @@ -27,7 +26,7 @@ class DiscordMocksTests(unittest.TestCase): """Test if MockRole initializes with the arguments provided.""" role = helpers.MockRole( name="Admins", - role_id=90210, + id=90210, position=10, ) @@ -67,22 +66,21 @@ class DiscordMocksTests(unittest.TestCase): self.assertIsInstance(member, discord.Member) self.assertEqual(member.name, "member") - self.assertEqual(member.id, 1) - self.assertListEqual(member.roles, [helpers.MockRole("@everyone", 1)]) + self.assertListEqual(member.roles, [helpers.MockRole(name="@everyone", position=1, id=0)]) self.assertEqual(member.mention, "@member") def test_mock_member_alternative_arguments(self): """Test if MockMember initializes with the arguments provided.""" - core_developer = helpers.MockRole("Core Developer", 2) + core_developer = helpers.MockRole(name="Core Developer", position=2) member = helpers.MockMember( name="Mark", - user_id=12345, + id=12345, roles=[core_developer] ) self.assertEqual(member.name, "Mark") self.assertEqual(member.id, 12345) - self.assertListEqual(member.roles, [helpers.MockRole("@everyone", 1), core_developer]) + self.assertListEqual(member.roles, [helpers.MockRole(name="@everyone", position=1, id=0), core_developer]) self.assertEqual(member.mention, "@Mark") def test_mock_member_accepts_dynamic_arguments(self): @@ -102,19 +100,19 @@ class DiscordMocksTests(unittest.TestCase): # The `spec` argument makes sure `isistance` checks with `discord.Guild` pass self.assertIsInstance(guild, discord.Guild) - self.assertListEqual(guild.roles, [helpers.MockRole("@everyone", 1)]) + self.assertListEqual(guild.roles, [helpers.MockRole(name="@everyone", position=1, id=0)]) self.assertListEqual(guild.members, []) def test_mock_guild_alternative_arguments(self): """Test if MockGuild initializes with the arguments provided.""" - core_developer = helpers.MockRole("Core Developer", 2) + core_developer = helpers.MockRole(name="Core Developer", position=2) guild = helpers.MockGuild( roles=[core_developer], - members=[helpers.MockMember(user_id=54321)], + members=[helpers.MockMember(id=54321)], ) - self.assertListEqual(guild.roles, [helpers.MockRole("@everyone", 1), core_developer]) - self.assertListEqual(guild.members, [helpers.MockMember(user_id=54321)]) + self.assertListEqual(guild.roles, [helpers.MockRole(name="@everyone", position=1, id=0), core_developer]) + self.assertListEqual(guild.members, [helpers.MockMember(id=54321)]) def test_mock_guild_accepts_dynamic_arguments(self): """Test if MockGuild accepts and sets abitrary keyword arguments.""" @@ -191,51 +189,18 @@ class DiscordMocksTests(unittest.TestCase): with self.assertRaises(AttributeError): mock.the_cake_is_a_lie - def test_custom_mock_methods_are_valid_discord_object_methods(self): - """The `AsyncMock` attributes of the mocks should be valid for the class they're mocking.""" - mocks = ( - (helpers.MockGuild, helpers.guild_instance), - (helpers.MockRole, helpers.role_instance), - (helpers.MockMember, helpers.member_instance), - (helpers.MockBot, helpers.bot_instance), - (helpers.MockContext, helpers.context_instance), - (helpers.MockTextChannel, helpers.channel_instance), - (helpers.MockMessage, helpers.message_instance), + def test_mocks_use_mention_when_provided_as_kwarg(self): + """The mock should use the passed `mention` instead of the default one if present.""" + test_cases = ( + (helpers.MockRole, "role mention"), + (helpers.MockMember, "member mention"), + (helpers.MockTextChannel, "channel mention"), ) - for mock_class, instance in mocks: - mock = mock_class() - async_methods = ( - attr for attr in dir(mock) if isinstance(getattr(mock, attr), helpers.AsyncMock) - ) - - # spec_mock = unittest.mock.MagicMock(spec=instance) - for method in async_methods: - with self.subTest(mock_class=mock_class, method=method): - try: - getattr(instance, method) - except AttributeError: - msg = f"method {method} is not a method attribute of {instance.__class__}" - self.fail(msg) - - @unittest.mock.patch(f'{__name__}.DiscordMocksTests.subTest') - def test_the_custom_mock_methods_test(self, subtest_mock): - """The custom method test should raise AssertionError for invalid methods.""" - class FakeMockBot(helpers.CustomMockMixin, unittest.mock.MagicMock): - """Fake MockBot class with invalid attribute/method `release_the_walrus`.""" - - child_mock_type = unittest.mock.MagicMock - - def __init__(self, **kwargs): - super().__init__(spec=helpers.bot_instance, **kwargs) - - # Fake attribute - self.release_the_walrus = helpers.AsyncMock() - - with unittest.mock.patch("tests.helpers.MockBot", new=FakeMockBot): - msg = "method release_the_walrus is not a valid method of " - with self.assertRaises(AssertionError, msg=msg): - self.test_custom_mock_methods_are_valid_discord_object_methods() + for mock_type, mention in test_cases: + with self.subTest(mock_type=mock_type, mention=mention): + mock = mock_type(mention=mention) + self.assertEqual(mock.mention, mention) class MockObjectTests(unittest.TestCase): @@ -266,14 +231,14 @@ class MockObjectTests(unittest.TestCase): def test_hashable_mixin_uses_id_for_equality_comparison(self): """Test if the HashableMixing uses the id attribute for hashing.""" - class MockScragly(unittest.mock.Mock, helpers.HashableMixin): + class MockScragly(helpers.HashableMixin): pass - scragly = MockScragly(spec=object) + scragly = MockScragly() scragly.id = 10 - eevee = MockScragly(spec=object) + eevee = MockScragly() eevee.id = 10 - python = MockScragly(spec=object) + python = MockScragly() python.id = 20 self.assertTrue(scragly == eevee) @@ -281,14 +246,14 @@ class MockObjectTests(unittest.TestCase): def test_hashable_mixin_uses_id_for_nonequality_comparison(self): """Test if the HashableMixing uses the id attribute for hashing.""" - class MockScragly(unittest.mock.Mock, helpers.HashableMixin): + class MockScragly(helpers.HashableMixin): pass - scragly = MockScragly(spec=object) + scragly = MockScragly() scragly.id = 10 - eevee = MockScragly(spec=object) + eevee = MockScragly() eevee.id = 10 - python = MockScragly(spec=object) + python = MockScragly() python.id = 20 self.assertTrue(scragly != python) @@ -298,7 +263,7 @@ class MockObjectTests(unittest.TestCase): """Test if the MagicMock subclasses that implement the HashableMixin use id for hash.""" for mock in self.hashable_mocks: with self.subTest(mock_class=mock): - instance = helpers.MockRole(role_id=100) + instance = helpers.MockRole(id=100) self.assertEqual(hash(instance), instance.id) def test_mock_class_with_hashable_mixin_uses_id_for_equality(self): @@ -396,11 +361,11 @@ class MockObjectTests(unittest.TestCase): @unittest.mock.patch("tests.helpers.CustomMockMixin._extract_coroutine_methods_from_spec_instance") def test_custom_mock_mixin_init_with_spec(self, extract_method_mock): """Test if CustomMockMixin correctly passes on spec/kwargs and calls the extraction method.""" - spec = "pydis" + spec_set = "pydis" - helpers.CustomMockMixin(spec=spec) + helpers.CustomMockMixin(spec_set=spec_set) - extract_method_mock.assert_called_once_with(spec) + extract_method_mock.assert_called_once_with(spec_set) @unittest.mock.patch("builtins.super", new=unittest.mock.MagicMock()) @unittest.mock.patch("tests.helpers.CustomMockMixin._extract_coroutine_methods_from_spec_instance") -- cgit v1.2.3 From 7f4829e9fab007690d48188f499bfcc1a7baa437 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Wed, 13 Nov 2019 17:29:09 +0100 Subject: Prevent await warnings for MockBot's create_task Previously, the coroutine object passed to `MockBot.loop.create_task` would trigger a `RuntimeWarning` for not being awaited as we do not actually create a task for it. To prevent these warnings, coroutine objects passed will now automatically be closed. --- tests/helpers.py | 8 +++++++- tests/test_helpers.py | 12 ++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/helpers.py b/tests/helpers.py index 35f2c288c..8a14aeef4 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -227,7 +227,8 @@ class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin if roles: self.roles.extend(roles) - self.mention = f"@{self.name}" + if 'mention' not in kwargs: + self.mention = f"@{self.name}" # Create a Bot instance to get a realistic MagicMock of `discord.ext.commands.Bot` @@ -251,6 +252,11 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): # is technically incorrect, since it's a regular def.) self.wait_for = AsyncMock() + # Since calling `create_task` on our MockBot does not actually schedule the coroutine object + # as a task in the asyncio loop, this `side_effect` calls `close()` on the coroutine object + # to prevent "has not been awaited"-warnings. + self.loop.create_task.side_effect = lambda coroutine: coroutine.close() + # Create a TextChannel instance to get a realistic MagicMock of `discord.TextChannel` channel_data = { diff --git a/tests/test_helpers.py b/tests/test_helpers.py index e879ef97a..7894e104a 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -202,6 +202,18 @@ class DiscordMocksTests(unittest.TestCase): mock = mock_type(mention=mention) self.assertEqual(mock.mention, mention) + def test_create_test_on_mock_bot_closes_passed_coroutine(self): + """`bot.loop.create_task` should close the passed coroutine object to prevent warnings.""" + async def dementati(): + """Dummy coroutine for testing purposes.""" + + coroutine_object = dementati() + + bot = helpers.MockBot() + bot.loop.create_task(coroutine_object) + with self.assertRaises(RuntimeError, msg="cannot reuse already awaited coroutine"): + asyncio.run(coroutine_object) + class MockObjectTests(unittest.TestCase): """Tests the mock objects and mixins we've defined.""" -- cgit v1.2.3