From 13866a11f764dec27bc5a600781caa17986e9957 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 16 Nov 2020 01:48:38 +0300 Subject: Adds VoiceChannel Mute Adds an optional channel parameter to silence and unsilence commands, and adds ability to silence voice channels. TODO: New Tests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 104293d8e..bac933115 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -123,7 +123,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): guild.get_role.side_effect = lambda id_: Mock(id=id_) await self.cog._async_init() - self.assertEqual(self.cog._verified_role.id, Roles.verified) + self.assertEqual(self.cog._verified_msg_role.id, Roles.verified) @autospec(silence, "SilenceNotifier", pass_mocks=False) async def test_async_init_got_channels(self): @@ -277,7 +277,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=was_silenced): with self.subTest(was_silenced=was_silenced, message=message, duration=duration): await self.cog.silence.callback(self.cog, ctx, duration) - ctx.send.assert_called_once_with(message) + ctx.channel.send.assert_called_once_with(message) async def test_skipped_already_silenced(self): """Permissions were not set and `False` was returned for an already silenced channel.""" @@ -302,7 +302,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.assertFalse(self.overwrite.send_messages) self.assertFalse(self.overwrite.add_reactions) self.channel.set_permissions.assert_awaited_once_with( - self.cog._verified_role, + self.cog._verified_msg_role, overwrite=self.overwrite ) @@ -372,7 +372,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): args = (300, ctx.channel.id, ctx.invoke.return_value) self.cog.scheduler.schedule_later.assert_called_once_with(*args) - ctx.invoke.assert_called_once_with(self.cog.unsilence) + ctx.invoke.assert_called_once_with(self.cog.unsilence, channel=ctx.channel) async def test_permanent_not_scheduled(self): """A task was not scheduled for a permanent silence.""" @@ -435,7 +435,7 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): """Channel's `send_message` and `add_reactions` overwrites were restored.""" await self.cog._unsilence(self.channel) self.channel.set_permissions.assert_awaited_once_with( - self.cog._verified_role, + self.cog._verified_msg_role, overwrite=self.overwrite, ) @@ -449,7 +449,7 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._unsilence(self.channel) self.channel.set_permissions.assert_awaited_once_with( - self.cog._verified_role, + self.cog._verified_msg_role, overwrite=self.overwrite, ) -- cgit v1.2.3 From 81f923ed784cf09e3b7e90e57a9ad0111099619c Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 23 Nov 2020 01:24:15 +0300 Subject: Write AnyChannelConverter Tests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/test_converters.py | 48 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) (limited to 'tests') diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py index c42111f3f..6bea71977 100644 --- a/tests/bot/test_converters.py +++ b/tests/bot/test_converters.py @@ -7,6 +7,7 @@ from dateutil.relativedelta import relativedelta from discord.ext.commands import BadArgument from bot.converters import ( + AnyChannelConverter, Duration, HushDurationConverter, ISODateTime, @@ -24,6 +25,18 @@ class ConverterTests(unittest.IsolatedAsyncioTestCase): cls.context = MagicMock cls.context.author = 'bob' + cls.context.guild = MagicMock + cls.context.guild.channels = [] + + for i in range(10): + channel = MagicMock() + channel.name = f"test-{i}" + channel.id = str(i) * 18 + + cls.context.guild.channels.append(channel) + if i > 7: + cls.context.guild.channels.append(channel) + cls.fixed_utc_now = datetime.datetime.fromisoformat('2019-01-01T00:00:00') async def test_tag_content_converter_for_valid(self): @@ -312,3 +325,38 @@ class ConverterTests(unittest.IsolatedAsyncioTestCase): with self.subTest(invalid_minutes_string=invalid_minutes_string, exception_message=exception_message): with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): await converter.convert(self.context, invalid_minutes_string) + + async def test_any_channel_converter_for_valid(self): + """AnyChannelConverter returns correct channel from input.""" + test_values = ( + ("test-0", self.context.guild.channels[0]), + ("#test-0", self.context.guild.channels[0]), + (" #tEsT-0 ", self.context.guild.channels[0]), + (f"<#{'0' * 18}>", self.context.guild.channels[0]), + (f"{'0' * 18}", self.context.guild.channels[0]), + ("test-5", self.context.guild.channels[5]), + ("#test-5", self.context.guild.channels[5]), + (f"<#{'5' * 18}>", self.context.guild.channels[5]), + (f"{'5' * 18}", self.context.guild.channels[5]), + ) + + converter = AnyChannelConverter() + for input_string, expected_channel in test_values: + with self.subTest(input_string=input_string, expected_channel=expected_channel): + converted = await converter.convert(self.context, input_string) + self.assertEqual(expected_channel, converted) + + async def test_any_channel_converter_for_invalid(self): + """AnyChannelConverter raises BadArgument for invalid channels.""" + test_values = ( + ("#test-8", "The provided argument returned too many matches (2)."), + ("#test-9", "The provided argument returned too many matches (2)."), + ("#random-name", "#random-name returned no matches."), + ("test-10", "test-10 returned no matches.") + ) + + converter = AnyChannelConverter() + for invalid_input, exception_message in test_values: + with self.subTest(invalid_input=invalid_input, exception_message=exception_message): + with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): + await converter.convert(self.context, invalid_input) -- cgit v1.2.3 From 483114bc29da713870346c0e4b88d008f6281185 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 23 Nov 2020 14:24:27 +0300 Subject: General Silence Class Tests Adds tests for helper functions in the silence cog. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 27 +++++---- tests/bot/exts/moderation/test_silence.py | 91 ++++++++++++++++++++++++++++++- tests/helpers.py | 25 ++++++++- 3 files changed, 126 insertions(+), 17 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 2d928182a..2aebee9d7 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -104,6 +104,20 @@ class Silence(commands.Cog): self.notifier = SilenceNotifier(self.bot.get_channel(Channels.mod_log)) await self._reschedule() + async def _get_related_text_channel(self, channel: VoiceChannel) -> Optional[TextChannel]: + """Returns the text channel related to a voice channel.""" + # TODO: Figure out a dynamic way of doing this + channels = { + "off-topic": Channels.voice_chat, + "code/help 1": Channels.code_help_voice, + "code/help 2": Channels.code_help_voice, + "admin": Channels.admins_voice, + "staff": Channels.staff_voice + } + for name in channels.keys(): + if name in channel.name.lower(): + return self.bot.get_channel(channels[name]) + async def send_message(self, message: str, source_channel: TextChannel, target_channel: Union[TextChannel, VoiceChannel], alert_target: bool = False, duration: HushDurationConverter = 0) -> None: @@ -116,18 +130,7 @@ class Silence(commands.Cog): voice_chat = None if isinstance(target_channel, VoiceChannel): # Send to relevant channel - # TODO: Figure out a dynamic way of doing this - channels = { - "offtopic": Channels.voice_chat, - "code/help 1": Channels.code_help_voice, - "code/help 2": Channels.code_help_voice, - "admin": Channels.admins_voice, - "staff": Channels.staff_voice - } - for name in channels.keys(): - if name in target_channel.name.lower(): - voice_chat = self.bot.get_channel(channels[name]) - break + voice_chat = await self._get_related_text_channel(target_channel) if alert_target and source_channel != target_channel: if isinstance(target_channel, VoiceChannel): diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index bac933115..577725071 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -7,9 +7,9 @@ from unittest.mock import Mock from async_rediscache import RedisSession from discord import PermissionOverwrite -from bot.constants import Channels, Guild, Roles +from bot.constants import Channels, Emojis, Guild, Roles from bot.exts.moderation import silence -from tests.helpers import MockBot, MockContext, MockTextChannel, autospec +from tests.helpers import MockBot, MockContext, MockTextChannel, MockVoiceChannel, autospec redis_session = None redis_loop = asyncio.get_event_loop() @@ -168,6 +168,93 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): role_check.assert_called_once_with(*(1, 2, 3)) role_check.return_value.predicate.assert_awaited_once_with(ctx) + @mock.patch.object(silence.Silence, "_get_related_text_channel") + async def test_send_message(self, mock_get_related_text_channel): + """Test the send function reports to the correct channels.""" + text_channel_1 = MockTextChannel() + text_channel_2 = MockTextChannel() + + voice_channel = MockVoiceChannel() + voice_channel.name = "General/Offtopic" + voice_channel.mention = f"#{voice_channel.name}" + + mock_get_related_text_channel.return_value = text_channel_2 + + def reset(): + text_channel_1.reset_mock() + text_channel_2.reset_mock() + voice_channel.reset_mock() + + with self.subTest("Basic One Channel Test"): + await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, False) + text_channel_1.send.assert_called_once_with("Text basic message.") + text_channel_2.send.assert_not_called() + + reset() + with self.subTest("Basic Two Channel Test"): + await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, True) + text_channel_1.send.assert_called_once_with("Text basic message.") + text_channel_2.send.assert_called_once_with("Text basic message.") + + reset() + with self.subTest("Replacement One Channel Test"): + await self.cog.send_message("The following should be replaced: current", + text_channel_1, text_channel_2, False) + text_channel_1.send.assert_called_once_with(f"The following should be replaced: {text_channel_1.mention}") + text_channel_2.send.assert_not_called() + + reset() + with self.subTest("Replacement Two Channel Test"): + await self.cog.send_message("The following should be replaced: current", + text_channel_1, text_channel_2, True) + text_channel_1.send.assert_called_once_with(f"The following should be replaced: {text_channel_1.mention}") + text_channel_2.send.assert_called_once_with("The following should be replaced: current") + + reset() + with self.subTest("Replace Duration"): + await self.cog.send_message(f"{Emojis.check_mark} The following should be replaced: {{duration}}", + text_channel_1, text_channel_2, False) + text_channel_1.send.assert_called_once_with(f"{Emojis.check_mark} The following should be replaced: 0") + text_channel_2.send.assert_not_called() + + reset() + with self.subTest("Text and Voice"): + await self.cog.send_message("This should show up just here", + text_channel_1, voice_channel, False) + text_channel_1.send.assert_called_once_with("This should show up just here") + + reset() + with self.subTest("Text and Voice"): + await self.cog.send_message("This should show up as current", + text_channel_1, voice_channel, True) + text_channel_1.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") + text_channel_2.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") + + reset() + with self.subTest("Text and Voice Same Invocation"): + await self.cog.send_message("This should show up as current", + text_channel_2, voice_channel, True) + text_channel_2.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") + + async def test_get_related_text_channel(self): + voice_channel = MockVoiceChannel() + + tests = ( + ("Off-Topic/General", Channels.voice_chat), + ("code/help 1", Channels.code_help_voice), + ("Staff", Channels.staff_voice), + ("ADMIN", Channels.admins_voice), + ("not in the channel list", None) + ) + + with mock.patch.object(self.cog.bot, "get_channel", lambda x: x): + for (name, channel_id) in tests: + voice_channel.name = name + voice_channel.id = channel_id + + result_id = await self.cog._get_related_text_channel(voice_channel) + self.assertEqual(result_id, channel_id) + @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) class RescheduleTests(unittest.IsolatedAsyncioTestCase): diff --git a/tests/helpers.py b/tests/helpers.py index 870f66197..5628ca31f 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -16,7 +16,6 @@ from bot.async_stats import AsyncStatsClient from bot.bot import Bot from tests._autospec import autospec # noqa: F401 other modules import it via this module - for logger in logging.Logger.manager.loggerDict.values(): # Set all loggers to CRITICAL by default to prevent screen clutter during testing @@ -320,7 +319,10 @@ channel_data = { } state = unittest.mock.MagicMock() guild = unittest.mock.MagicMock() -channel_instance = discord.TextChannel(state=state, guild=guild, data=channel_data) +text_channel_instance = discord.TextChannel(state=state, guild=guild, data=channel_data) + +channel_data["type"] = "VoiceChannel" +voice_channel_instance = discord.VoiceChannel(state=state, guild=guild, data=channel_data) class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): @@ -330,7 +332,24 @@ class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): Instances of this class will follow the specifications of `discord.TextChannel` instances. For more information, see the `MockGuild` docstring. """ - spec_set = channel_instance + spec_set = text_channel_instance + + def __init__(self, **kwargs) -> None: + default_kwargs = {'id': next(self.discord_id), 'name': 'channel', 'guild': MockGuild()} + super().__init__(**collections.ChainMap(kwargs, default_kwargs)) + + if 'mention' not in kwargs: + self.mention = f"#{self.name}" + + +class MockVoiceChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): + """ + A MagicMock subclass to mock VoiceChannel objects. + + Instances of this class will follow the specifications of `discord.VoiceChannel` instances. For + more information, see the `MockGuild` docstring. + """ + spec_set = voice_channel_instance def __init__(self, **kwargs) -> None: default_kwargs = {'id': next(self.discord_id), 'name': 'channel', 'guild': MockGuild()} -- cgit v1.2.3 From 00a26b95bc248e3e7b13263df8c6c0d46ebbb2ff Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 23 Nov 2020 23:09:13 +0300 Subject: Adds Silence & Unsilence UnitTests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 212 +++++++++++++++++++++++++++++- 1 file changed, 211 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 577725071..1d05ee357 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -9,7 +9,7 @@ from discord import PermissionOverwrite from bot.constants import Channels, Emojis, Guild, Roles from bot.exts.moderation import silence -from tests.helpers import MockBot, MockContext, MockTextChannel, MockVoiceChannel, autospec +from tests.helpers import MockBot, MockContext, MockGuild, MockMember, MockTextChannel, MockVoiceChannel, autospec redis_session = None redis_loop = asyncio.get_event_loop() @@ -237,6 +237,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): text_channel_2.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") async def test_get_related_text_channel(self): + """Tests the helper function that connects voice to text channels.""" voice_channel = MockVoiceChannel() tests = ( @@ -255,6 +256,72 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): result_id = await self.cog._get_related_text_channel(voice_channel) self.assertEqual(result_id, channel_id) + async def test_force_voice_sync(self): + """Tests the _force_voice_sync helper function.""" + await self.cog._async_init() + + afk_channel = MockVoiceChannel() + channel = MockVoiceChannel(guild=MockGuild(afk_channel=afk_channel)) + + members = [] + for _ in range(10): + members.append(MockMember()) + + channel.members = members + test_cases = ( + (members[0], False, "Muting member."), + (members[0], True, "Kicking voice channel member."), + (None, False, "Muting member."), + (None, True, "Kicking voice channel member."), + ) + + for member, kick, reason in test_cases: + with self.subTest(members=member, kick=kick, reason=reason): + await self.cog._force_voice_sync(channel, member, kick) + + for single_member in channel.members if member is None else [member]: + if kick: + single_member.move_to.assert_called_once_with(None, reason=reason) + else: + self.assertEqual(single_member.move_to.call_count, 2) + single_member.move_to.assert_has_calls([ + mock.call(afk_channel, reason=reason), + mock.call(channel, reason=reason) + ], any_order=False) + + single_member.reset_mock() + + async def test_force_voice_sync_staff(self): + """Tests to ensure _force_voice_sync does not kick staff members.""" + await self.cog._async_init() + member = MockMember(roles=[self.cog._helper_role]) + + await self.cog._force_voice_sync(MockVoiceChannel(), member) + member.move_to.assert_not_called() + + async def test_force_voice_sync_no_channel(self): + """Test to ensure _force_voice_sync can create its own voice channel if one is not available.""" + await self.cog._async_init() + + member = MockMember() + channel = MockVoiceChannel(guild=MockGuild(afk_channel=None)) + + new_channel = MockVoiceChannel(delete=Mock()) + channel.guild.create_voice_channel.return_value = new_channel + + with mock.patch.object(self.cog.scheduler, "schedule_later") as scheduler: + await self.cog._force_voice_sync(channel, member) + + # Check channel creation + overwrites = { + channel.guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) + } + channel.guild.create_voice_channel.assert_called_once_with("mute-temp", overwrites=overwrites) + + # Check bot queued deletion + new_channel.delete.assert_called_once_with(reason="Deleting temp mute channel.") + scheduler.assert_called_once_with(30, new_channel.id, new_channel.delete()) + @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) class RescheduleTests(unittest.IsolatedAsyncioTestCase): @@ -366,6 +433,31 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog.silence.callback(self.cog, ctx, duration) ctx.channel.send.assert_called_once_with(message) + async def test_sent_to_correct_channel(self): + """Test function sends messages to the correct channels.""" + text_channel = MockTextChannel() + ctx = MockContext() + + test_cases = ( + (None, silence.MSG_SILENCE_SUCCESS.format(duration=10)), + (text_channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), + (ctx.channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), + ) + + for target, message in test_cases: + with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): + with self.subTest(target_channel=target, message=message): + await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) + if ctx.channel == target or target is None: + ctx.channel.send.assert_called_once_with(message) + else: + ctx.channel.send.assert_called_once_with(message.replace("current", text_channel.mention)) + target.send.assert_called_once_with(message) + + ctx.channel.send.reset_mock() + if target is not None: + target.send.reset_mock() + async def test_skipped_already_silenced(self): """Permissions were not set and `False` was returned for an already silenced channel.""" subtests = ( @@ -509,6 +601,40 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog.unsilence.callback(self.cog, ctx) ctx.channel.send.assert_called_once_with(message) + async def test_sent_to_correct_channel(self): + """Test function sends messages to the correct channels.""" + unsilenced_overwrite = PermissionOverwrite(send_messages=True, add_reactions=True) + text_channel = MockTextChannel() + ctx = MockContext() + + test_cases = ( + (None, silence.MSG_UNSILENCE_SUCCESS.format(duration=10)), + (text_channel, silence.MSG_UNSILENCE_SUCCESS.format(duration=10)), + (ctx.channel, silence.MSG_UNSILENCE_SUCCESS.format(duration=10)), + ) + + for target, message in test_cases: + with self.subTest(target_channel=target, message=message): + with mock.patch.object(self.cog, "_unsilence", return_value=True): + # Assign Return + if ctx.channel == target or target is None: + ctx.channel.overwrites_for.return_value = unsilenced_overwrite + else: + target.overwrites_for.return_value = unsilenced_overwrite + + await self.cog.unsilence.callback(self.cog, ctx, channel=target) + + # Check Messages + if ctx.channel == target or target is None: + ctx.channel.send.assert_called_once_with(message) + else: + ctx.channel.send.assert_called_once_with(message.replace("current", text_channel.mention)) + target.send.assert_called_once_with(message) + + ctx.channel.send.reset_mock() + if target is not None: + target.send.reset_mock() + async def test_skipped_already_unsilenced(self): """Permissions were not set and `False` was returned for an already unsilenced channel.""" self.cog.scheduler.__contains__.return_value = False @@ -549,6 +675,10 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._unsilence(self.channel) self.cog._mod_alerts_channel.send.assert_awaited_once() + self.cog._mod_alerts_channel.send.reset_mock() + + await self.cog._unsilence(MockVoiceChannel()) + self.cog._mod_alerts_channel.send.assert_awaited_once() async def test_removed_notifier(self): """Channel was removed from `notifier`.""" @@ -587,3 +717,83 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): del new_overwrite_dict['add_reactions'] self.assertDictEqual(prev_overwrite_dict, new_overwrite_dict) + + async def test_unsilence_helper_fail(self): + """Tests unsilence_wrapper when `_unsilence` fails.""" + ctx = MockContext() + + text_channel = MockTextChannel() + text_role = self.cog.bot.get_guild(Guild.id).get_role(Roles.verified) + + voice_channel = MockVoiceChannel() + voice_role = self.cog.bot.get_guild(Guild.id).get_role(Roles.voice_verified) + + test_cases = ( + (ctx, text_channel, text_role, True, silence.MSG_UNSILENCE_FAIL), + (ctx, text_channel, text_role, False, silence.MSG_UNSILENCE_MANUAL), + (ctx, voice_channel, voice_role, True, silence.MSG_UNSILENCE_FAIL), + (ctx, voice_channel, voice_role, False, silence.MSG_UNSILENCE_MANUAL), + ) + + class PermClass: + """Class to Mock return permissions""" + def __init__(self, value: bool): + self.val = value + + def __getattr__(self, item): + return self.val + + for context, channel, role, permission, message in test_cases: + with self.subTest(channel=channel, message=message): + with mock.patch.object(channel, "overwrites_for", return_value=PermClass(permission)) as overwrites: + with mock.patch.object(self.cog, "send_message") as send_message: + with mock.patch.object(self.cog, "_unsilence", return_value=False): + await self.cog._unsilence_wrapper(channel, context) + + overwrites.assert_called_once_with(role) + send_message.assert_called_once_with(message, ctx.channel, channel) + + async def test_correct_overwrites(self): + """Tests the overwrites returned by the _unsilence_wrapper are correct for voice and text channels.""" + ctx = MockContext() + + text_channel = MockTextChannel() + text_role = self.cog.bot.get_guild(Guild.id).get_role(Roles.verified) + + voice_channel = MockVoiceChannel() + voice_role = self.cog.bot.get_guild(Guild.id).get_role(Roles.voice_verified) + + async def reset(): + await text_channel.set_permissions(text_role, PermissionOverwrite(send_messages=False, add_reactions=False)) + await voice_channel.set_permissions(voice_role, PermissionOverwrite(speak=False, connect=False)) + + text_channel.reset_mock() + voice_channel.reset_mock() + await reset() + + default_text_overwrites = text_channel.overwrites_for(text_role) + default_voice_overwrites = voice_channel.overwrites_for(voice_role) + + test_cases = ( + (ctx, text_channel, text_role, default_text_overwrites, silence.MSG_UNSILENCE_SUCCESS), + (ctx, voice_channel, voice_role, default_voice_overwrites, silence.MSG_UNSILENCE_SUCCESS), + (ctx, ctx.channel, text_role, ctx.channel.overwrites_for(text_role), silence.MSG_UNSILENCE_SUCCESS), + (None, text_channel, text_role, default_text_overwrites, silence.MSG_UNSILENCE_SUCCESS), + ) + + for context, channel, role, overwrites, message in test_cases: + with self.subTest(ctx=context, channel=channel): + with mock.patch.object(self.cog, "send_message") as send_message: + with mock.patch.object(self.cog, "_force_voice_sync"): + await self.cog._unsilence_wrapper(channel, context) + + if context is None: + send_message.assert_called_once_with(message, channel, channel, True) + else: + send_message.assert_called_once_with(message, context.channel, channel, True) + + channel.set_permissions.assert_called_once_with(role, overwrite=overwrites) + if channel != ctx.channel: + ctx.channel.assert_not_called() + + await reset() -- cgit v1.2.3 From dc202302fa9bfa7ab54d80cc486acdd5b278ad6a Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Mon, 23 Nov 2020 23:50:23 +0300 Subject: Finalizes Silence & Unsilence UnitTests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 49 ++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 17 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 1d05ee357..2392e6e59 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -415,9 +415,13 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): asyncio.run(self.cog._async_init()) # Populate instance attributes. - self.channel = MockTextChannel() - self.overwrite = PermissionOverwrite(stream=True, send_messages=True, add_reactions=False) - self.channel.overwrites_for.return_value = self.overwrite + self.text_channel = MockTextChannel() + self.text_overwrite = PermissionOverwrite(stream=True, send_messages=True, add_reactions=False) + self.text_channel.overwrites_for.return_value = self.text_overwrite + + self.voice_channel = MockVoiceChannel() + self.voice_overwrite = PermissionOverwrite(speak=True) + self.voice_channel.overwrites_for.return_value = self.voice_overwrite async def test_sent_correct_message(self): """Appropriate failure/success message was sent by the command.""" @@ -477,19 +481,19 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_silenced_channel(self): """Channel had `send_message` and `add_reactions` permissions revoked for verified role.""" - self.assertTrue(await self.cog._set_silence_overwrites(self.channel)) - self.assertFalse(self.overwrite.send_messages) - self.assertFalse(self.overwrite.add_reactions) - self.channel.set_permissions.assert_awaited_once_with( + self.assertTrue(await self.cog._set_silence_overwrites(self.text_channel)) + self.assertFalse(self.text_overwrite.send_messages) + self.assertFalse(self.text_overwrite.add_reactions) + self.text_channel.set_permissions.assert_awaited_once_with( self.cog._verified_msg_role, - overwrite=self.overwrite + overwrite=self.text_overwrite ) async def test_preserved_other_overwrites(self): """Channel's other unrelated overwrites were not changed.""" - prev_overwrite_dict = dict(self.overwrite) - await self.cog._set_silence_overwrites(self.channel) - new_overwrite_dict = dict(self.overwrite) + prev_overwrite_dict = dict(self.text_overwrite) + await self.cog._set_silence_overwrites(self.text_channel) + new_overwrite_dict = dict(self.text_overwrite) # Remove 'send_messages' & 'add_reactions' keys because they were changed by the method. del prev_overwrite_dict['send_messages'] @@ -520,8 +524,8 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_cached_previous_overwrites(self): """Channel's previous overwrites were cached.""" overwrite_json = '{"send_messages": true, "add_reactions": false}' - await self.cog._set_silence_overwrites(self.channel) - self.cog.previous_overwrites.set.assert_called_once_with(self.channel.id, overwrite_json) + await self.cog._set_silence_overwrites(self.text_channel) + self.cog.previous_overwrites.set.assert_called_once_with(self.text_channel.id, overwrite_json) @autospec(silence, "datetime") async def test_cached_unsilence_time(self, datetime_mock): @@ -531,7 +535,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): timestamp = now_timestamp + duration * 60 datetime_mock.now.return_value = datetime.fromtimestamp(now_timestamp, tz=timezone.utc) - ctx = MockContext(channel=self.channel) + ctx = MockContext(channel=self.text_channel) await self.cog.silence.callback(self.cog, ctx, duration) self.cog.unsilence_timestamps.set.assert_awaited_once_with(ctx.channel.id, timestamp) @@ -539,13 +543,13 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_cached_indefinite_time(self): """A value of -1 was cached for a permanent silence.""" - ctx = MockContext(channel=self.channel) + ctx = MockContext(channel=self.text_channel) await self.cog.silence.callback(self.cog, ctx, None) self.cog.unsilence_timestamps.set.assert_awaited_once_with(ctx.channel.id, -1) async def test_scheduled_task(self): """An unsilence task was scheduled.""" - ctx = MockContext(channel=self.channel, invoke=mock.MagicMock()) + ctx = MockContext(channel=self.text_channel, invoke=mock.MagicMock()) await self.cog.silence.callback(self.cog, ctx, 5) @@ -555,10 +559,21 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_permanent_not_scheduled(self): """A task was not scheduled for a permanent silence.""" - ctx = MockContext(channel=self.channel) + ctx = MockContext(channel=self.text_channel) await self.cog.silence.callback(self.cog, ctx, None) self.cog.scheduler.schedule_later.assert_not_called() + async def test_correct_permission_updates(self): + """Tests if _set_silence_overwrites can correctly get and update permissions.""" + self.assertTrue(await self.cog._set_silence_overwrites(self.text_channel)) + self.assertFalse(self.text_overwrite.send_messages or self.text_overwrite.add_reactions) + + self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel)) + self.assertFalse(self.voice_overwrite.speak) + + self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel, True)) + self.assertFalse(self.voice_overwrite.speak or self.voice_overwrite.connect) + @autospec(silence.Silence, "unsilence_timestamps", pass_mocks=False) class UnsilenceTests(unittest.IsolatedAsyncioTestCase): -- cgit v1.2.3 From b2d88f860d5ca9d08c72be676a7bfdfd5943b417 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 12:20:01 +0300 Subject: Removes AnyChannel Converter Removes the AnyChannel converter in favor of a combination of Text and Voice converters. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/converters.py | 40 ----------------------------------- bot/exts/moderation/silence.py | 6 +++--- tests/bot/test_converters.py | 48 ------------------------------------------ 3 files changed, 3 insertions(+), 91 deletions(-) (limited to 'tests') diff --git a/bot/converters.py b/bot/converters.py index 613be73eb..2e118d476 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -536,46 +536,6 @@ class FetchedUser(UserConverter): raise BadArgument(f"User `{arg}` does not exist") -class AnyChannelConverter(Converter): - """ - Converts to a `discord.Channel` or, raises an error. - - Unlike the default Channel Converter, this converter can handle channels given - in string, id, or mention formatting for both `TextChannel`s and `VoiceChannel`s. - Always returns 1 or fewer channels, errors if more than one match exists. - - It is able to handle the following formats (caveats noted below:) - 1. Convert from ID - Example: 267631170882240512 - 2. Convert from Explicit Mention - Example: #welcome - 3. Convert from ID Mention - Example: <#267631170882240512> - 4. Convert from Unmentioned Name: - Example: welcome - - All the previous conversions are valid for both text and voice channels, but explicit - raw names (#4) do not work for non-unique channels, instead opting for an error. - Explicit mentions (#2) do not work for non-unique voice channels either. - """ - - async def convert(self, ctx: Context, arg: str) -> t.Union[discord.TextChannel, discord.VoiceChannel]: - """Convert the `arg` to a `TextChannel` or `VoiceChannel`.""" - stripped = arg.strip().lstrip("<").lstrip("#").rstrip(">").lower() - - # Filter channels by name and ID - channels = [channel for channel in ctx.guild.channels if stripped in (channel.name.lower(), str(channel.id))] - - if len(channels) == 0: - # Couldn't find a matching channel - log.debug(f"Could not convert `{arg}` to channel, no matches found.") - raise BadArgument(f"{arg} returned no matches.") - - elif len(channels) > 1: - # Couldn't discern the desired channel - log.debug(f"Could not convert `{arg}` to channel, {len(channels)} matches found.") - raise BadArgument(f"The provided argument returned too many matches ({len(channels)}).") - - else: - return channels[0] - - def _snowflake_from_regex(pattern: t.Pattern, arg: str) -> int: """ Extract the snowflake from `arg` using a regex `pattern` and return it as an int. diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 62f3ede73..8ad30f0d9 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -12,7 +12,7 @@ from discord.ext.commands import Context from bot.bot import Bot from bot.constants import Channels, Emojis, Guild, MODERATION_ROLES, Roles -from bot.converters import AnyChannelConverter, HushDurationConverter +from bot.converters import HushDurationConverter from bot.utils.lock import LockedResourceError, lock_arg from bot.utils.scheduling import Scheduler @@ -148,7 +148,7 @@ class Silence(commands.Cog): @lock_arg(LOCK_NAMESPACE, "ctx", attrgetter("channel"), raise_error=True) async def silence( self, ctx: Context, duration: HushDurationConverter = 10, kick: bool = False, - *, channel: Optional[AnyChannelConverter] = None + *, channel: Union[TextChannel, VoiceChannel] = None ) -> None: """ Silence the current channel for `duration` minutes or `forever`. @@ -182,7 +182,7 @@ class Silence(commands.Cog): await self.send_message(MSG_SILENCE_SUCCESS, ctx.channel, channel, True, duration) @commands.command(aliases=("unhush",)) - async def unsilence(self, ctx: Context, *, channel: AnyChannelConverter = None) -> None: + async def unsilence(self, ctx: Context, *, channel: Union[TextChannel, VoiceChannel] = None) -> None: """ Unsilence the given channel if given, else the current one. diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py index 6bea71977..c42111f3f 100644 --- a/tests/bot/test_converters.py +++ b/tests/bot/test_converters.py @@ -7,7 +7,6 @@ from dateutil.relativedelta import relativedelta from discord.ext.commands import BadArgument from bot.converters import ( - AnyChannelConverter, Duration, HushDurationConverter, ISODateTime, @@ -25,18 +24,6 @@ class ConverterTests(unittest.IsolatedAsyncioTestCase): cls.context = MagicMock cls.context.author = 'bob' - cls.context.guild = MagicMock - cls.context.guild.channels = [] - - for i in range(10): - channel = MagicMock() - channel.name = f"test-{i}" - channel.id = str(i) * 18 - - cls.context.guild.channels.append(channel) - if i > 7: - cls.context.guild.channels.append(channel) - cls.fixed_utc_now = datetime.datetime.fromisoformat('2019-01-01T00:00:00') async def test_tag_content_converter_for_valid(self): @@ -325,38 +312,3 @@ class ConverterTests(unittest.IsolatedAsyncioTestCase): with self.subTest(invalid_minutes_string=invalid_minutes_string, exception_message=exception_message): with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): await converter.convert(self.context, invalid_minutes_string) - - async def test_any_channel_converter_for_valid(self): - """AnyChannelConverter returns correct channel from input.""" - test_values = ( - ("test-0", self.context.guild.channels[0]), - ("#test-0", self.context.guild.channels[0]), - (" #tEsT-0 ", self.context.guild.channels[0]), - (f"<#{'0' * 18}>", self.context.guild.channels[0]), - (f"{'0' * 18}", self.context.guild.channels[0]), - ("test-5", self.context.guild.channels[5]), - ("#test-5", self.context.guild.channels[5]), - (f"<#{'5' * 18}>", self.context.guild.channels[5]), - (f"{'5' * 18}", self.context.guild.channels[5]), - ) - - converter = AnyChannelConverter() - for input_string, expected_channel in test_values: - with self.subTest(input_string=input_string, expected_channel=expected_channel): - converted = await converter.convert(self.context, input_string) - self.assertEqual(expected_channel, converted) - - async def test_any_channel_converter_for_invalid(self): - """AnyChannelConverter raises BadArgument for invalid channels.""" - test_values = ( - ("#test-8", "The provided argument returned too many matches (2)."), - ("#test-9", "The provided argument returned too many matches (2)."), - ("#random-name", "#random-name returned no matches."), - ("test-10", "test-10 returned no matches.") - ) - - converter = AnyChannelConverter() - for invalid_input, exception_message in test_values: - with self.subTest(invalid_input=invalid_input, exception_message=exception_message): - with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): - await converter.convert(self.context, invalid_input) -- cgit v1.2.3 From c37e9842602f6e8e7bb2cc0897a6c8e0988f7cff Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 12:29:51 +0300 Subject: Fixes Typo in Doc Co-authored-by: Mark --- tests/bot/exts/moderation/test_silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 2392e6e59..5c6e2d0f1 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -564,7 +564,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.cog.scheduler.schedule_later.assert_not_called() async def test_correct_permission_updates(self): - """Tests if _set_silence_overwrites can correctly get and update permissions.""" + """Tests if _set_silence_overwrites can correctly get and update permissions.""" self.assertTrue(await self.cog._set_silence_overwrites(self.text_channel)) self.assertFalse(self.text_overwrite.send_messages or self.text_overwrite.add_reactions) -- cgit v1.2.3 From 7cb3024e71cd81e9ef29f5f10cb2bc5fe62ad846 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 13:19:51 +0300 Subject: Refractors SendMessage Function Refractors the send message function in silence to make it more understandable and flexible. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 31 ++++++++---------- tests/bot/exts/moderation/test_silence.py | 54 ++++++++++++++++++------------- 2 files changed, 46 insertions(+), 39 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 8ad30f0d9..7bc51ee93 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -26,7 +26,7 @@ MSG_SILENCE_SUCCESS = f"{Emojis.check_mark} silenced current channel for {{durat MSG_UNSILENCE_FAIL = f"{Emojis.cross_mark} current channel was not silenced." MSG_UNSILENCE_MANUAL = ( - f"{Emojis.cross_mark} current channel was not unsilenced because the channel overwrites were " + f"{Emojis.cross_mark} current channel was not unsilenced because the current overwrites were " f"set manually or the cache was prematurely cleared. " f"Please edit the overwrites manually to unsilence." ) @@ -119,30 +119,27 @@ class Silence(commands.Cog): async def send_message( self, message: str, source_channel: TextChannel, target_channel: Union[TextChannel, VoiceChannel], - alert_target: bool = False, duration: int = 0 + alert_target: bool = False ) -> None: """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`.""" - await source_channel.send( - message.replace("current", target_channel.mention if source_channel != target_channel else "current") - .replace("{duration}", str(duration)) - ) - + # Get TextChannel connected to VoiceChannel if channel is of type voice voice_chat = None if isinstance(target_channel, VoiceChannel): - # Send to relevant channel voice_chat = await self._get_related_text_channel(target_channel) - if alert_target and source_channel != target_channel: - if isinstance(target_channel, VoiceChannel): - if voice_chat is None or voice_chat == source_channel: - return + # Reply to invocation channel + source_reply = message + if source_channel != target_channel: + source_reply = source_reply.replace("current channel", target_channel.mention) + await source_channel.send(source_reply) - await voice_chat.send( - message.replace("{duration}", str(duration)).replace("current", target_channel.mention) - ) + # Reply to target channel + if alert_target and source_channel != target_channel and source_channel != voice_chat: + if isinstance(target_channel, VoiceChannel) and (voice_chat is not None or voice_chat != source_channel): + await voice_chat.send(message.replace("current channel", target_channel.mention)) else: - await target_channel.send(message.replace("{duration}", str(duration))) + await target_channel.send(message) @commands.command(aliases=("hush",)) @lock_arg(LOCK_NAMESPACE, "ctx", attrgetter("channel"), raise_error=True) @@ -179,7 +176,7 @@ class Silence(commands.Cog): else: log.info(f"Silenced {channel_info} for {duration} minute(s).") - await self.send_message(MSG_SILENCE_SUCCESS, ctx.channel, channel, True, duration) + await self.send_message(MSG_SILENCE_SUCCESS.format(duration=duration), ctx.channel, channel, True) @commands.command(aliases=("unhush",)) async def unsilence(self, ctx: Context, *, channel: Union[TextChannel, VoiceChannel] = None) -> None: diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 5c6e2d0f1..70678d207 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -7,7 +7,7 @@ from unittest.mock import Mock from async_rediscache import RedisSession from discord import PermissionOverwrite -from bot.constants import Channels, Emojis, Guild, Roles +from bot.constants import Channels, Guild, Roles from bot.exts.moderation import silence from tests.helpers import MockBot, MockContext, MockGuild, MockMember, MockTextChannel, MockVoiceChannel, autospec @@ -198,42 +198,48 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): reset() with self.subTest("Replacement One Channel Test"): - await self.cog.send_message("The following should be replaced: current", - text_channel_1, text_channel_2, False) - text_channel_1.send.assert_called_once_with(f"The following should be replaced: {text_channel_1.mention}") + await self.cog.send_message( + "Current. The following should be replaced: current channel.", text_channel_1, text_channel_2, False + ) + + text_channel_1.send.assert_called_once_with( + f"Current. The following should be replaced: {text_channel_1.mention}." + ) + text_channel_2.send.assert_not_called() reset() with self.subTest("Replacement Two Channel Test"): - await self.cog.send_message("The following should be replaced: current", - text_channel_1, text_channel_2, True) - text_channel_1.send.assert_called_once_with(f"The following should be replaced: {text_channel_1.mention}") - text_channel_2.send.assert_called_once_with("The following should be replaced: current") + await self.cog.send_message( + "Current. The following should be replaced: current channel.", text_channel_1, text_channel_2, True + ) - reset() - with self.subTest("Replace Duration"): - await self.cog.send_message(f"{Emojis.check_mark} The following should be replaced: {{duration}}", - text_channel_1, text_channel_2, False) - text_channel_1.send.assert_called_once_with(f"{Emojis.check_mark} The following should be replaced: 0") - text_channel_2.send.assert_not_called() + text_channel_1.send.assert_called_once_with( + f"Current. The following should be replaced: {text_channel_1.mention}." + ) + + text_channel_2.send.assert_called_once_with("Current. The following should be replaced: current channel.") reset() with self.subTest("Text and Voice"): - await self.cog.send_message("This should show up just here", - text_channel_1, voice_channel, False) + await self.cog.send_message( + "This should show up just here", text_channel_1, voice_channel, False + ) text_channel_1.send.assert_called_once_with("This should show up just here") reset() with self.subTest("Text and Voice"): - await self.cog.send_message("This should show up as current", - text_channel_1, voice_channel, True) + await self.cog.send_message( + "This should show up as current channel", text_channel_1, voice_channel, True + ) text_channel_1.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") text_channel_2.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") reset() with self.subTest("Text and Voice Same Invocation"): - await self.cog.send_message("This should show up as current", - text_channel_2, voice_channel, True) + await self.cog.send_message( + "This should show up as current channel", text_channel_2, voice_channel, True + ) text_channel_2.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") async def test_get_related_text_channel(self): @@ -455,7 +461,9 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): if ctx.channel == target or target is None: ctx.channel.send.assert_called_once_with(message) else: - ctx.channel.send.assert_called_once_with(message.replace("current", text_channel.mention)) + ctx.channel.send.assert_called_once_with( + message.replace("current channel", text_channel.mention) + ) target.send.assert_called_once_with(message) ctx.channel.send.reset_mock() @@ -643,7 +651,9 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): if ctx.channel == target or target is None: ctx.channel.send.assert_called_once_with(message) else: - ctx.channel.send.assert_called_once_with(message.replace("current", text_channel.mention)) + ctx.channel.send.assert_called_once_with( + message.replace("current channel", text_channel.mention) + ) target.send.assert_called_once_with(message) ctx.channel.send.reset_mock() -- cgit v1.2.3 From d2d7db69c4e54120d36146e2334f702f4259ed6b Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 20:36:48 +0300 Subject: Moves VoiceChat Sync Out of Overwrites Function VoiceChat sync only needs to be called when the command is invoked, instead of while updating permissions. Moved call to command function to reflect that, and fixed failing tests. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 11 +++++++---- tests/bot/exts/moderation/test_silence.py | 12 +++++++----- 2 files changed, 14 insertions(+), 9 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 7bc51ee93..64ffaa347 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -134,9 +134,10 @@ class Silence(commands.Cog): await source_channel.send(source_reply) # Reply to target channel - if alert_target and source_channel != target_channel and source_channel != voice_chat: - if isinstance(target_channel, VoiceChannel) and (voice_chat is not None or voice_chat != source_channel): - await voice_chat.send(message.replace("current channel", target_channel.mention)) + if alert_target and source_channel not in [target_channel, voice_chat]: + if isinstance(target_channel, VoiceChannel): + if voice_chat is not None: + await voice_chat.send(message.replace("current channel", target_channel.mention)) else: await target_channel.send(message) @@ -167,6 +168,9 @@ class Silence(commands.Cog): await self.send_message(MSG_SILENCE_FAIL, ctx.channel, channel) return + if isinstance(channel, VoiceChannel): + await self._force_voice_sync(channel, kick=kick) + await self._schedule_unsilence(ctx, channel, duration) if duration is None: @@ -244,7 +248,6 @@ class Silence(commands.Cog): overwrite.update(connect=False) await channel.set_permissions(self._verified_voice_role, overwrite=overwrite) - await self._force_voice_sync(channel, kick=kick) await self.previous_overwrites.set(channel.id, json.dumps(prev_overwrites)) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 70678d207..aab607392 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -446,11 +446,13 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_sent_to_correct_channel(self): """Test function sends messages to the correct channels.""" text_channel = MockTextChannel() + voice_channel = MockVoiceChannel() ctx = MockContext() test_cases = ( (None, silence.MSG_SILENCE_SUCCESS.format(duration=10)), (text_channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), + (voice_channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), (ctx.channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), ) @@ -460,14 +462,14 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) if ctx.channel == target or target is None: ctx.channel.send.assert_called_once_with(message) + else: - ctx.channel.send.assert_called_once_with( - message.replace("current channel", text_channel.mention) - ) - target.send.assert_called_once_with(message) + ctx.channel.send.assert_called_once_with(message.replace("current channel", target.mention)) + if isinstance(target, MockTextChannel): + target.send.assert_called_once_with(message) ctx.channel.send.reset_mock() - if target is not None: + if target is not None and isinstance(target, MockTextChannel): target.send.reset_mock() async def test_skipped_already_silenced(self): -- cgit v1.2.3 From 675d7504977acafdb73d6a51e91228180a7c02a2 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 24 Nov 2020 20:59:43 +0300 Subject: Fixes Typo in Silence Tests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index aab607392..8f3c1cb8b 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -821,6 +821,6 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): channel.set_permissions.assert_called_once_with(role, overwrite=overwrites) if channel != ctx.channel: - ctx.channel.assert_not_called() + ctx.channel.send.assert_not_called() await reset() -- cgit v1.2.3 From 3ea66f3a61720258e0dda44fc59e547692375280 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sat, 28 Nov 2020 20:13:54 +0300 Subject: Clarifies Constants Use in Silence Changes all usages of bot.constant to use dotted path to remove confusion and namespace collision. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 46 ++++++++++++++++--------------- tests/bot/exts/moderation/test_silence.py | 2 +- 2 files changed, 25 insertions(+), 23 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 8c71d422d..d1db0da9b 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -5,12 +5,12 @@ from datetime import datetime, timedelta, timezone from typing import Optional, Union from async_rediscache import RedisCache -from discord import Member, PermissionOverwrite, TextChannel, VoiceChannel +from discord import Guild, PermissionOverwrite, TextChannel, VoiceChannel from discord.ext import commands, tasks from discord.ext.commands import Context +from bot import constants from bot.bot import Bot -from bot.constants import Channels, Emojis, Guild, MODERATION_ROLES, Roles from bot.converters import HushDurationConverter from bot.utils.lock import LockedResourceError, lock, lock_arg from bot.utils.scheduling import Scheduler @@ -19,17 +19,17 @@ log = logging.getLogger(__name__) LOCK_NAMESPACE = "silence" -MSG_SILENCE_FAIL = f"{Emojis.cross_mark} current channel is already silenced." -MSG_SILENCE_PERMANENT = f"{Emojis.check_mark} silenced current channel indefinitely." -MSG_SILENCE_SUCCESS = f"{Emojis.check_mark} silenced current channel for {{duration}} minute(s)." +MSG_SILENCE_FAIL = f"{constants.Emojis.cross_mark} current channel is already silenced." +MSG_SILENCE_PERMANENT = f"{constants.Emojis.check_mark} silenced current channel indefinitely." +MSG_SILENCE_SUCCESS = f"{constants.Emojis.check_mark} silenced current channel for {{duration}} minute(s)." -MSG_UNSILENCE_FAIL = f"{Emojis.cross_mark} current channel was not silenced." +MSG_UNSILENCE_FAIL = f"{constants.Emojis.cross_mark} current channel was not silenced." MSG_UNSILENCE_MANUAL = ( - f"{Emojis.cross_mark} current channel was not unsilenced because the current overwrites were " + f"{constants.Emojis.cross_mark} current channel was not unsilenced because the current overwrites were " f"set manually or the cache was prematurely cleared. " f"Please edit the overwrites manually to unsilence." ) -MSG_UNSILENCE_SUCCESS = f"{Emojis.check_mark} unsilenced current channel." +MSG_UNSILENCE_SUCCESS = f"{constants.Emojis.check_mark} unsilenced current channel." class SilenceNotifier(tasks.Loop): @@ -67,7 +67,9 @@ class SilenceNotifier(tasks.Loop): f"{channel.mention} for {(self._current_loop-start)//60} min" for channel, start in self._silenced_channels.items() ) - await self._alert_channel.send(f"<@&{Roles.moderators}> currently silenced channels: {channels_text}") + await self._alert_channel.send( + f"<@&{constants.Roles.moderators}> currently silenced channels: {channels_text}" + ) class Silence(commands.Cog): @@ -91,26 +93,26 @@ class Silence(commands.Cog): """Set instance attributes once the guild is available and reschedule unsilences.""" await self.bot.wait_until_guild_available() - guild = self.bot.get_guild(Guild.id) + guild = self.bot.get_guild(constants.Guild.id) - self._verified_msg_role = guild.get_role(Roles.verified) - self._verified_voice_role = guild.get_role(Roles.voice_verified) - self._helper_role = guild.get_role(Roles.helpers) + self._verified_msg_role = guild.get_role(constants.Roles.verified) + self._verified_voice_role = guild.get_role(constants.Roles.voice_verified) + self._helper_role = guild.get_role(constants.Roles.helpers) - self._mod_alerts_channel = self.bot.get_channel(Channels.mod_alerts) + self._mod_alerts_channel = self.bot.get_channel(constants.Channels.mod_alerts) - self.notifier = SilenceNotifier(self.bot.get_channel(Channels.mod_log)) + self.notifier = SilenceNotifier(self.bot.get_channel(constants.Channels.mod_log)) await self._reschedule() async def _get_related_text_channel(self, channel: VoiceChannel) -> Optional[TextChannel]: """Returns the text channel related to a voice channel.""" # TODO: Figure out a dynamic way of doing this channels = { - "off-topic": Channels.voice_chat, - "code/help 1": Channels.code_help_voice, - "code/help 2": Channels.code_help_voice_2, - "admin": Channels.admins_voice, - "staff": Channels.staff_voice + "off-topic": constants.Channels.voice_chat, + "code/help 1": constants.Channels.code_help_voice, + "code/help 2": constants.Channels.code_help_voice_2, + "admin": constants.Channels.admins_voice, + "staff": constants.Channels.staff_voice } for name in channels.keys(): if name in channel.name.lower(): @@ -360,7 +362,7 @@ class Silence(commands.Cog): # Alert Admin team if old overwrites were not available if prev_overwrites is None: await self._mod_alerts_channel.send( - f"<@&{Roles.admins}> Restored overwrites with default values after unsilencing " + f"<@&{constants.Roles.admins}> Restored overwrites with default values after unsilencing " f"{channel.mention}. Please check that the {permissions} " f"overwrites for {role.mention} are at their desired values." ) @@ -402,7 +404,7 @@ class Silence(commands.Cog): # This cannot be static (must have a __func__ attribute). async def cog_check(self, ctx: Context) -> bool: """Only allow moderators to invoke the commands in this cog.""" - return await commands.has_any_role(*MODERATION_ROLES).predicate(ctx) + return await commands.has_any_role(*constants.MODERATION_ROLES).predicate(ctx) def setup(bot: Bot) -> None: diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 8f3c1cb8b..bff2888b9 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -158,7 +158,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): self.assertTrue(self.cog._init_task.cancelled()) @autospec("discord.ext.commands", "has_any_role") - @mock.patch.object(silence, "MODERATION_ROLES", new=(1, 2, 3)) + @mock.patch.object(silence.constants, "MODERATION_ROLES", new=(1, 2, 3)) async def test_cog_check(self, role_check): """Role check was called with `MODERATION_ROLES`""" ctx = MockContext() -- cgit v1.2.3 From a6c8a9aca63f7d40d6c5701a08626da198c1d54a Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sat, 28 Nov 2020 20:14:48 +0300 Subject: Refractors Voice Sync Helper Refractors the voice sync helper function into two different functions, one for each purpose. Moves the afk_channel get/creation code to its own function. Updates tests. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 88 +++++++++++---------- tests/bot/exts/moderation/test_silence.py | 124 ++++++++++++++++++------------ 2 files changed, 121 insertions(+), 91 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index d1db0da9b..9b3725326 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -174,7 +174,10 @@ class Silence(commands.Cog): return if isinstance(channel, VoiceChannel): - await self._force_voice_sync(channel, kick=kick) + if kick: + await self._kick_voice_members(channel) + else: + await self._force_voice_sync(channel) await self._schedule_unsilence(ctx, channel, duration) @@ -252,56 +255,57 @@ class Silence(commands.Cog): return True - async def _force_voice_sync( - self, channel: VoiceChannel, member: Optional[Member] = None, kick: bool = False - ) -> None: - """ - Move all non-staff members from `channel` to a temporary channel and back to force toggle role mute. - - If `member` is passed, the mute only occurs to that member. - Permission modification has to happen before this function. - - If `kick_all` is True, members will not be added back to the voice channel. - """ - # Handle member picking logic - if member is not None: - members = [member] - else: - members = channel.members - - # Handle kick logic - if kick: - for member in members: - await member.move_to(None, reason="Kicking voice channel member.") + @staticmethod + async def _get_afk_channel(guild: Guild) -> VoiceChannel: + """Get a guild's AFK channel, or create one if it does not exist.""" + afk_channel = guild.afk_channel - log.debug(f"Kicked all members from #{channel.name} ({channel.id}).") - return - - # Obtain temporary channel - afk_channel = channel.guild.afk_channel if afk_channel is None: overwrites = { - channel.guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) + guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) } - afk_channel = await channel.guild.create_voice_channel("mute-temp", overwrites=overwrites) + afk_channel = await guild.create_voice_channel("mute-temp", overwrites=overwrites) log.info(f"Failed to get afk-channel, created temporary channel #{afk_channel} ({afk_channel.id})") - # Schedule channel deletion in case function errors out - self.scheduler.schedule_later( - 30, afk_channel.id, afk_channel.delete(reason="Deleting temp mute channel.") - ) + return afk_channel - # Move all members to temporary channel and back - for member in members: - # Skip staff - if self._helper_role in member.roles: - continue + async def _kick_voice_members(self, channel: VoiceChannel) -> None: + """Remove all non-staff members from a voice channel.""" + log.debug(f"Removing all non staff members from #{channel.name} ({channel.id}).") + + for member in channel.members: + if self._helper_role not in member.roles: + await member.move_to(None, reason="Kicking member from voice channel.") - await member.move_to(afk_channel, reason="Muting member.") - log.debug(f"Moved {member.name} to afk channel.") + log.debug("Removed all members.") - await member.move_to(channel, reason="Muting member.") - log.debug(f"Moved {member.name} to original voice channel.") + async def _force_voice_sync(self, channel: VoiceChannel) -> None: + """ + Move all non-staff members from `channel` to a temporary channel and back to force toggle role mute. + + Permission modification has to happen before this function. + """ + # Obtain temporary channel + delete_channel = channel.guild.afk_channel is None + afk_channel = await self._get_afk_channel(channel.guild) + + try: + # Move all members to temporary channel and back + for member in channel.members: + # Skip staff + if self._helper_role in member.roles: + continue + + await member.move_to(afk_channel, reason="Muting VC member.") + log.debug(f"Moved {member.name} to afk channel.") + + await member.move_to(channel, reason="Muting VC member.") + log.debug(f"Moved {member.name} to original voice channel.") + + finally: + # Delete VC channel if it was created. + if delete_channel: + await afk_channel.delete(reason="Deleting temp mute channel.") async def _schedule_unsilence( self, ctx: Context, channel: Union[TextChannel, VoiceChannel], duration: Optional[int] diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index bff2888b9..9fb3e404a 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -2,7 +2,7 @@ import asyncio import unittest from datetime import datetime, timezone from unittest import mock -from unittest.mock import Mock +from unittest.mock import AsyncMock, Mock from async_rediscache import RedisSession from discord import PermissionOverwrite @@ -266,67 +266,69 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): """Tests the _force_voice_sync helper function.""" await self.cog._async_init() - afk_channel = MockVoiceChannel() - channel = MockVoiceChannel(guild=MockGuild(afk_channel=afk_channel)) - members = [] for _ in range(10): members.append(MockMember()) - channel.members = members - test_cases = ( - (members[0], False, "Muting member."), - (members[0], True, "Kicking voice channel member."), - (None, False, "Muting member."), - (None, True, "Kicking voice channel member."), - ) - - for member, kick, reason in test_cases: - with self.subTest(members=member, kick=kick, reason=reason): - await self.cog._force_voice_sync(channel, member, kick) - - for single_member in channel.members if member is None else [member]: - if kick: - single_member.move_to.assert_called_once_with(None, reason=reason) - else: - self.assertEqual(single_member.move_to.call_count, 2) - single_member.move_to.assert_has_calls([ - mock.call(afk_channel, reason=reason), - mock.call(channel, reason=reason) - ], any_order=False) + afk_channel = MockVoiceChannel() + channel = MockVoiceChannel(guild=MockGuild(afk_channel=afk_channel), members=members) - single_member.reset_mock() + await self.cog._force_voice_sync(channel) + for member in members: + self.assertEqual(member.move_to.call_count, 2) + member.move_to.assert_has_calls([ + mock.call(afk_channel, reason="Muting VC member."), + mock.call(channel, reason="Muting VC member.") + ], any_order=False) async def test_force_voice_sync_staff(self): """Tests to ensure _force_voice_sync does not kick staff members.""" await self.cog._async_init() member = MockMember(roles=[self.cog._helper_role]) - await self.cog._force_voice_sync(MockVoiceChannel(), member) + await self.cog._force_voice_sync(MockVoiceChannel(members=[member])) member.move_to.assert_not_called() async def test_force_voice_sync_no_channel(self): """Test to ensure _force_voice_sync can create its own voice channel if one is not available.""" await self.cog._async_init() - member = MockMember() channel = MockVoiceChannel(guild=MockGuild(afk_channel=None)) - - new_channel = MockVoiceChannel(delete=Mock()) + new_channel = MockVoiceChannel(delete=AsyncMock()) channel.guild.create_voice_channel.return_value = new_channel - with mock.patch.object(self.cog.scheduler, "schedule_later") as scheduler: - await self.cog._force_voice_sync(channel, member) + await self.cog._force_voice_sync(channel) + + # Check channel creation + overwrites = { + channel.guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) + } + channel.guild.create_voice_channel.assert_called_once_with("mute-temp", overwrites=overwrites) + + # Check bot deleted channel + new_channel.delete.assert_called_once_with(reason="Deleting temp mute channel.") + + async def test_voice_kick(self): + """Test to ensure kick function can remove all members from a voice channel.""" + await self.cog._async_init() + + members = [] + for _ in range(10): + members.append(MockMember()) + + channel = MockVoiceChannel(members=members) + await self.cog._kick_voice_members(channel) + + for member in members: + member.move_to.assert_called_once_with(None, reason="Kicking member from voice channel.") - # Check channel creation - overwrites = { - channel.guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) - } - channel.guild.create_voice_channel.assert_called_once_with("mute-temp", overwrites=overwrites) + async def test_voice_kick_staff(self): + """Test to ensure voice kick skips staff members.""" + await self.cog._async_init() + member = MockMember(roles=[self.cog._helper_role]) - # Check bot queued deletion - new_channel.delete.assert_called_once_with(reason="Deleting temp mute channel.") - scheduler.assert_called_once_with(30, new_channel.id, new_channel.delete()) + await self.cog._kick_voice_members(MockVoiceChannel(members=[member])) + member.move_to.assert_not_called() @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) @@ -457,21 +459,45 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): ) for target, message in test_cases: - with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): - with self.subTest(target_channel=target, message=message): - await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) - if ctx.channel == target or target is None: - ctx.channel.send.assert_called_once_with(message) + with mock.patch.object(self.cog, "_force_voice_sync") as voice_sync: + with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): + with self.subTest(target_channel=target, message=message): + await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) + if ctx.channel == target or target is None: + ctx.channel.send.assert_called_once_with(message) - else: - ctx.channel.send.assert_called_once_with(message.replace("current channel", target.mention)) - if isinstance(target, MockTextChannel): - target.send.assert_called_once_with(message) + else: + ctx.channel.send.assert_called_once_with(message.replace("current channel", target.mention)) + if isinstance(target, MockTextChannel): + target.send.assert_called_once_with(message) + else: + voice_sync.assert_called_once_with(target) ctx.channel.send.reset_mock() if target is not None and isinstance(target, MockTextChannel): target.send.reset_mock() + @mock.patch.object(silence.Silence, "_kick_voice_members") + @mock.patch.object(silence.Silence, "_force_voice_sync") + async def test_sync_or_kick_called(self, sync, kick): + """Tests if silence command calls kick or sync on voice channels when appropriate.""" + channel = MockVoiceChannel() + ctx = MockContext() + + with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): + with self.subTest("Test calls kick"): + await self.cog.silence.callback(self.cog, ctx, 10, kick=True, channel=channel) + kick.assert_called_once_with(channel) + sync.assert_not_called() + + kick.reset_mock() + sync.reset_mock() + + with self.subTest("Test calls sync"): + await self.cog.silence.callback(self.cog, ctx, 10, kick=False, channel=channel) + sync.assert_called_once_with(channel) + kick.assert_not_called() + async def test_skipped_already_silenced(self): """Permissions were not set and `False` was returned for an already silenced channel.""" subtests = ( -- cgit v1.2.3 From a8a8c104823fa1a23a9b33cd52c7c4e574d84330 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sat, 28 Nov 2020 21:10:24 +0300 Subject: Refractors According To Style Guide Updates changes made in the PR to be more inline with style guide. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 32 ++++---- tests/bot/exts/moderation/test_silence.py | 120 ++++++++++++++---------------- 2 files changed, 74 insertions(+), 78 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 9b3725326..45c3f5b92 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -31,6 +31,8 @@ MSG_UNSILENCE_MANUAL = ( ) MSG_UNSILENCE_SUCCESS = f"{constants.Emojis.check_mark} unsilenced current channel." +TextOrVoiceChannel = Union[TextChannel, VoiceChannel] + class SilenceNotifier(tasks.Loop): """Loop notifier for posting notices to `alert_channel` containing added channels.""" @@ -40,7 +42,7 @@ class SilenceNotifier(tasks.Loop): self._silenced_channels = {} self._alert_channel = alert_channel - def add_channel(self, channel: Union[TextChannel, VoiceChannel]) -> None: + def add_channel(self, channel: TextOrVoiceChannel) -> None: """Add channel to `_silenced_channels` and start loop if not launched.""" if not self._silenced_channels: self.start() @@ -119,7 +121,10 @@ class Silence(commands.Cog): return self.bot.get_channel(channels[name]) async def send_message( - self, message: str, source_channel: TextChannel, target_channel: Union[TextChannel, VoiceChannel], + self, + message: str, + source_channel: TextChannel, + target_channel: TextOrVoiceChannel, alert_target: bool = False ) -> None: """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`.""" @@ -138,7 +143,7 @@ class Silence(commands.Cog): elif source_channel != target_channel: await target_channel.send(message) - async def _select_lock_channel(*args) -> Union[TextChannel, VoiceChannel]: + async def _select_lock_channel(*args) -> TextOrVoiceChannel: """Passes the channel to be silenced to the resource lock.""" channel = args[0].get("channel") if channel is not None: @@ -150,8 +155,11 @@ class Silence(commands.Cog): @commands.command(aliases=("hush",)) @lock(LOCK_NAMESPACE, _select_lock_channel, raise_error=True) async def silence( - self, ctx: Context, duration: HushDurationConverter = 10, kick: bool = False, - *, channel: Union[TextChannel, VoiceChannel] = None + self, + ctx: Context, + duration: HushDurationConverter = 10, + kick: bool = False, + *, channel: TextOrVoiceChannel = None ) -> None: """ Silence the current channel for `duration` minutes or `forever`. @@ -191,7 +199,7 @@ class Silence(commands.Cog): await self.send_message(MSG_SILENCE_SUCCESS.format(duration=duration), ctx.channel, channel, True) @commands.command(aliases=("unhush",)) - async def unsilence(self, ctx: Context, *, channel: Union[TextChannel, VoiceChannel] = None) -> None: + async def unsilence(self, ctx: Context, *, channel: TextOrVoiceChannel = None) -> None: """ Unsilence the given channel if given, else the current one. @@ -204,9 +212,7 @@ class Silence(commands.Cog): await self._unsilence_wrapper(channel, ctx) @lock_arg(LOCK_NAMESPACE, "channel", raise_error=True) - async def _unsilence_wrapper( - self, channel: Union[TextChannel, VoiceChannel], ctx: Optional[Context] = None - ) -> None: + async def _unsilence_wrapper(self, channel: TextOrVoiceChannel, ctx: Optional[Context] = None) -> None: """Unsilence `channel` and send a success/failure message.""" msg_channel = channel if ctx is not None: @@ -229,7 +235,7 @@ class Silence(commands.Cog): else: await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, True) - async def _set_silence_overwrites(self, channel: Union[TextChannel, VoiceChannel], kick: bool = False) -> bool: + async def _set_silence_overwrites(self, channel: TextOrVoiceChannel, kick: bool = False) -> bool: """Set silence permission overwrites for `channel` and return True if successful.""" # Get the original channel overwrites if isinstance(channel, TextChannel): @@ -307,9 +313,7 @@ class Silence(commands.Cog): if delete_channel: await afk_channel.delete(reason="Deleting temp mute channel.") - async def _schedule_unsilence( - self, ctx: Context, channel: Union[TextChannel, VoiceChannel], duration: Optional[int] - ) -> None: + async def _schedule_unsilence(self, ctx: Context, channel: TextOrVoiceChannel, duration: Optional[int]) -> None: """Schedule `ctx.channel` to be unsilenced if `duration` is not None.""" if duration is None: await self.unsilence_timestamps.set(channel.id, -1) @@ -318,7 +322,7 @@ class Silence(commands.Cog): unsilence_time = datetime.now(tz=timezone.utc) + timedelta(minutes=duration) await self.unsilence_timestamps.set(channel.id, unsilence_time.timestamp()) - async def _unsilence(self, channel: Union[TextChannel, VoiceChannel]) -> bool: + async def _unsilence(self, channel: TextOrVoiceChannel) -> bool: """ Unsilence `channel`. diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 9fb3e404a..635e017e3 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -198,49 +198,37 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): reset() with self.subTest("Replacement One Channel Test"): - await self.cog.send_message( - "Current. The following should be replaced: current channel.", text_channel_1, text_channel_2, False - ) - - text_channel_1.send.assert_called_once_with( - f"Current. The following should be replaced: {text_channel_1.mention}." - ) + message = "Current. The following should be replaced: current channel." + await self.cog.send_message(message, text_channel_1, text_channel_2, False) + text_channel_1.send.assert_called_once_with(message.replace("current channel", text_channel_1.mention)) text_channel_2.send.assert_not_called() reset() with self.subTest("Replacement Two Channel Test"): - await self.cog.send_message( - "Current. The following should be replaced: current channel.", text_channel_1, text_channel_2, True - ) - - text_channel_1.send.assert_called_once_with( - f"Current. The following should be replaced: {text_channel_1.mention}." - ) + message = "Current. The following should be replaced: current channel." + await self.cog.send_message(message, text_channel_1, text_channel_2, True) - text_channel_2.send.assert_called_once_with("Current. The following should be replaced: current channel.") + text_channel_1.send.assert_called_once_with(message.replace("current channel", text_channel_1.mention)) + text_channel_2.send.assert_called_once_with(message) reset() with self.subTest("Text and Voice"): - await self.cog.send_message( - "This should show up just here", text_channel_1, voice_channel, False - ) + await self.cog.send_message("This should show up just here", text_channel_1, voice_channel, False) text_channel_1.send.assert_called_once_with("This should show up just here") reset() with self.subTest("Text and Voice"): - await self.cog.send_message( - "This should show up as current channel", text_channel_1, voice_channel, True - ) - text_channel_1.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") - text_channel_2.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") + message = "This should show up as current channel" + await self.cog.send_message(message, text_channel_1, voice_channel, True) + text_channel_1.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) + text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) reset() with self.subTest("Text and Voice Same Invocation"): - await self.cog.send_message( - "This should show up as current channel", text_channel_2, voice_channel, True - ) - text_channel_2.send.assert_called_once_with(f"This should show up as {voice_channel.mention}") + message = "This should show up as current channel" + await self.cog.send_message(message, text_channel_2, voice_channel, True) + text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) async def test_get_related_text_channel(self): """Tests the helper function that connects voice to text channels.""" @@ -276,10 +264,11 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): await self.cog._force_voice_sync(channel) for member in members: self.assertEqual(member.move_to.call_count, 2) - member.move_to.assert_has_calls([ + calls = [ mock.call(afk_channel, reason="Muting VC member."), mock.call(channel, reason="Muting VC member.") - ], any_order=False) + ] + member.move_to.assert_has_calls(calls, any_order=False) async def test_force_voice_sync_staff(self): """Tests to ensure _force_voice_sync does not kick staff members.""" @@ -445,7 +434,9 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog.silence.callback(self.cog, ctx, duration) ctx.channel.send.assert_called_once_with(message) - async def test_sent_to_correct_channel(self): + @mock.patch.object(silence.Silence, "_set_silence_overwrites", return_value=True) + @mock.patch.object(silence.Silence, "_force_voice_sync") + async def test_sent_to_correct_channel(self, voice_sync, _): """Test function sends messages to the correct channels.""" text_channel = MockTextChannel() voice_channel = MockVoiceChannel() @@ -459,19 +450,17 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): ) for target, message in test_cases: - with mock.patch.object(self.cog, "_force_voice_sync") as voice_sync: - with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): - with self.subTest(target_channel=target, message=message): - await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) - if ctx.channel == target or target is None: - ctx.channel.send.assert_called_once_with(message) - - else: - ctx.channel.send.assert_called_once_with(message.replace("current channel", target.mention)) - if isinstance(target, MockTextChannel): - target.send.assert_called_once_with(message) - else: - voice_sync.assert_called_once_with(target) + with self.subTest(target_channel=target, message=message): + await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) + if ctx.channel == target or target is None: + ctx.channel.send.assert_called_once_with(message) + + else: + ctx.channel.send.assert_called_once_with(message.replace("current channel", target.mention)) + if isinstance(target, MockTextChannel): + target.send.assert_called_once_with(message) + else: + voice_sync.assert_called_once_with(target) ctx.channel.send.reset_mock() if target is not None and isinstance(target, MockTextChannel): @@ -771,7 +760,9 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): self.assertDictEqual(prev_overwrite_dict, new_overwrite_dict) - async def test_unsilence_helper_fail(self): + @mock.patch.object(silence.Silence, "_unsilence", return_value=False) + @mock.patch.object(silence.Silence, "send_message") + async def test_unsilence_helper_fail(self, send_message, _): """Tests unsilence_wrapper when `_unsilence` fails.""" ctx = MockContext() @@ -797,16 +788,18 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): return self.val for context, channel, role, permission, message in test_cases: - with self.subTest(channel=channel, message=message): - with mock.patch.object(channel, "overwrites_for", return_value=PermClass(permission)) as overwrites: - with mock.patch.object(self.cog, "send_message") as send_message: - with mock.patch.object(self.cog, "_unsilence", return_value=False): - await self.cog._unsilence_wrapper(channel, context) + with mock.patch.object(channel, "overwrites_for", return_value=PermClass(permission)) as overwrites: + with self.subTest(channel=channel, message=message): + await self.cog._unsilence_wrapper(channel, context) + + overwrites.assert_called_once_with(role) + send_message.assert_called_once_with(message, ctx.channel, channel) - overwrites.assert_called_once_with(role) - send_message.assert_called_once_with(message, ctx.channel, channel) + send_message.reset_mock() - async def test_correct_overwrites(self): + @mock.patch.object(silence.Silence, "_force_voice_sync") + @mock.patch.object(silence.Silence, "send_message") + async def test_correct_overwrites(self, send_message, _): """Tests the overwrites returned by the _unsilence_wrapper are correct for voice and text channels.""" ctx = MockContext() @@ -822,6 +815,7 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): text_channel.reset_mock() voice_channel.reset_mock() + send_message.reset_mock() await reset() default_text_overwrites = text_channel.overwrites_for(text_role) @@ -836,17 +830,15 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): for context, channel, role, overwrites, message in test_cases: with self.subTest(ctx=context, channel=channel): - with mock.patch.object(self.cog, "send_message") as send_message: - with mock.patch.object(self.cog, "_force_voice_sync"): - await self.cog._unsilence_wrapper(channel, context) - - if context is None: - send_message.assert_called_once_with(message, channel, channel, True) - else: - send_message.assert_called_once_with(message, context.channel, channel, True) - - channel.set_permissions.assert_called_once_with(role, overwrite=overwrites) - if channel != ctx.channel: - ctx.channel.send.assert_not_called() + await self.cog._unsilence_wrapper(channel, context) + + if context is None: + send_message.assert_called_once_with(message, channel, channel, True) + else: + send_message.assert_called_once_with(message, context.channel, channel, True) + + channel.set_permissions.assert_called_once_with(role, overwrite=overwrites) + if channel != ctx.channel: + ctx.channel.send.assert_not_called() await reset() -- cgit v1.2.3 From ca266629c8083c261208ddba86ffe9e2a8b65bf3 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 1 Dec 2020 10:19:22 +0300 Subject: Fixes Voice Silence Tests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 66 +++++++++++++++---------------- 1 file changed, 32 insertions(+), 34 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 635e017e3..90ddf6ad7 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -168,8 +168,8 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): role_check.assert_called_once_with(*(1, 2, 3)) role_check.return_value.predicate.assert_awaited_once_with(ctx) - @mock.patch.object(silence.Silence, "_get_related_text_channel") - async def test_send_message(self, mock_get_related_text_channel): + @mock.patch.object(silence, "VOICE_CHANNELS") + async def test_send_message(self, mock_voice_channels): """Test the send function reports to the correct channels.""" text_channel_1 = MockTextChannel() text_channel_2 = MockTextChannel() @@ -178,12 +178,13 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): voice_channel.name = "General/Offtopic" voice_channel.mention = f"#{voice_channel.name}" - mock_get_related_text_channel.return_value = text_channel_2 + mock_voice_channels.get.return_value = text_channel_2.id def reset(): text_channel_1.reset_mock() text_channel_2.reset_mock() voice_channel.reset_mock() + mock_voice_channels.reset_mock() with self.subTest("Basic One Channel Test"): await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, False) @@ -217,38 +218,29 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): await self.cog.send_message("This should show up just here", text_channel_1, voice_channel, False) text_channel_1.send.assert_called_once_with("This should show up just here") - reset() - with self.subTest("Text and Voice"): - message = "This should show up as current channel" - await self.cog.send_message(message, text_channel_1, voice_channel, True) - text_channel_1.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) - text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) + with mock.patch.object(self.cog, "bot") as bot_mock: + bot_mock.get_channel.return_value = text_channel_2 - reset() - with self.subTest("Text and Voice Same Invocation"): - message = "This should show up as current channel" - await self.cog.send_message(message, text_channel_2, voice_channel, True) - text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) - - async def test_get_related_text_channel(self): - """Tests the helper function that connects voice to text channels.""" - voice_channel = MockVoiceChannel() + reset() + with self.subTest("Text and Voice"): + message = "This should show up as current channel" + await self.cog.send_message(message, text_channel_1, voice_channel, True) + text_channel_1.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) + text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) - tests = ( - ("Off-Topic/General", Channels.voice_chat), - ("code/help 1", Channels.code_help_voice), - ("Staff", Channels.staff_voice), - ("ADMIN", Channels.admins_voice), - ("not in the channel list", None) - ) + mock_voice_channels.get.assert_called_once_with(voice_channel.id) + bot_mock.get_channel.assert_called_once_with(text_channel_2.id) + bot_mock.reset_mock() - with mock.patch.object(self.cog.bot, "get_channel", lambda x: x): - for (name, channel_id) in tests: - voice_channel.name = name - voice_channel.id = channel_id + reset() + with self.subTest("Text and Voice Same Invocation"): + message = "This should show up as current channel" + await self.cog.send_message(message, text_channel_2, voice_channel, True) + text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) - result_id = await self.cog._get_related_text_channel(voice_channel) - self.assertEqual(result_id, channel_id) + mock_voice_channels.get.assert_called_once_with(voice_channel.id) + bot_mock.get_channel.assert_called_once_with(text_channel_2.id) + bot_mock.reset_mock() async def test_force_voice_sync(self): """Tests the _force_voice_sync helper function.""" @@ -451,7 +443,10 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): for target, message in test_cases: with self.subTest(target_channel=target, message=message): - await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) + with mock.patch.object(self.cog, "bot") as bot_mock: + bot_mock.get_channel.return_value = AsyncMock() + await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) + if ctx.channel == target or target is None: ctx.channel.send.assert_called_once_with(message) @@ -466,14 +461,17 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): if target is not None and isinstance(target, MockTextChannel): target.send.reset_mock() + @mock.patch.object(silence.Silence, "_set_silence_overwrites", return_value=True) @mock.patch.object(silence.Silence, "_kick_voice_members") @mock.patch.object(silence.Silence, "_force_voice_sync") - async def test_sync_or_kick_called(self, sync, kick): + async def test_sync_or_kick_called(self, sync, kick, _): """Tests if silence command calls kick or sync on voice channels when appropriate.""" channel = MockVoiceChannel() ctx = MockContext() - with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): + with mock.patch.object(self.cog, "bot") as bot_mock: + bot_mock.get_channel.return_value = AsyncMock() + with self.subTest("Test calls kick"): await self.cog.silence.callback(self.cog, ctx, 10, kick=True, channel=channel) kick.assert_called_once_with(channel) -- cgit v1.2.3 From 68c2545e794fa284a957809a0cb1022740966118 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 29 Dec 2020 19:11:51 +0300 Subject: Refractors Helper Method Signatures Changes the signatures of a few helper methods to make them more concise and understandable. --- bot/exts/moderation/silence.py | 34 ++++++++++++++++--------------- tests/bot/exts/moderation/test_silence.py | 23 +++++++++++---------- 2 files changed, 30 insertions(+), 27 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index befcb2cc4..31103bc3e 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -2,7 +2,7 @@ import json import logging from contextlib import suppress from datetime import datetime, timedelta, timezone -from typing import Optional, Union +from typing import Optional, OrderedDict, Union from async_rediscache import RedisCache from discord import Guild, PermissionOverwrite, TextChannel, VoiceChannel @@ -81,6 +81,16 @@ class SilenceNotifier(tasks.Loop): ) +async def _select_lock_channel(args: OrderedDict[str, any]) -> TextOrVoiceChannel: + """Passes the channel to be silenced to the resource lock.""" + channel = args["channel"] + if channel is not None: + return channel + + else: + return args["ctx"].channel + + class Silence(commands.Cog): """Commands for stopping channel messages for `verified` role in a channel.""" @@ -118,7 +128,7 @@ class Silence(commands.Cog): message: str, source_channel: TextChannel, target_channel: TextOrVoiceChannel, - alert_target: bool = False + *, alert_target: bool = False ) -> None: """Helper function to send message confirmation to `source_channel`, and notification to `target_channel`.""" # Reply to invocation channel @@ -137,23 +147,14 @@ class Silence(commands.Cog): elif source_channel != target_channel: await target_channel.send(message) - async def _select_lock_channel(*args) -> TextOrVoiceChannel: - """Passes the channel to be silenced to the resource lock.""" - channel = args[0].get("channel") - if channel is not None: - return channel - - else: - return args[0].get("ctx").channel - @commands.command(aliases=("hush",)) @lock(LOCK_NAMESPACE, _select_lock_channel, raise_error=True) async def silence( self, ctx: Context, duration: HushDurationConverter = 10, - kick: bool = False, - *, channel: TextOrVoiceChannel = None + channel: TextOrVoiceChannel = None, + *, kick: bool = False ) -> None: """ Silence the current channel for `duration` minutes or `forever`. @@ -186,11 +187,12 @@ class Silence(commands.Cog): if duration is None: self.notifier.add_channel(channel) log.info(f"Silenced {channel_info} indefinitely.") - await self.send_message(MSG_SILENCE_PERMANENT, ctx.channel, channel, True) + await self.send_message(MSG_SILENCE_PERMANENT, ctx.channel, channel, alert_target=True) else: log.info(f"Silenced {channel_info} for {duration} minute(s).") - await self.send_message(MSG_SILENCE_SUCCESS.format(duration=duration), ctx.channel, channel, True) + formatted_message = MSG_SILENCE_SUCCESS.format(duration=duration) + await self.send_message(formatted_message, ctx.channel, channel, alert_target=True) @commands.command(aliases=("unhush",)) async def unsilence(self, ctx: Context, *, channel: TextOrVoiceChannel = None) -> None: @@ -227,7 +229,7 @@ class Silence(commands.Cog): await self.send_message(MSG_UNSILENCE_FAIL, msg_channel, channel) else: - await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, True) + await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, alert_target=True) async def _set_silence_overwrites(self, channel: TextOrVoiceChannel, kick: bool = False) -> bool: """Set silence permission overwrites for `channel` and return True if successful.""" diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 31894761c..038e0a1a4 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -178,20 +178,20 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): mock_voice_channels.reset_mock() with self.subTest("Basic One Channel Test"): - await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, False) + await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, alert_target=False) text_channel_1.send.assert_called_once_with("Text basic message.") text_channel_2.send.assert_not_called() reset() with self.subTest("Basic Two Channel Test"): - await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, True) + await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, alert_target=True) text_channel_1.send.assert_called_once_with("Text basic message.") text_channel_2.send.assert_called_once_with("Text basic message.") reset() with self.subTest("Replacement One Channel Test"): message = "Current. The following should be replaced: current channel." - await self.cog.send_message(message, text_channel_1, text_channel_2, False) + await self.cog.send_message(message, text_channel_1, text_channel_2, alert_target=False) text_channel_1.send.assert_called_once_with(message.replace("current channel", text_channel_1.mention)) text_channel_2.send.assert_not_called() @@ -199,15 +199,16 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): reset() with self.subTest("Replacement Two Channel Test"): message = "Current. The following should be replaced: current channel." - await self.cog.send_message(message, text_channel_1, text_channel_2, True) + await self.cog.send_message(message, text_channel_1, text_channel_2, alert_target=True) text_channel_1.send.assert_called_once_with(message.replace("current channel", text_channel_1.mention)) text_channel_2.send.assert_called_once_with(message) reset() with self.subTest("Text and Voice"): - await self.cog.send_message("This should show up just here", text_channel_1, voice_channel, False) - text_channel_1.send.assert_called_once_with("This should show up just here") + message = "This should show up just here" + await self.cog.send_message(message, text_channel_1, voice_channel, alert_target=False) + text_channel_1.send.assert_called_once_with(message) with mock.patch.object(self.cog, "bot") as bot_mock: bot_mock.get_channel.return_value = text_channel_2 @@ -215,7 +216,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): reset() with self.subTest("Text and Voice"): message = "This should show up as current channel" - await self.cog.send_message(message, text_channel_1, voice_channel, True) + await self.cog.send_message(message, text_channel_1, voice_channel, alert_target=True) text_channel_1.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) @@ -226,7 +227,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): reset() with self.subTest("Text and Voice Same Invocation"): message = "This should show up as current channel" - await self.cog.send_message(message, text_channel_2, voice_channel, True) + await self.cog.send_message(message, text_channel_2, voice_channel, alert_target=True) text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) mock_voice_channels.get.assert_called_once_with(voice_channel.id) @@ -436,7 +437,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): with self.subTest(target_channel=target, message=message): with mock.patch.object(self.cog, "bot") as bot_mock: bot_mock.get_channel.return_value = AsyncMock() - await self.cog.silence.callback(self.cog, ctx, 10, False, channel=target) + await self.cog.silence.callback(self.cog, ctx, 10, target, kick=False) if ctx.channel == target or target is None: ctx.channel.send.assert_called_once_with(message) @@ -822,9 +823,9 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._unsilence_wrapper(channel, context) if context is None: - send_message.assert_called_once_with(message, channel, channel, True) + send_message.assert_called_once_with(message, channel, channel, alert_target=True) else: - send_message.assert_called_once_with(message, context.channel, channel, True) + send_message.assert_called_once_with(message, context.channel, channel, alert_target=True) channel.set_permissions.assert_called_once_with(role, overwrite=overwrites) if channel != ctx.channel: -- cgit v1.2.3 From 1b747fccd16d2667c6f4129a222cd9ea3eda5602 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 29 Dec 2020 19:18:03 +0300 Subject: Makes Kick Keyword Only Parameter --- bot/exts/moderation/silence.py | 4 ++-- tests/bot/exts/moderation/test_silence.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 157c150fd..e91e558ec 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -171,7 +171,7 @@ class Silence(commands.Cog): channel_info = f"#{channel} ({channel.id})" log.debug(f"{ctx.author} is silencing channel {channel_info}.") - if not await self._set_silence_overwrites(channel, kick): + if not await self._set_silence_overwrites(channel, kick=kick): log.info(f"Tried to silence channel {channel_info} but the channel was already silenced.") await self.send_message(MSG_SILENCE_FAIL, ctx.channel, channel) return @@ -231,7 +231,7 @@ class Silence(commands.Cog): else: await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, alert_target=True) - async def _set_silence_overwrites(self, channel: TextOrVoiceChannel, kick: bool = False) -> bool: + async def _set_silence_overwrites(self, channel: TextOrVoiceChannel, *, kick: bool = False) -> bool: """Set silence permission overwrites for `channel` and return True if successful.""" # Get the original channel overwrites if isinstance(channel, TextChannel): diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 038e0a1a4..44c3620ac 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -586,7 +586,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel)) self.assertFalse(self.voice_overwrite.speak) - self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel, True)) + self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel, kick=True)) self.assertFalse(self.voice_overwrite.speak or self.voice_overwrite.connect) -- cgit v1.2.3 From 9d3fba1c3143a529e32bd660696922c3ff902d16 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Tue, 29 Dec 2020 20:06:51 +0300 Subject: Breaks Out Send Message Tests Moves the tests for the helper method `send_message` to simplify tests, and avoid repeated code. --- tests/bot/exts/moderation/test_silence.py | 156 ++++++++++++++++-------------- 1 file changed, 81 insertions(+), 75 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 44c3620ac..8f4574d13 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -159,81 +159,6 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): role_check.assert_called_once_with(*(1, 2, 3)) role_check.return_value.predicate.assert_awaited_once_with(ctx) - @mock.patch.object(silence, "VOICE_CHANNELS") - async def test_send_message(self, mock_voice_channels): - """Test the send function reports to the correct channels.""" - text_channel_1 = MockTextChannel() - text_channel_2 = MockTextChannel() - - voice_channel = MockVoiceChannel() - voice_channel.name = "General/Offtopic" - voice_channel.mention = f"#{voice_channel.name}" - - mock_voice_channels.get.return_value = text_channel_2.id - - def reset(): - text_channel_1.reset_mock() - text_channel_2.reset_mock() - voice_channel.reset_mock() - mock_voice_channels.reset_mock() - - with self.subTest("Basic One Channel Test"): - await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, alert_target=False) - text_channel_1.send.assert_called_once_with("Text basic message.") - text_channel_2.send.assert_not_called() - - reset() - with self.subTest("Basic Two Channel Test"): - await self.cog.send_message("Text basic message.", text_channel_1, text_channel_2, alert_target=True) - text_channel_1.send.assert_called_once_with("Text basic message.") - text_channel_2.send.assert_called_once_with("Text basic message.") - - reset() - with self.subTest("Replacement One Channel Test"): - message = "Current. The following should be replaced: current channel." - await self.cog.send_message(message, text_channel_1, text_channel_2, alert_target=False) - - text_channel_1.send.assert_called_once_with(message.replace("current channel", text_channel_1.mention)) - text_channel_2.send.assert_not_called() - - reset() - with self.subTest("Replacement Two Channel Test"): - message = "Current. The following should be replaced: current channel." - await self.cog.send_message(message, text_channel_1, text_channel_2, alert_target=True) - - text_channel_1.send.assert_called_once_with(message.replace("current channel", text_channel_1.mention)) - text_channel_2.send.assert_called_once_with(message) - - reset() - with self.subTest("Text and Voice"): - message = "This should show up just here" - await self.cog.send_message(message, text_channel_1, voice_channel, alert_target=False) - text_channel_1.send.assert_called_once_with(message) - - with mock.patch.object(self.cog, "bot") as bot_mock: - bot_mock.get_channel.return_value = text_channel_2 - - reset() - with self.subTest("Text and Voice"): - message = "This should show up as current channel" - await self.cog.send_message(message, text_channel_1, voice_channel, alert_target=True) - text_channel_1.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) - text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) - - mock_voice_channels.get.assert_called_once_with(voice_channel.id) - bot_mock.get_channel.assert_called_once_with(text_channel_2.id) - bot_mock.reset_mock() - - reset() - with self.subTest("Text and Voice Same Invocation"): - message = "This should show up as current channel" - await self.cog.send_message(message, text_channel_2, voice_channel, alert_target=True) - text_channel_2.send.assert_called_once_with(message.replace("current channel", voice_channel.mention)) - - mock_voice_channels.get.assert_called_once_with(voice_channel.id) - bot_mock.get_channel.assert_called_once_with(text_channel_2.id) - bot_mock.reset_mock() - async def test_force_voice_sync(self): """Tests the _force_voice_sync helper function.""" await self.cog._async_init() @@ -832,3 +757,84 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): ctx.channel.send.assert_not_called() await reset() + + +class SendMessageTests(unittest.IsolatedAsyncioTestCase): + """Unittests for the send message helper function.""" + + def setUp(self) -> None: + self.bot = MockBot() + self.cog = silence.Silence(self.bot) + + self.text_channels = [MockTextChannel() for _ in range(2)] + self.bot.get_channel.return_value = self.text_channels[1] + + self.voice_channel = MockVoiceChannel(name="General/Offtopic") + + async def test_send_to_channel(self): + """Tests a basic case for the send function.""" + message = "Test basic message." + await self.cog.send_message(message, *self.text_channels, alert_target=False) + + self.text_channels[0].send.assert_called_once_with(message) + self.text_channels[1].send.assert_not_called() + + async def test_send_to_multiple_channels(self): + """Tests sending messages to two channels.""" + message = "Test basic message." + await self.cog.send_message(message, *self.text_channels, alert_target=True) + + self.text_channels[0].send.assert_called_once_with(message) + self.text_channels[1].send.assert_called_once_with(message) + + async def test_duration_replacement(self): + """Tests that the channel name was set correctly for one target channel.""" + message = "Current. The following should be replaced: current channel." + await self.cog.send_message(message, *self.text_channels, alert_target=False) + + updated_message = message.replace("current channel", self.text_channels[0].mention) + self.text_channels[0].send.assert_called_once_with(updated_message) + self.text_channels[1].send.assert_not_called() + + async def test_name_replacement_multiple_channels(self): + """Tests that the channel name was set correctly for two channels.""" + message = "Current. The following should be replaced: current channel." + await self.cog.send_message(message, *self.text_channels, alert_target=True) + + updated_message = message.replace("current channel", self.text_channels[0].mention) + self.text_channels[0].send.assert_called_once_with(updated_message) + self.text_channels[1].send.assert_called_once_with(message) + + async def test_silence_voice(self): + """Tests that the correct message was sent when a voice channel is muted without alerting.""" + message = "This should show up just here." + await self.cog.send_message(message, self.text_channels[0], self.voice_channel, alert_target=False) + self.text_channels[0].send.assert_called_once_with(message) + + async def test_silence_voice_alert(self): + """Tests that the correct message was sent when a voice channel is muted with alerts.""" + with unittest.mock.patch.object(silence, "VOICE_CHANNELS") as mock_voice_channels: + mock_voice_channels.get.return_value = self.text_channels[1].id + + message = "This should show up as current channel." + await self.cog.send_message(message, self.text_channels[0], self.voice_channel, alert_target=True) + + updated_message = message.replace("current channel", self.voice_channel.mention) + self.text_channels[0].send.assert_called_once_with(updated_message) + self.text_channels[1].send.assert_called_once_with(updated_message) + + mock_voice_channels.get.assert_called_once_with(self.voice_channel.id) + + async def test_silence_voice_sibling_channel(self): + """Tests silencing a voice channel from the related text channel.""" + with unittest.mock.patch.object(silence, "VOICE_CHANNELS") as mock_voice_channels: + mock_voice_channels.get.return_value = self.text_channels[1].id + + message = "This should show up as current channel." + await self.cog.send_message(message, self.text_channels[1], self.voice_channel, alert_target=True) + + updated_message = message.replace("current channel", self.voice_channel.mention) + self.text_channels[1].send.assert_called_once_with(updated_message) + + mock_voice_channels.get.assert_called_once_with(self.voice_channel.id) + self.bot.get_channel.assert_called_once_with(self.text_channels[1].id) -- cgit v1.2.3 From 70fcd2b68706ae5e8e35407beaa04e2895f3dae8 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sun, 10 Jan 2021 16:22:15 +0300 Subject: Cleans Up & Simplifies Tests Cleans up the silence tests by removing unneeded or repeated mocks. Simplifies tests where possible by joining similar tests. --- bot/exts/moderation/silence.py | 4 +- tests/bot/exts/moderation/test_silence.py | 111 +++++++++--------------------- 2 files changed, 35 insertions(+), 80 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 26aa77b61..1a3c48394 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -229,9 +229,9 @@ class Silence(commands.Cog): # Send fail message to muted channel or voice chat channel, and invocation channel if manual: - await self.send_message(MSG_UNSILENCE_MANUAL, msg_channel, channel) + await self.send_message(MSG_UNSILENCE_MANUAL, msg_channel, channel, alert_target=False) else: - await self.send_message(MSG_UNSILENCE_FAIL, msg_channel, channel) + await self.send_message(MSG_UNSILENCE_FAIL, msg_channel, channel, alert_target=False) else: await self.send_message(MSG_UNSILENCE_SUCCESS, msg_channel, channel, alert_target=True) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 8f4574d13..5505d7a53 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -163,29 +163,22 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): """Tests the _force_voice_sync helper function.""" await self.cog._async_init() - members = [] - for _ in range(10): - members.append(MockMember()) + members = [MockMember() for _ in range(10)] + members.extend([MockMember(roles=[self.cog._helper_role]) for _ in range(3)]) - afk_channel = MockVoiceChannel() - channel = MockVoiceChannel(guild=MockGuild(afk_channel=afk_channel), members=members) + channel = MockVoiceChannel(members=members) await self.cog._force_voice_sync(channel) for member in members: - self.assertEqual(member.move_to.call_count, 2) - calls = [ - mock.call(afk_channel, reason="Muting VC member."), - mock.call(channel, reason="Muting VC member.") - ] - member.move_to.assert_has_calls(calls, any_order=False) - - async def test_force_voice_sync_staff(self): - """Tests to ensure _force_voice_sync does not kick staff members.""" - await self.cog._async_init() - member = MockMember(roles=[self.cog._helper_role]) + if self.cog._helper_role in member.roles: + member.move_to.assert_not_called() + else: + self.assertEqual(member.move_to.call_count, 2) + calls = member.move_to.call_args_list - await self.cog._force_voice_sync(MockVoiceChannel(members=[member])) - member.move_to.assert_not_called() + # Tests that the member was moved to the afk channel, and back. + self.assertEqual((channel.guild.afk_channel,), calls[0].args) + self.assertEqual((channel,), calls[1].args) async def test_force_voice_sync_no_channel(self): """Test to ensure _force_voice_sync can create its own voice channel if one is not available.""" @@ -204,29 +197,23 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): channel.guild.create_voice_channel.assert_called_once_with("mute-temp", overwrites=overwrites) # Check bot deleted channel - new_channel.delete.assert_called_once_with(reason="Deleting temp mute channel.") + new_channel.delete.assert_called_once() async def test_voice_kick(self): """Test to ensure kick function can remove all members from a voice channel.""" await self.cog._async_init() - members = [] - for _ in range(10): - members.append(MockMember()) + members = [MockMember() for _ in range(10)] + members.extend([MockMember(roles=[self.cog._helper_role]) for _ in range(3)]) channel = MockVoiceChannel(members=members) await self.cog._kick_voice_members(channel) for member in members: - member.move_to.assert_called_once_with(None, reason="Kicking member from voice channel.") - - async def test_voice_kick_staff(self): - """Test to ensure voice kick skips staff members.""" - await self.cog._async_init() - member = MockMember(roles=[self.cog._helper_role]) - - await self.cog._kick_voice_members(MockVoiceChannel(members=[member])) - member.move_to.assert_not_called() + if self.cog._helper_role in member.roles: + member.move_to.assert_not_called() + else: + self.assertEqual((None,), member.move_to.call_args_list[0].args) @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) @@ -311,7 +298,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): @autospec(silence.Silence, "_reschedule", pass_mocks=False) @autospec(silence, "Scheduler", "SilenceNotifier", pass_mocks=False) def setUp(self) -> None: - self.bot = MockBot() + self.bot = MockBot(get_channel=lambda _: MockTextChannel()) self.cog = silence.Silence(self.bot) self.cog._init_task = asyncio.Future() self.cog._init_task.set_result(None) @@ -360,9 +347,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): for target, message in test_cases: with self.subTest(target_channel=target, message=message): - with mock.patch.object(self.cog, "bot") as bot_mock: - bot_mock.get_channel.return_value = AsyncMock() - await self.cog.silence.callback(self.cog, ctx, 10, target, kick=False) + await self.cog.silence.callback(self.cog, ctx, 10, target, kick=False) if ctx.channel == target or target is None: ctx.channel.send.assert_called_once_with(message) @@ -505,9 +490,6 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_correct_permission_updates(self): """Tests if _set_silence_overwrites can correctly get and update permissions.""" - self.assertTrue(await self.cog._set_silence_overwrites(self.text_channel)) - self.assertFalse(self.text_overwrite.send_messages or self.text_overwrite.add_reactions) - self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel)) self.assertFalse(self.voice_overwrite.speak) @@ -548,49 +530,22 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): (False, silence.MSG_UNSILENCE_MANUAL, PermissionOverwrite(send_messages=False)), (False, silence.MSG_UNSILENCE_MANUAL, PermissionOverwrite(add_reactions=False)), ) + for was_unsilenced, message, overwrite in test_cases: ctx = MockContext() - with self.subTest(was_unsilenced=was_unsilenced, message=message, overwrite=overwrite): - with mock.patch.object(self.cog, "_unsilence", return_value=was_unsilenced): - ctx.channel.overwrites_for.return_value = overwrite - await self.cog.unsilence.callback(self.cog, ctx) - ctx.channel.send.assert_called_once_with(message) - - async def test_sent_to_correct_channel(self): - """Test function sends messages to the correct channels.""" - unsilenced_overwrite = PermissionOverwrite(send_messages=True, add_reactions=True) - text_channel = MockTextChannel() - ctx = MockContext() - - test_cases = ( - (None, silence.MSG_UNSILENCE_SUCCESS.format(duration=10)), - (text_channel, silence.MSG_UNSILENCE_SUCCESS.format(duration=10)), - (ctx.channel, silence.MSG_UNSILENCE_SUCCESS.format(duration=10)), - ) - for target, message in test_cases: - with self.subTest(target_channel=target, message=message): - with mock.patch.object(self.cog, "_unsilence", return_value=True): - # Assign Return - if ctx.channel == target or target is None: - ctx.channel.overwrites_for.return_value = unsilenced_overwrite - else: - target.overwrites_for.return_value = unsilenced_overwrite + for target in [None, MockTextChannel()]: + ctx.channel.overwrites_for.return_value = overwrite + if target: + target.overwrites_for.return_value = overwrite - await self.cog.unsilence.callback(self.cog, ctx, channel=target) + with mock.patch.object(self.cog, "_unsilence", return_value=was_unsilenced): + with mock.patch.object(self.cog, "send_message") as send_message: + with self.subTest(was_unsilenced=was_unsilenced, overwrite=overwrite, target=target): + await self.cog.unsilence.callback(self.cog, ctx, channel=target) - # Check Messages - if ctx.channel == target or target is None: - ctx.channel.send.assert_called_once_with(message) - else: - ctx.channel.send.assert_called_once_with( - message.replace("current channel", text_channel.mention) - ) - target.send.assert_called_once_with(message) - - ctx.channel.send.reset_mock() - if target is not None: - target.send.reset_mock() + call_args = (message, ctx.channel, target or ctx.channel) + send_message.assert_called_once_with(*call_args, alert_target=was_unsilenced) async def test_skipped_already_unsilenced(self): """Permissions were not set and `False` was returned for an already unsilenced channel.""" @@ -708,7 +663,7 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._unsilence_wrapper(channel, context) overwrites.assert_called_once_with(role) - send_message.assert_called_once_with(message, ctx.channel, channel) + send_message.assert_called_once_with(message, ctx.channel, channel, alert_target=False) send_message.reset_mock() @@ -769,7 +724,7 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): self.text_channels = [MockTextChannel() for _ in range(2)] self.bot.get_channel.return_value = self.text_channels[1] - self.voice_channel = MockVoiceChannel(name="General/Offtopic") + self.voice_channel = MockVoiceChannel() async def test_send_to_channel(self): """Tests a basic case for the send function.""" -- cgit v1.2.3 From 52c69c5d51d9938cd7a56bf5cbc2c26a371883d9 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Sun, 10 Jan 2021 16:23:32 +0300 Subject: Cleans Up Voice Sync Tests Cleans up the tests related to the voice sync/kick functions by adding a helper method to simplify mocking. --- tests/bot/exts/moderation/test_silence.py | 59 +++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 18 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 5505d7a53..a365b2aae 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -291,6 +291,17 @@ class RescheduleTests(unittest.IsolatedAsyncioTestCase): self.cog.notifier.add_channel.assert_not_called() +def voice_sync_helper(function): + """Helper wrapper to test the sync and kick functions for voice channels.""" + @autospec(silence.Silence, "_force_voice_sync", "_kick_voice_members", "_set_silence_overwrites") + async def inner(self, sync, kick, overwrites): + overwrites.return_value = True + await function(self, MockContext(), + sync, kick) + + return inner + + @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) class SilenceTests(unittest.IsolatedAsyncioTestCase): """Tests for the silence command and its related helper methods.""" @@ -363,29 +374,41 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): if target is not None and isinstance(target, MockTextChannel): target.send.reset_mock() - @mock.patch.object(silence.Silence, "_set_silence_overwrites", return_value=True) - @mock.patch.object(silence.Silence, "_kick_voice_members") - @mock.patch.object(silence.Silence, "_force_voice_sync") - async def test_sync_or_kick_called(self, sync, kick, _): - """Tests if silence command calls kick or sync on voice channels when appropriate.""" + @voice_sync_helper + async def test_sync_called(self, ctx, sync, kick): + """Tests if silence command calls sync on a voice channel.""" channel = MockVoiceChannel() - ctx = MockContext() + await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=False) - with mock.patch.object(self.cog, "bot") as bot_mock: - bot_mock.get_channel.return_value = AsyncMock() + sync.assert_called_once_with(self.cog, channel) + kick.assert_not_called() - with self.subTest("Test calls kick"): - await self.cog.silence.callback(self.cog, ctx, 10, kick=True, channel=channel) - kick.assert_called_once_with(channel) - sync.assert_not_called() + @voice_sync_helper + async def test_kick_called(self, ctx, sync, kick): + """Tests if silence command calls kick on a voice channel.""" + channel = MockVoiceChannel() + await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=True) + + kick.assert_called_once_with(self.cog, channel) + sync.assert_not_called() + + @voice_sync_helper + async def test_sync_not_called(self, ctx, sync, kick): + """Tests that silence command does not call sync on a text channel.""" + channel = MockTextChannel() + await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=False) + + sync.assert_not_called() + kick.assert_not_called() - kick.reset_mock() - sync.reset_mock() + @voice_sync_helper + async def test_kick_not_called(self, ctx, sync, kick): + """Tests that silence command does not call kick on a text channel.""" + channel = MockTextChannel() + await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=True) - with self.subTest("Test calls sync"): - await self.cog.silence.callback(self.cog, ctx, 10, kick=False, channel=channel) - sync.assert_called_once_with(channel) - kick.assert_not_called() + sync.assert_not_called() + kick.assert_not_called() async def test_skipped_already_silenced(self): """Permissions were not set and `False` was returned for an already silenced channel.""" -- cgit v1.2.3 From 1a1a283d617592fbf3995d7cd5e1d88be75f92ea Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Thu, 4 Feb 2021 12:29:59 +0300 Subject: Updates Voice Kick Restriction Tests --- bot/exts/moderation/silence.py | 4 ++-- tests/bot/exts/moderation/test_silence.py | 26 +++++++++++++++++--------- 2 files changed, 19 insertions(+), 11 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 4253cd4f3..ea531c37a 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -117,7 +117,6 @@ class Silence(commands.Cog): self._everyone_role = guild.default_role self._verified_voice_role = guild.get_role(constants.Roles.voice_verified) - self._helper_role = guild.get_role(constants.Roles.helpers) self._mod_alerts_channel = self.bot.get_channel(constants.Channels.mod_alerts) @@ -277,7 +276,8 @@ class Silence(commands.Cog): return afk_channel - async def _kick_voice_members(self, channel: VoiceChannel) -> None: + @staticmethod + async def _kick_voice_members(channel: VoiceChannel) -> None: """Remove all non-staff members from a voice channel.""" log.debug(f"Removing all non staff members from #{channel.name} ({channel.id}).") diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index a365b2aae..2d85af7e0 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -7,9 +7,18 @@ from unittest.mock import AsyncMock, Mock from async_rediscache import RedisSession from discord import PermissionOverwrite -from bot.constants import Channels, Guild, Roles +from bot.constants import Channels, Guild, MODERATION_ROLES, Roles from bot.exts.moderation import silence -from tests.helpers import MockBot, MockContext, MockGuild, MockMember, MockTextChannel, MockVoiceChannel, autospec +from tests.helpers import ( + MockBot, + MockContext, + MockGuild, + MockMember, + MockRole, + MockTextChannel, + MockVoiceChannel, + autospec +) redis_session = None redis_loop = asyncio.get_event_loop() @@ -164,13 +173,13 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): await self.cog._async_init() members = [MockMember() for _ in range(10)] - members.extend([MockMember(roles=[self.cog._helper_role]) for _ in range(3)]) + members.extend([MockMember(roles=[MockRole(id=role)]) for role in MODERATION_ROLES]) channel = MockVoiceChannel(members=members) await self.cog._force_voice_sync(channel) for member in members: - if self.cog._helper_role in member.roles: + if any(role.id in MODERATION_ROLES for role in member.roles): member.move_to.assert_not_called() else: self.assertEqual(member.move_to.call_count, 2) @@ -204,13 +213,13 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): await self.cog._async_init() members = [MockMember() for _ in range(10)] - members.extend([MockMember(roles=[self.cog._helper_role]) for _ in range(3)]) + members.extend([MockMember(roles=[MockRole(id=role)]) for role in MODERATION_ROLES]) channel = MockVoiceChannel(members=members) await self.cog._kick_voice_members(channel) for member in members: - if self.cog._helper_role in member.roles: + if any(role.id in MODERATION_ROLES for role in member.roles): member.move_to.assert_not_called() else: self.assertEqual((None,), member.move_to.call_args_list[0].args) @@ -296,8 +305,7 @@ def voice_sync_helper(function): @autospec(silence.Silence, "_force_voice_sync", "_kick_voice_members", "_set_silence_overwrites") async def inner(self, sync, kick, overwrites): overwrites.return_value = True - await function(self, MockContext(), - sync, kick) + await function(self, MockContext(), sync, kick) return inner @@ -389,7 +397,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockVoiceChannel() await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=True) - kick.assert_called_once_with(self.cog, channel) + kick.assert_called_once_with(channel) sync.assert_not_called() @voice_sync_helper -- cgit v1.2.3 From baa907f5ef056ec3001759cc6f9a9523953afc39 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Thu, 4 Feb 2021 13:07:16 +0300 Subject: Adds Move To Failure Tests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 2d85af7e0..70fe756fd 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -224,6 +224,32 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): else: self.assertEqual((None,), member.move_to.call_args_list[0].args) + async def test_voice_move_to_error(self): + """Test to ensure move_to get called on all members, even if some fail.""" + await self.cog._async_init() + + def failing_move_to(*_): + raise Exception() + failing_members = [MockMember(move_to=Mock(failing_move_to)) for _ in range(5)] + + members = [] + for i in range(5): + members.append(MockMember()) + members.append(failing_members[i]) + + channel = MockVoiceChannel(members=members) + + with self.subTest("Kick"): + await self.cog._kick_voice_members(channel) + for member in members: + member.move_to.assert_called_once() + member.reset_mock() + + with self.subTest("Sync"): + await self.cog._force_voice_sync(channel) + for member in members: + self.assertEqual(member.move_to.call_count, 1 if member in failing_members else 2) + @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) class RescheduleTests(unittest.IsolatedAsyncioTestCase): -- cgit v1.2.3 From 09886c234c9840dc2c2eca0f0a26e72ae6cee527 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Thu, 4 Feb 2021 19:58:50 +0300 Subject: Separates Voice Overwrite Tests Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 70fe756fd..6b48792cb 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -461,7 +461,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.assertFalse(await self.cog._set_silence_overwrites(channel)) channel.set_permissions.assert_not_called() - async def test_silenced_channel(self): + async def test_silenced_text_channel(self): """Channel had `send_message` and `add_reactions` permissions revoked for verified role.""" self.assertTrue(await self.cog._set_silence_overwrites(self.text_channel)) self.assertFalse(self.text_overwrite.send_messages) @@ -471,6 +471,24 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): overwrite=self.text_overwrite ) + async def test_silenced_voice_channel_speak(self): + """Channel had `speak` permissions revoked for verified role.""" + self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel)) + self.assertFalse(self.voice_overwrite.speak) + self.voice_channel.set_permissions.assert_awaited_once_with( + self.cog._verified_voice_role, + overwrite=self.voice_overwrite + ) + + async def test_silenced_voice_channel_full(self): + """Channel had `speak` and `connect` permissions revoked for verified role.""" + self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel, kick=True)) + self.assertFalse(self.voice_overwrite.speak or self.voice_overwrite.connect) + self.voice_channel.set_permissions.assert_awaited_once_with( + self.cog._verified_voice_role, + overwrite=self.voice_overwrite + ) + async def test_preserved_other_overwrites(self): """Channel's other unrelated overwrites were not changed.""" prev_overwrite_dict = dict(self.text_overwrite) @@ -545,14 +563,6 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog.silence.callback(self.cog, ctx, None) self.cog.scheduler.schedule_later.assert_not_called() - async def test_correct_permission_updates(self): - """Tests if _set_silence_overwrites can correctly get and update permissions.""" - self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel)) - self.assertFalse(self.voice_overwrite.speak) - - self.assertTrue(await self.cog._set_silence_overwrites(self.voice_channel, kick=True)) - self.assertFalse(self.voice_overwrite.speak or self.voice_overwrite.connect) - @autospec(silence.Silence, "unsilence_timestamps", pass_mocks=False) class UnsilenceTests(unittest.IsolatedAsyncioTestCase): -- cgit v1.2.3 From 4fea67cb8a535228cb37a83c4a2d44b5112fb707 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Fri, 5 Feb 2021 02:23:10 +0300 Subject: Modifies Silence Tests Adds a missing test assertion, and seperates the voice and text components of a test. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 6b48792cb..2fcf4de43 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -648,14 +648,15 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): self.assertIsNone(self.overwrite.send_messages) self.assertIsNone(self.overwrite.add_reactions) - async def test_cache_miss_sent_mod_alert(self): - """A message was sent to the mod alerts channel.""" + async def test_cache_miss_sent_mod_alert_text(self): + """A message was sent to the mod alerts channel upon muting a text channel.""" self.cog.previous_overwrites.get.return_value = None - await self.cog._unsilence(self.channel) self.cog._mod_alerts_channel.send.assert_awaited_once() - self.cog._mod_alerts_channel.send.reset_mock() + async def test_cache_miss_sent_mod_alert_voice(self): + """A message was sent to the mod alerts channel upon muting a voice channel.""" + self.cog.previous_overwrites.get.return_value = None await self.cog._unsilence(MockVoiceChannel()) self.cog._mod_alerts_channel.send.assert_awaited_once() @@ -832,6 +833,7 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): message = "This should show up just here." await self.cog.send_message(message, self.text_channels[0], self.voice_channel, alert_target=False) self.text_channels[0].send.assert_called_once_with(message) + self.text_channels[1].send.assert_not_called() async def test_silence_voice_alert(self): """Tests that the correct message was sent when a voice channel is muted with alerts.""" -- cgit v1.2.3 From 1343a1c22bcd8a21c2d3fc38293033f546c86036 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Fri, 5 Feb 2021 02:49:06 +0300 Subject: Modifies Silence Tests Adds a missing test assertion, and seperates the voice and text components of a test. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 43 ++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 15 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 2fcf4de43..a52f2447d 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -1,6 +1,7 @@ import asyncio import unittest from datetime import datetime, timezone +from typing import List, Tuple from unittest import mock from unittest.mock import AsyncMock, Mock @@ -224,31 +225,43 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): else: self.assertEqual((None,), member.move_to.call_args_list[0].args) - async def test_voice_move_to_error(self): - """Test to ensure move_to get called on all members, even if some fail.""" - await self.cog._async_init() + @staticmethod + def create_erroneous_members() -> Tuple[List[MockMember], List[MockMember]]: + """ + Helper method to generate a list of members that error out on move_to call. + Returns the list of erroneous members, + as well as a list of regular and erroneous members combined, in that order. + """ def failing_move_to(*_): raise Exception() - failing_members = [MockMember(move_to=Mock(failing_move_to)) for _ in range(5)] + erroneous_members = [MockMember(move_to=Mock(failing_move_to)) for _ in range(5)] members = [] for i in range(5): members.append(MockMember()) - members.append(failing_members[i]) + members.append(erroneous_members[i]) - channel = MockVoiceChannel(members=members) + return erroneous_members, members + + async def test_kick_move_to_error(self): + """Test to ensure move_to gets called on all members during kick, even if some fail.""" + await self.cog._async_init() + failing_members, members = self.create_erroneous_members() - with self.subTest("Kick"): - await self.cog._kick_voice_members(channel) - for member in members: - member.move_to.assert_called_once() - member.reset_mock() + await self.cog._kick_voice_members(MockVoiceChannel(members=members)) + for member in members: + member.move_to.assert_called_once() + member.reset_mock() + + async def test_sync_move_to_error(self): + """Test to ensure move_to gets called on all members during sync, even if some fail.""" + await self.cog._async_init() + failing_members, members = self.create_erroneous_members() - with self.subTest("Sync"): - await self.cog._force_voice_sync(channel) - for member in members: - self.assertEqual(member.move_to.call_count, 1 if member in failing_members else 2) + await self.cog._force_voice_sync(MockVoiceChannel(members=members)) + for member in members: + self.assertEqual(member.move_to.call_count, 1 if member in failing_members else 2) @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) -- cgit v1.2.3 From acf2b643672703d2bc2011558e9fb68c76d0bc17 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Wed, 10 Mar 2021 20:36:36 +0300 Subject: Combine Silence Target Tests Combine two tests that are responsible for checking the silence helper uses the correct channel and message. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- bot/exts/moderation/silence.py | 2 +- tests/bot/exts/moderation/test_silence.py | 53 +++++++++---------------------- 2 files changed, 16 insertions(+), 39 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index dd379b412..616dfbefb 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -173,7 +173,7 @@ class Silence(commands.Cog): if not await self._set_silence_overwrites(channel, kick=kick): log.info(f"Tried to silence channel {channel_info} but the channel was already silenced.") - await self.send_message(MSG_SILENCE_FAIL, ctx.channel, channel) + await self.send_message(MSG_SILENCE_FAIL, ctx.channel, channel, alert_target=False) return if isinstance(channel, VoiceChannel): diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index a52f2447d..c3b30450f 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -375,51 +375,28 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.voice_channel.overwrites_for.return_value = self.voice_overwrite async def test_sent_correct_message(self): - """Appropriate failure/success message was sent by the command.""" + """Appropriate failure/success message was sent by the command to the correct channel.""" + # The following test tuples are made up of: + # duration, expected message, and the success of the _set_silence_overwrites function test_cases = ( (0.0001, silence.MSG_SILENCE_SUCCESS.format(duration=0.0001), True,), (None, silence.MSG_SILENCE_PERMANENT, True,), (5, silence.MSG_SILENCE_FAIL, False,), ) + for duration, message, was_silenced in test_cases: - ctx = MockContext() with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=was_silenced): - with self.subTest(was_silenced=was_silenced, message=message, duration=duration): - await self.cog.silence.callback(self.cog, ctx, duration) - ctx.channel.send.assert_called_once_with(message) - - @mock.patch.object(silence.Silence, "_set_silence_overwrites", return_value=True) - @mock.patch.object(silence.Silence, "_force_voice_sync") - async def test_sent_to_correct_channel(self, voice_sync, _): - """Test function sends messages to the correct channels.""" - text_channel = MockTextChannel() - voice_channel = MockVoiceChannel() - ctx = MockContext() - - test_cases = ( - (None, silence.MSG_SILENCE_SUCCESS.format(duration=10)), - (text_channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), - (voice_channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), - (ctx.channel, silence.MSG_SILENCE_SUCCESS.format(duration=10)), - ) - - for target, message in test_cases: - with self.subTest(target_channel=target, message=message): - await self.cog.silence.callback(self.cog, ctx, 10, target, kick=False) - - if ctx.channel == target or target is None: - ctx.channel.send.assert_called_once_with(message) - - else: - ctx.channel.send.assert_called_once_with(message.replace("current channel", target.mention)) - if isinstance(target, MockTextChannel): - target.send.assert_called_once_with(message) - else: - voice_sync.assert_called_once_with(target) - - ctx.channel.send.reset_mock() - if target is not None and isinstance(target, MockTextChannel): - target.send.reset_mock() + for target in [MockTextChannel(), MockVoiceChannel(), None]: + with self.subTest(was_silenced=was_silenced, target=target, message=message): + with mock.patch.object(self.cog, "send_message") as send_message: + ctx = MockContext() + await self.cog.silence.callback(self.cog, ctx, duration, target) + send_message.assert_called_once_with( + message, + ctx.channel, + target or ctx.channel, + alert_target=was_silenced + ) @voice_sync_helper async def test_sync_called(self, ctx, sync, kick): -- cgit v1.2.3 From e62707dfaf2dd833ff057ee472472d27a86ac223 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Wed, 10 Mar 2021 20:55:24 +0300 Subject: Simplifies Redundant Unsilence Target Test Removes redundant functionality from the `test_unsilence_helper_fail` test as it is covered by another test. Keeps the functionality that isn't being tested elsewhere. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 41 ++++++------------------------- 1 file changed, 8 insertions(+), 33 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index c3b30450f..5f2e67ac2 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -688,42 +688,17 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): self.assertDictEqual(prev_overwrite_dict, new_overwrite_dict) - @mock.patch.object(silence.Silence, "_unsilence", return_value=False) - @mock.patch.object(silence.Silence, "send_message") - async def test_unsilence_helper_fail(self, send_message, _): - """Tests unsilence_wrapper when `_unsilence` fails.""" - ctx = MockContext() - - text_channel = MockTextChannel() - text_role = self.cog.bot.get_guild(Guild.id).default_role - - voice_channel = MockVoiceChannel() - voice_role = self.cog.bot.get_guild(Guild.id).get_role(Roles.voice_verified) - + async def test_unsilence_role(self): + """Tests unsilence_wrapper applies permission to the correct role.""" test_cases = ( - (ctx, text_channel, text_role, True, silence.MSG_UNSILENCE_FAIL), - (ctx, text_channel, text_role, False, silence.MSG_UNSILENCE_MANUAL), - (ctx, voice_channel, voice_role, True, silence.MSG_UNSILENCE_FAIL), - (ctx, voice_channel, voice_role, False, silence.MSG_UNSILENCE_MANUAL), + (MockTextChannel(), self.cog.bot.get_guild(Guild.id).default_role), + (MockVoiceChannel(), self.cog.bot.get_guild(Guild.id).get_role(Roles.voice_verified)) ) - class PermClass: - """Class to Mock return permissions""" - def __init__(self, value: bool): - self.val = value - - def __getattr__(self, item): - return self.val - - for context, channel, role, permission, message in test_cases: - with mock.patch.object(channel, "overwrites_for", return_value=PermClass(permission)) as overwrites: - with self.subTest(channel=channel, message=message): - await self.cog._unsilence_wrapper(channel, context) - - overwrites.assert_called_once_with(role) - send_message.assert_called_once_with(message, ctx.channel, channel, alert_target=False) - - send_message.reset_mock() + for channel, role in test_cases: + with self.subTest(channel=channel, role=role): + await self.cog._unsilence_wrapper(channel, MockContext()) + channel.overwrites_for.assert_called_with(role) @mock.patch.object(silence.Silence, "_force_voice_sync") @mock.patch.object(silence.Silence, "send_message") -- cgit v1.2.3 From b867fb21b6a14f91aa608009aa7eef540a4792dc Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Wed, 10 Mar 2021 22:38:59 +0300 Subject: Use Mock Side Effect Instead Of Extra Function Changes the mock used for creating an erroneous function in the silence tests cog to use the side effect property instead of an extra function. Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 5f2e67ac2..a297cc8cb 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -233,9 +233,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): Returns the list of erroneous members, as well as a list of regular and erroneous members combined, in that order. """ - def failing_move_to(*_): - raise Exception() - erroneous_members = [MockMember(move_to=Mock(failing_move_to)) for _ in range(5)] + erroneous_members = [MockMember(move_to=Mock(side_effect=Exception())) for _ in range(5)] members = [] for i in range(5): -- cgit v1.2.3 From b0381d7164ceba20784fc244d724962ec35a9b13 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> Date: Fri, 12 Mar 2021 22:58:28 +0300 Subject: Removes Unused Mock Reset Signed-off-by: Hassan Abouelela <47495861+HassanAbouelela@users.noreply.github.com> --- tests/bot/exts/moderation/test_silence.py | 1 - 1 file changed, 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index a297cc8cb..d7542c562 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -250,7 +250,6 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): await self.cog._kick_voice_members(MockVoiceChannel(members=members)) for member in members: member.move_to.assert_called_once() - member.reset_mock() async def test_sync_move_to_error(self): """Test to ensure move_to gets called on all members during sync, even if some fail.""" -- cgit v1.2.3 From 2446c1728c3ba660d33b686d93d62a93debc74d2 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Wed, 21 Apr 2021 00:25:51 +0300 Subject: Removes Unnecessary Members In Silence Tests Reduces the number of members created for each test to the bare minimum required. Signed-off-by: Hassan Abouelela --- tests/bot/exts/moderation/test_silence.py | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index d7542c562..459048f68 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -173,14 +173,15 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): """Tests the _force_voice_sync helper function.""" await self.cog._async_init() - members = [MockMember() for _ in range(10)] - members.extend([MockMember(roles=[MockRole(id=role)]) for role in MODERATION_ROLES]) + # Create a regular member, and one member for each of the moderation roles + moderation_members = [MockMember(roles=[MockRole(id=role)]) for role in MODERATION_ROLES] + members = [MockMember(), *moderation_members] channel = MockVoiceChannel(members=members) await self.cog._force_voice_sync(channel) for member in members: - if any(role.id in MODERATION_ROLES for role in member.roles): + if member in moderation_members: member.move_to.assert_not_called() else: self.assertEqual(member.move_to.call_count, 2) @@ -213,14 +214,15 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): """Test to ensure kick function can remove all members from a voice channel.""" await self.cog._async_init() - members = [MockMember() for _ in range(10)] - members.extend([MockMember(roles=[MockRole(id=role)]) for role in MODERATION_ROLES]) + # Create a regular member, and one member for each of the moderation roles + moderation_members = [MockMember(roles=[MockRole(id=role)]) for role in MODERATION_ROLES] + members = [MockMember(), *moderation_members] channel = MockVoiceChannel(members=members) await self.cog._kick_voice_members(channel) for member in members: - if any(role.id in MODERATION_ROLES for role in member.roles): + if member in moderation_members: member.move_to.assert_not_called() else: self.assertEqual((None,), member.move_to.call_args_list[0].args) @@ -233,19 +235,15 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): Returns the list of erroneous members, as well as a list of regular and erroneous members combined, in that order. """ - erroneous_members = [MockMember(move_to=Mock(side_effect=Exception())) for _ in range(5)] + erroneous_member = MockMember(move_to=Mock(side_effect=Exception())) + members = [MockMember(), erroneous_member] - members = [] - for i in range(5): - members.append(MockMember()) - members.append(erroneous_members[i]) - - return erroneous_members, members + return erroneous_member, members async def test_kick_move_to_error(self): """Test to ensure move_to gets called on all members during kick, even if some fail.""" await self.cog._async_init() - failing_members, members = self.create_erroneous_members() + _, members = self.create_erroneous_members() await self.cog._kick_voice_members(MockVoiceChannel(members=members)) for member in members: @@ -254,11 +252,11 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): async def test_sync_move_to_error(self): """Test to ensure move_to gets called on all members during sync, even if some fail.""" await self.cog._async_init() - failing_members, members = self.create_erroneous_members() + failing_member, members = self.create_erroneous_members() await self.cog._force_voice_sync(MockVoiceChannel(members=members)) for member in members: - self.assertEqual(member.move_to.call_count, 1 if member in failing_members else 2) + self.assertEqual(member.move_to.call_count, 1 if member == failing_member else 2) @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) -- cgit v1.2.3 From da694fde0f78813a2787f371b8da84c39b72bcd9 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 24 Apr 2021 01:10:13 +0300 Subject: Uses Async Asserts Where Possible Signed-off-by: Hassan Abouelela --- tests/bot/exts/moderation/test_silence.py | 42 +++++++++++++++---------------- 1 file changed, 21 insertions(+), 21 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 459048f68..ce76dc945 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -205,10 +205,10 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): overwrites = { channel.guild.default_role: PermissionOverwrite(speak=False, connect=False, view_channel=False) } - channel.guild.create_voice_channel.assert_called_once_with("mute-temp", overwrites=overwrites) + channel.guild.create_voice_channel.assert_awaited_once_with("mute-temp", overwrites=overwrites) # Check bot deleted channel - new_channel.delete.assert_called_once() + new_channel.delete.assert_awaited_once() async def test_voice_kick(self): """Test to ensure kick function can remove all members from a voice channel.""" @@ -235,7 +235,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): Returns the list of erroneous members, as well as a list of regular and erroneous members combined, in that order. """ - erroneous_member = MockMember(move_to=Mock(side_effect=Exception())) + erroneous_member = MockMember(move_to=AsyncMock(side_effect=Exception())) members = [MockMember(), erroneous_member] return erroneous_member, members @@ -247,7 +247,7 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): await self.cog._kick_voice_members(MockVoiceChannel(members=members)) for member in members: - member.move_to.assert_called_once() + member.move_to.assert_awaited_once() async def test_sync_move_to_error(self): """Test to ensure move_to gets called on all members during sync, even if some fail.""" @@ -399,7 +399,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockVoiceChannel() await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=False) - sync.assert_called_once_with(self.cog, channel) + sync.assert_awaited_once_with(self.cog, channel) kick.assert_not_called() @voice_sync_helper @@ -408,7 +408,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): channel = MockVoiceChannel() await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=True) - kick.assert_called_once_with(channel) + kick.assert_awaited_once_with(channel) sync.assert_not_called() @voice_sync_helper @@ -510,7 +510,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): """Channel's previous overwrites were cached.""" overwrite_json = '{"send_messages": true, "add_reactions": false}' await self.cog._set_silence_overwrites(self.text_channel) - self.cog.previous_overwrites.set.assert_called_once_with(self.text_channel.id, overwrite_json) + self.cog.previous_overwrites.set.assert_awaited_once_with(self.text_channel.id, overwrite_json) @autospec(silence, "datetime") async def test_cached_unsilence_time(self, datetime_mock): @@ -597,7 +597,7 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog.unsilence.callback(self.cog, ctx, channel=target) call_args = (message, ctx.channel, target or ctx.channel) - send_message.assert_called_once_with(*call_args, alert_target=was_unsilenced) + send_message.assert_awaited_once_with(*call_args, alert_target=was_unsilenced) async def test_skipped_already_unsilenced(self): """Permissions were not set and `False` was returned for an already unsilenced channel.""" @@ -731,11 +731,11 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._unsilence_wrapper(channel, context) if context is None: - send_message.assert_called_once_with(message, channel, channel, alert_target=True) + send_message.assert_awaited_once_with(message, channel, channel, alert_target=True) else: - send_message.assert_called_once_with(message, context.channel, channel, alert_target=True) + send_message.assert_awaited_once_with(message, context.channel, channel, alert_target=True) - channel.set_permissions.assert_called_once_with(role, overwrite=overwrites) + channel.set_permissions.assert_awaited_once_with(role, overwrite=overwrites) if channel != ctx.channel: ctx.channel.send.assert_not_called() @@ -759,7 +759,7 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): message = "Test basic message." await self.cog.send_message(message, *self.text_channels, alert_target=False) - self.text_channels[0].send.assert_called_once_with(message) + self.text_channels[0].send.assert_awaited_once_with(message) self.text_channels[1].send.assert_not_called() async def test_send_to_multiple_channels(self): @@ -767,8 +767,8 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): message = "Test basic message." await self.cog.send_message(message, *self.text_channels, alert_target=True) - self.text_channels[0].send.assert_called_once_with(message) - self.text_channels[1].send.assert_called_once_with(message) + self.text_channels[0].send.assert_awaited_once_with(message) + self.text_channels[1].send.assert_awaited_once_with(message) async def test_duration_replacement(self): """Tests that the channel name was set correctly for one target channel.""" @@ -776,7 +776,7 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): await self.cog.send_message(message, *self.text_channels, alert_target=False) updated_message = message.replace("current channel", self.text_channels[0].mention) - self.text_channels[0].send.assert_called_once_with(updated_message) + self.text_channels[0].send.assert_awaited_once_with(updated_message) self.text_channels[1].send.assert_not_called() async def test_name_replacement_multiple_channels(self): @@ -785,14 +785,14 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): await self.cog.send_message(message, *self.text_channels, alert_target=True) updated_message = message.replace("current channel", self.text_channels[0].mention) - self.text_channels[0].send.assert_called_once_with(updated_message) - self.text_channels[1].send.assert_called_once_with(message) + self.text_channels[0].send.assert_awaited_once_with(updated_message) + self.text_channels[1].send.assert_awaited_once_with(message) async def test_silence_voice(self): """Tests that the correct message was sent when a voice channel is muted without alerting.""" message = "This should show up just here." await self.cog.send_message(message, self.text_channels[0], self.voice_channel, alert_target=False) - self.text_channels[0].send.assert_called_once_with(message) + self.text_channels[0].send.assert_awaited_once_with(message) self.text_channels[1].send.assert_not_called() async def test_silence_voice_alert(self): @@ -804,8 +804,8 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): await self.cog.send_message(message, self.text_channels[0], self.voice_channel, alert_target=True) updated_message = message.replace("current channel", self.voice_channel.mention) - self.text_channels[0].send.assert_called_once_with(updated_message) - self.text_channels[1].send.assert_called_once_with(updated_message) + self.text_channels[0].send.assert_awaited_once_with(updated_message) + self.text_channels[1].send.assert_awaited_once_with(updated_message) mock_voice_channels.get.assert_called_once_with(self.voice_channel.id) @@ -818,7 +818,7 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): await self.cog.send_message(message, self.text_channels[1], self.voice_channel, alert_target=True) updated_message = message.replace("current channel", self.voice_channel.mention) - self.text_channels[1].send.assert_called_once_with(updated_message) + self.text_channels[1].send.assert_awaited_once_with(updated_message) mock_voice_channels.get.assert_called_once_with(self.voice_channel.id) self.bot.get_channel.assert_called_once_with(self.text_channels[1].id) -- cgit v1.2.3 From db5b0751fb9531525e12e6d421aa7d0772b3054a Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 4 May 2021 04:37:49 +0300 Subject: Copy Existing Text Channel Cache Tests For Voice Duplicates existing silence and unsilence cache tests for voice channels. Signed-off-by: Hassan Abouelela --- tests/bot/exts/moderation/test_silence.py | 179 ++++++++++++++++-------------- 1 file changed, 97 insertions(+), 82 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index ce76dc945..347471c13 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -362,11 +362,11 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): asyncio.run(self.cog._async_init()) # Populate instance attributes. self.text_channel = MockTextChannel() - self.text_overwrite = PermissionOverwrite(stream=True, send_messages=True, add_reactions=False) + self.text_overwrite = PermissionOverwrite(send_messages=True, add_reactions=False) self.text_channel.overwrites_for.return_value = self.text_overwrite self.voice_channel = MockVoiceChannel() - self.voice_overwrite = PermissionOverwrite(speak=True) + self.voice_overwrite = PermissionOverwrite(connect=True, speak=True) self.voice_channel.overwrites_for.return_value = self.voice_overwrite async def test_sent_correct_message(self): @@ -474,8 +474,8 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): overwrite=self.voice_overwrite ) - async def test_preserved_other_overwrites(self): - """Channel's other unrelated overwrites were not changed.""" + async def test_preserved_other_overwrites_text(self): + """Channel's other unrelated overwrites were not changed for a text channel mute.""" prev_overwrite_dict = dict(self.text_overwrite) await self.cog._set_silence_overwrites(self.text_channel) new_overwrite_dict = dict(self.text_overwrite) @@ -488,6 +488,20 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.assertDictEqual(prev_overwrite_dict, new_overwrite_dict) + async def test_preserved_other_overwrites_voice(self): + """Channel's other unrelated overwrites were not changed for a voice channel mute.""" + prev_overwrite_dict = dict(self.voice_overwrite) + await self.cog._set_silence_overwrites(self.voice_channel) + new_overwrite_dict = dict(self.voice_overwrite) + + # Remove 'connect' & 'speak' keys because they were changed by the method. + del prev_overwrite_dict['connect'] + del prev_overwrite_dict['speak'] + del new_overwrite_dict['connect'] + del new_overwrite_dict['speak'] + + self.assertDictEqual(prev_overwrite_dict, new_overwrite_dict) + async def test_temp_not_added_to_notifier(self): """Channel was not added to notifier if a duration was set for the silence.""" with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): @@ -568,9 +582,13 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): self.cog.scheduler.__contains__.return_value = True overwrites_cache.get.return_value = '{"send_messages": true, "add_reactions": false}' - self.channel = MockTextChannel() - self.overwrite = PermissionOverwrite(stream=True, send_messages=False, add_reactions=False) - self.channel.overwrites_for.return_value = self.overwrite + self.text_channel = MockTextChannel() + self.text_overwrite = PermissionOverwrite(send_messages=False, add_reactions=False) + self.text_channel.overwrites_for.return_value = self.text_overwrite + + self.voice_channel = MockVoiceChannel() + self.voice_overwrite = PermissionOverwrite(connect=True, speak=True) + self.voice_channel.overwrites_for.return_value = self.voice_overwrite async def test_sent_correct_message(self): """Appropriate failure/success message was sent by the command.""" @@ -578,7 +596,7 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): test_cases = ( (True, silence.MSG_UNSILENCE_SUCCESS, unsilenced_overwrite), (False, silence.MSG_UNSILENCE_FAIL, unsilenced_overwrite), - (False, silence.MSG_UNSILENCE_MANUAL, self.overwrite), + (False, silence.MSG_UNSILENCE_MANUAL, self.text_overwrite), (False, silence.MSG_UNSILENCE_MANUAL, PermissionOverwrite(send_messages=False)), (False, silence.MSG_UNSILENCE_MANUAL, PermissionOverwrite(add_reactions=False)), ) @@ -608,35 +626,60 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): self.assertFalse(await self.cog._unsilence(channel)) channel.set_permissions.assert_not_called() - async def test_restored_overwrites(self): - """Channel's `send_message` and `add_reactions` overwrites were restored.""" - await self.cog._unsilence(self.channel) - self.channel.set_permissions.assert_awaited_once_with( + async def test_restored_overwrites_text(self): + """Text channel's `send_message` and `add_reactions` overwrites were restored.""" + await self.cog._unsilence(self.text_channel) + self.text_channel.set_permissions.assert_awaited_once_with( self.cog._everyone_role, - overwrite=self.overwrite, + overwrite=self.text_overwrite, + ) + + # Recall that these values are determined by the fixture. + self.assertTrue(self.text_overwrite.send_messages) + self.assertFalse(self.text_overwrite.add_reactions) + + async def test_restored_overwrites_voice(self): + """Voice channel's `connect` and `speak` overwrites were restored.""" + await self.cog._unsilence(self.voice_channel) + self.voice_channel.set_permissions.assert_awaited_once_with( + self.cog._verified_voice_role, + overwrite=self.voice_overwrite, ) # Recall that these values are determined by the fixture. - self.assertTrue(self.overwrite.send_messages) - self.assertFalse(self.overwrite.add_reactions) + self.assertTrue(self.voice_overwrite.connect) + self.assertTrue(self.voice_overwrite.speak) - async def test_cache_miss_used_default_overwrites(self): - """Both overwrites were set to None due previous values not being found in the cache.""" + async def test_cache_miss_used_default_overwrites_text(self): + """Text overwrites were set to None due previous values not being found in the cache.""" self.cog.previous_overwrites.get.return_value = None - await self.cog._unsilence(self.channel) - self.channel.set_permissions.assert_awaited_once_with( + await self.cog._unsilence(self.text_channel) + self.text_channel.set_permissions.assert_awaited_once_with( self.cog._everyone_role, - overwrite=self.overwrite, + overwrite=self.text_overwrite, ) - self.assertIsNone(self.overwrite.send_messages) - self.assertIsNone(self.overwrite.add_reactions) + self.assertIsNone(self.text_overwrite.send_messages) + self.assertIsNone(self.text_overwrite.add_reactions) + + async def test_cache_miss_used_default_overwrites_voice(self): + """Voice overwrites were set to None due previous values not being found in the cache.""" + self.cog.previous_overwrites.get.return_value = None + + await self.cog._unsilence(self.voice_channel) + self.voice_channel.set_permissions.assert_awaited_once_with( + self.cog._verified_voice_role, + overwrite=self.voice_overwrite, + ) + + self.assertIsNone(self.voice_overwrite.connect) + self.assertIsNone(self.voice_overwrite.speak) async def test_cache_miss_sent_mod_alert_text(self): """A message was sent to the mod alerts channel upon muting a text channel.""" self.cog.previous_overwrites.get.return_value = None - await self.cog._unsilence(self.channel) + await self.cog._unsilence(self.text_channel) self.cog._mod_alerts_channel.send.assert_awaited_once() async def test_cache_miss_sent_mod_alert_voice(self): @@ -647,33 +690,33 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): async def test_removed_notifier(self): """Channel was removed from `notifier`.""" - await self.cog._unsilence(self.channel) - self.cog.notifier.remove_channel.assert_called_once_with(self.channel) + await self.cog._unsilence(self.text_channel) + self.cog.notifier.remove_channel.assert_called_once_with(self.text_channel) async def test_deleted_cached_overwrite(self): """Channel was deleted from the overwrites cache.""" - await self.cog._unsilence(self.channel) - self.cog.previous_overwrites.delete.assert_awaited_once_with(self.channel.id) + await self.cog._unsilence(self.text_channel) + self.cog.previous_overwrites.delete.assert_awaited_once_with(self.text_channel.id) async def test_deleted_cached_time(self): """Channel was deleted from the timestamp cache.""" - await self.cog._unsilence(self.channel) - self.cog.unsilence_timestamps.delete.assert_awaited_once_with(self.channel.id) + await self.cog._unsilence(self.text_channel) + self.cog.unsilence_timestamps.delete.assert_awaited_once_with(self.text_channel.id) async def test_cancelled_task(self): """The scheduled unsilence task should be cancelled.""" - await self.cog._unsilence(self.channel) - self.cog.scheduler.cancel.assert_called_once_with(self.channel.id) + await self.cog._unsilence(self.text_channel) + self.cog.scheduler.cancel.assert_called_once_with(self.text_channel.id) - async def test_preserved_other_overwrites(self): - """Channel's other unrelated overwrites were not changed, including cache misses.""" + async def test_preserved_other_overwrites_text(self): + """Text channel's other unrelated overwrites were not changed, including cache misses.""" for overwrite_json in ('{"send_messages": true, "add_reactions": null}', None): with self.subTest(overwrite_json=overwrite_json): self.cog.previous_overwrites.get.return_value = overwrite_json - prev_overwrite_dict = dict(self.overwrite) - await self.cog._unsilence(self.channel) - new_overwrite_dict = dict(self.overwrite) + prev_overwrite_dict = dict(self.text_overwrite) + await self.cog._unsilence(self.text_channel) + new_overwrite_dict = dict(self.text_overwrite) # Remove these keys because they were modified by the unsilence. del prev_overwrite_dict['send_messages'] @@ -683,6 +726,24 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): self.assertDictEqual(prev_overwrite_dict, new_overwrite_dict) + async def test_preserved_other_overwrites_voice(self): + """Voice channel's other unrelated overwrites were not changed, including cache misses.""" + for overwrite_json in ('{"connect": true, "speak": true}', None): + with self.subTest(overwrite_json=overwrite_json): + self.cog.previous_overwrites.get.return_value = overwrite_json + + prev_overwrite_dict = dict(self.voice_overwrite) + await self.cog._unsilence(self.voice_channel) + new_overwrite_dict = dict(self.voice_overwrite) + + # Remove these keys because they were modified by the unsilence. + del prev_overwrite_dict['connect'] + del prev_overwrite_dict['speak'] + del new_overwrite_dict['connect'] + del new_overwrite_dict['speak'] + + self.assertDictEqual(prev_overwrite_dict, new_overwrite_dict) + async def test_unsilence_role(self): """Tests unsilence_wrapper applies permission to the correct role.""" test_cases = ( @@ -695,52 +756,6 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog._unsilence_wrapper(channel, MockContext()) channel.overwrites_for.assert_called_with(role) - @mock.patch.object(silence.Silence, "_force_voice_sync") - @mock.patch.object(silence.Silence, "send_message") - async def test_correct_overwrites(self, send_message, _): - """Tests the overwrites returned by the _unsilence_wrapper are correct for voice and text channels.""" - ctx = MockContext() - - text_channel = MockTextChannel() - text_role = self.cog.bot.get_guild(Guild.id).default_role - - voice_channel = MockVoiceChannel() - voice_role = self.cog.bot.get_guild(Guild.id).get_role(Roles.voice_verified) - - async def reset(): - await text_channel.set_permissions(text_role, PermissionOverwrite(send_messages=False, add_reactions=False)) - await voice_channel.set_permissions(voice_role, PermissionOverwrite(speak=False, connect=False)) - - text_channel.reset_mock() - voice_channel.reset_mock() - send_message.reset_mock() - await reset() - - default_text_overwrites = text_channel.overwrites_for(text_role) - default_voice_overwrites = voice_channel.overwrites_for(voice_role) - - test_cases = ( - (ctx, text_channel, text_role, default_text_overwrites, silence.MSG_UNSILENCE_SUCCESS), - (ctx, voice_channel, voice_role, default_voice_overwrites, silence.MSG_UNSILENCE_SUCCESS), - (ctx, ctx.channel, text_role, ctx.channel.overwrites_for(text_role), silence.MSG_UNSILENCE_SUCCESS), - (None, text_channel, text_role, default_text_overwrites, silence.MSG_UNSILENCE_SUCCESS), - ) - - for context, channel, role, overwrites, message in test_cases: - with self.subTest(ctx=context, channel=channel): - await self.cog._unsilence_wrapper(channel, context) - - if context is None: - send_message.assert_awaited_once_with(message, channel, channel, alert_target=True) - else: - send_message.assert_awaited_once_with(message, context.channel, channel, alert_target=True) - - channel.set_permissions.assert_awaited_once_with(role, overwrite=overwrites) - if channel != ctx.channel: - ctx.channel.send.assert_not_called() - - await reset() - class SendMessageTests(unittest.IsolatedAsyncioTestCase): """Unittests for the send message helper function.""" -- cgit v1.2.3 From bac68d1d584b398f2bc5cc7a8f3df39ed48174ae Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 4 May 2021 04:47:15 +0300 Subject: Adds Voice Test Cases To Already Silenced Test Signed-off-by: Hassan Abouelela --- tests/bot/exts/moderation/test_silence.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 347471c13..0d135698e 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -432,15 +432,17 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_skipped_already_silenced(self): """Permissions were not set and `False` was returned for an already silenced channel.""" subtests = ( - (False, PermissionOverwrite(send_messages=False, add_reactions=False)), - (True, PermissionOverwrite(send_messages=True, add_reactions=True)), - (True, PermissionOverwrite(send_messages=False, add_reactions=False)), + (False, MockTextChannel(), PermissionOverwrite(send_messages=False, add_reactions=False)), + (True, MockTextChannel(), PermissionOverwrite(send_messages=True, add_reactions=True)), + (True, MockTextChannel(), PermissionOverwrite(send_messages=False, add_reactions=False)), + (False, MockVoiceChannel(), PermissionOverwrite(connect=False, speak=False)), + (True, MockVoiceChannel(), PermissionOverwrite(connect=True, speak=True)), + (True, MockVoiceChannel(), PermissionOverwrite(connect=False, speak=False)), ) - for contains, overwrite in subtests: - with self.subTest(contains=contains, overwrite=overwrite): + for contains, channel, overwrite in subtests: + with self.subTest(contains=contains, is_text=isinstance(channel, MockTextChannel), overwrite=overwrite): self.cog.scheduler.__contains__.return_value = contains - channel = MockTextChannel() channel.overwrites_for.return_value = overwrite self.assertFalse(await self.cog._set_silence_overwrites(channel)) -- cgit v1.2.3 From 4bf9a7a545e2ffb507fbb379df1695755a2eea1b Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Tue, 4 May 2021 05:09:04 +0300 Subject: Adds Missing Voice Version Of Tests Signed-off-by: Hassan Abouelela --- tests/bot/exts/moderation/test_silence.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 0d135698e..729b28412 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -623,10 +623,11 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): """Permissions were not set and `False` was returned for an already unsilenced channel.""" self.cog.scheduler.__contains__.return_value = False self.cog.previous_overwrites.get.return_value = None - channel = MockTextChannel() - self.assertFalse(await self.cog._unsilence(channel)) - channel.set_permissions.assert_not_called() + for channel in (MockVoiceChannel(), MockTextChannel()): + with self.subTest(channel=channel): + self.assertFalse(await self.cog._unsilence(channel)) + channel.set_permissions.assert_not_called() async def test_restored_overwrites_text(self): """Text channel's `send_message` and `add_reactions` overwrites were restored.""" -- cgit v1.2.3 From 2722352236bcad411bd6417e7009e43625a7da9a Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sun, 9 May 2021 00:32:13 +0300 Subject: Uses Itertools Product To Reduce Nesting Uses itertools.product to eliminate some nested for loops in tests. Signed-off-by: Hassan Abouelela --- tests/bot/exts/moderation/test_silence.py | 50 ++++++++++++++++--------------- 1 file changed, 26 insertions(+), 24 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 729b28412..de7230ae5 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -1,4 +1,5 @@ import asyncio +import itertools import unittest from datetime import datetime, timezone from typing import List, Tuple @@ -379,19 +380,20 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): (5, silence.MSG_SILENCE_FAIL, False,), ) - for duration, message, was_silenced in test_cases: + targets = (MockTextChannel(), MockVoiceChannel(), None) + + for (duration, message, was_silenced), target in itertools.product(test_cases, targets): with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=was_silenced): - for target in [MockTextChannel(), MockVoiceChannel(), None]: - with self.subTest(was_silenced=was_silenced, target=target, message=message): - with mock.patch.object(self.cog, "send_message") as send_message: - ctx = MockContext() - await self.cog.silence.callback(self.cog, ctx, duration, target) - send_message.assert_called_once_with( - message, - ctx.channel, - target or ctx.channel, - alert_target=was_silenced - ) + with self.subTest(was_silenced=was_silenced, target=target, message=message): + with mock.patch.object(self.cog, "send_message") as send_message: + ctx = MockContext() + await self.cog.silence.callback(self.cog, ctx, duration, target) + send_message.assert_called_once_with( + message, + ctx.channel, + target or ctx.channel, + alert_target=was_silenced + ) @voice_sync_helper async def test_sync_called(self, ctx, sync, kick): @@ -603,21 +605,21 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): (False, silence.MSG_UNSILENCE_MANUAL, PermissionOverwrite(add_reactions=False)), ) - for was_unsilenced, message, overwrite in test_cases: - ctx = MockContext() + targets = (None, MockTextChannel()) - for target in [None, MockTextChannel()]: - ctx.channel.overwrites_for.return_value = overwrite - if target: - target.overwrites_for.return_value = overwrite + for (was_unsilenced, message, overwrite), target in itertools.product(test_cases, targets): + ctx = MockContext() + ctx.channel.overwrites_for.return_value = overwrite + if target: + target.overwrites_for.return_value = overwrite - with mock.patch.object(self.cog, "_unsilence", return_value=was_unsilenced): - with mock.patch.object(self.cog, "send_message") as send_message: - with self.subTest(was_unsilenced=was_unsilenced, overwrite=overwrite, target=target): - await self.cog.unsilence.callback(self.cog, ctx, channel=target) + with mock.patch.object(self.cog, "_unsilence", return_value=was_unsilenced): + with mock.patch.object(self.cog, "send_message") as send_message: + with self.subTest(was_unsilenced=was_unsilenced, overwrite=overwrite, target=target): + await self.cog.unsilence.callback(self.cog, ctx, channel=target) - call_args = (message, ctx.channel, target or ctx.channel) - send_message.assert_awaited_once_with(*call_args, alert_target=was_unsilenced) + call_args = (message, ctx.channel, target or ctx.channel) + send_message.assert_awaited_once_with(*call_args, alert_target=was_unsilenced) async def test_skipped_already_unsilenced(self): """Permissions were not set and `False` was returned for an already unsilenced channel.""" -- cgit v1.2.3 From 2f772082f288671354b6bbd71d9a9caad6fe87af Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Wed, 12 May 2021 00:03:26 +0300 Subject: Updates Silence To Use `.format` Uses `.format` to create silence and unsilence messages instead of `.replace`. Signed-off-by: Hassan Abouelela --- bot/exts/moderation/silence.py | 20 +++++++++++--------- tests/bot/exts/moderation/test_silence.py | 19 +++++++++---------- 2 files changed, 20 insertions(+), 19 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 2012d75d9..e5c96e76f 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -19,17 +19,17 @@ log = logging.getLogger(__name__) LOCK_NAMESPACE = "silence" -MSG_SILENCE_FAIL = f"{constants.Emojis.cross_mark} current channel is already silenced." -MSG_SILENCE_PERMANENT = f"{constants.Emojis.check_mark} silenced current channel indefinitely." -MSG_SILENCE_SUCCESS = f"{constants.Emojis.check_mark} silenced current channel for {{duration}} minute(s)." +MSG_SILENCE_FAIL = f"{constants.Emojis.cross_mark} {{channel}} is already silenced." +MSG_SILENCE_PERMANENT = f"{constants.Emojis.check_mark} silenced {{channel}} indefinitely." +MSG_SILENCE_SUCCESS = f"{constants.Emojis.check_mark} silenced {{{{channel}}}} for {{duration}} minute(s)." -MSG_UNSILENCE_FAIL = f"{constants.Emojis.cross_mark} current channel was not silenced." +MSG_UNSILENCE_FAIL = f"{constants.Emojis.cross_mark} {{channel}} was not silenced." MSG_UNSILENCE_MANUAL = ( - f"{constants.Emojis.cross_mark} current channel was not unsilenced because the current overwrites were " + f"{constants.Emojis.cross_mark} {{channel}} was not unsilenced because the current overwrites were " f"set manually or the cache was prematurely cleared. " f"Please edit the overwrites manually to unsilence." ) -MSG_UNSILENCE_SUCCESS = f"{constants.Emojis.check_mark} unsilenced current channel." +MSG_UNSILENCE_SUCCESS = f"{constants.Emojis.check_mark} unsilenced {{channel}}." TextOrVoiceChannel = Union[TextChannel, VoiceChannel] @@ -135,7 +135,9 @@ class Silence(commands.Cog): # Reply to invocation channel source_reply = message if source_channel != target_channel: - source_reply = source_reply.replace("current channel", target_channel.mention) + source_reply = source_reply.format(channel=target_channel.mention) + else: + source_reply = source_reply.format(channel="current channel") await source_channel.send(source_reply) # Reply to target channel @@ -143,10 +145,10 @@ class Silence(commands.Cog): if isinstance(target_channel, VoiceChannel): voice_chat = self.bot.get_channel(VOICE_CHANNELS.get(target_channel.id)) if voice_chat and source_channel != voice_chat: - await voice_chat.send(message.replace("current channel", target_channel.mention)) + await voice_chat.send(message.format(channel=target_channel.mention)) elif source_channel != target_channel: - await target_channel.send(message) + await target_channel.send(message.format(channel="current channel")) @commands.command(aliases=("hush",)) @lock(LOCK_NAMESPACE, _select_lock_channel, raise_error=True) diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index de7230ae5..af6dd5a37 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -792,21 +792,20 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): async def test_duration_replacement(self): """Tests that the channel name was set correctly for one target channel.""" - message = "Current. The following should be replaced: current channel." + message = "Current. The following should be replaced: {channel}." await self.cog.send_message(message, *self.text_channels, alert_target=False) - updated_message = message.replace("current channel", self.text_channels[0].mention) + updated_message = message.format(channel=self.text_channels[0].mention) self.text_channels[0].send.assert_awaited_once_with(updated_message) self.text_channels[1].send.assert_not_called() async def test_name_replacement_multiple_channels(self): """Tests that the channel name was set correctly for two channels.""" - message = "Current. The following should be replaced: current channel." + message = "Current. The following should be replaced: {channel}." await self.cog.send_message(message, *self.text_channels, alert_target=True) - updated_message = message.replace("current channel", self.text_channels[0].mention) - self.text_channels[0].send.assert_awaited_once_with(updated_message) - self.text_channels[1].send.assert_awaited_once_with(message) + self.text_channels[0].send.assert_awaited_once_with(message.format(channel=self.text_channels[0].mention)) + self.text_channels[1].send.assert_awaited_once_with(message.format(channel="current channel")) async def test_silence_voice(self): """Tests that the correct message was sent when a voice channel is muted without alerting.""" @@ -820,10 +819,10 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): with unittest.mock.patch.object(silence, "VOICE_CHANNELS") as mock_voice_channels: mock_voice_channels.get.return_value = self.text_channels[1].id - message = "This should show up as current channel." + message = "This should show up as {channel}." await self.cog.send_message(message, self.text_channels[0], self.voice_channel, alert_target=True) - updated_message = message.replace("current channel", self.voice_channel.mention) + updated_message = message.format(channel=self.voice_channel.mention) self.text_channels[0].send.assert_awaited_once_with(updated_message) self.text_channels[1].send.assert_awaited_once_with(updated_message) @@ -834,10 +833,10 @@ class SendMessageTests(unittest.IsolatedAsyncioTestCase): with unittest.mock.patch.object(silence, "VOICE_CHANNELS") as mock_voice_channels: mock_voice_channels.get.return_value = self.text_channels[1].id - message = "This should show up as current channel." + message = "This should show up as {channel}." await self.cog.send_message(message, self.text_channels[1], self.voice_channel, alert_target=True) - updated_message = message.replace("current channel", self.voice_channel.mention) + updated_message = message.format(channel=self.voice_channel.mention) self.text_channels[1].send.assert_awaited_once_with(updated_message) mock_voice_channels.get.assert_called_once_with(self.voice_channel.id) -- cgit v1.2.3 From 0fe7755733bafefe5f4dfba6458f4dbec27ac9f4 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Thu, 13 May 2021 00:32:58 +0300 Subject: Updates Silence To Accept Duration Or Channel Updates the silence command to accept the silence duration or channel as the first argument to the command. Updates tests. Signed-off-by: Hassan Abouelela --- bot/exts/moderation/silence.py | 35 +++++++++--- tests/bot/exts/moderation/test_silence.py | 91 ++++++++++++++++++++++++++++--- 2 files changed, 109 insertions(+), 17 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index e5c96e76f..8e4ce7ae2 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -1,5 +1,6 @@ import json import logging +import typing from contextlib import suppress from datetime import datetime, timedelta, timezone from typing import Optional, OrderedDict, Union @@ -84,12 +85,8 @@ class SilenceNotifier(tasks.Loop): async def _select_lock_channel(args: OrderedDict[str, any]) -> TextOrVoiceChannel: """Passes the channel to be silenced to the resource lock.""" - channel = args["channel"] - if channel is not None: - return channel - - else: - return args["ctx"].channel + channel, _ = Silence.parse_silence_args(args["ctx"], args["duration_or_channel"], args["duration"]) + return channel class Silence(commands.Cog): @@ -155,8 +152,8 @@ class Silence(commands.Cog): async def silence( self, ctx: Context, + duration_or_channel: typing.Union[TextOrVoiceChannel, HushDurationConverter] = None, duration: HushDurationConverter = 10, - channel: TextOrVoiceChannel = None, *, kick: bool = False ) -> None: @@ -170,8 +167,8 @@ class Silence(commands.Cog): If `kick` is True, members will not be added back to the voice channel, and members will be unable to rejoin. """ await self._init_task - if channel is None: - channel = ctx.channel + channel, duration = self.parse_silence_args(ctx, duration_or_channel, duration) + channel_info = f"#{channel} ({channel.id})" log.debug(f"{ctx.author} is silencing channel {channel_info}.") @@ -198,6 +195,26 @@ class Silence(commands.Cog): formatted_message = MSG_SILENCE_SUCCESS.format(duration=duration) await self.send_message(formatted_message, ctx.channel, channel, alert_target=True) + @staticmethod + def parse_silence_args( + ctx: Context, + duration_or_channel: typing.Union[TextOrVoiceChannel, int], + duration: HushDurationConverter + ) -> typing.Tuple[TextOrVoiceChannel, int]: + """Helper method to parse the arguments of the silence command.""" + duration: int + + if duration_or_channel: + if isinstance(duration_or_channel, (TextChannel, VoiceChannel)): + channel = duration_or_channel + else: + channel = ctx.channel + duration = duration_or_channel + else: + channel = ctx.channel + + return channel, duration + async def _set_silence_overwrites(self, channel: TextOrVoiceChannel, *, kick: bool = False) -> bool: """Set silence permission overwrites for `channel` and return True if successful.""" # Get the original channel overwrites diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index af6dd5a37..a7ea733c5 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -260,6 +260,81 @@ class SilenceCogTests(unittest.IsolatedAsyncioTestCase): self.assertEqual(member.move_to.call_count, 1 if member == failing_member else 2) +class SilenceArgumentParserTests(unittest.IsolatedAsyncioTestCase): + """Tests for the silence argument parser utility function.""" + + def setUp(self): + self.bot = MockBot() + self.cog = silence.Silence(self.bot) + self.cog._init_task = asyncio.Future() + self.cog._init_task.set_result(None) + + @autospec(silence.Silence, "send_message", pass_mocks=False) + @autospec(silence.Silence, "_set_silence_overwrites", return_value=False, pass_mocks=False) + @autospec(silence.Silence, "parse_silence_args") + async def test_command(self, parser_mock): + """Test that the command passes in the correct arguments for different calls.""" + test_cases = ( + (), + (15, ), + (MockTextChannel(),), + (MockTextChannel(), 15), + ) + + ctx = MockContext() + parser_mock.return_value = (ctx.channel, 10) + + for case in test_cases: + with self.subTest("Test command converters", args=case): + await self.cog.silence.callback(self.cog, ctx, *case) + + try: + first_arg = case[0] + except IndexError: + # Default value when the first argument is not passed + first_arg = None + + try: + second_arg = case[1] + except IndexError: + # Default value when the second argument is not passed + second_arg = 10 + + parser_mock.assert_called_with(ctx, first_arg, second_arg) + + async def test_no_arguments(self): + """Test the parser when no arguments are passed to the command.""" + ctx = MockContext() + channel, duration = self.cog.parse_silence_args(ctx, None, 10) + + self.assertEqual(ctx.channel, channel) + self.assertEqual(10, duration) + + async def test_channel_only(self): + """Test the parser when just the channel argument is passed.""" + expected_channel = MockTextChannel() + actual_channel, duration = self.cog.parse_silence_args(MockContext(), expected_channel, 10) + + self.assertEqual(expected_channel, actual_channel) + self.assertEqual(10, duration) + + async def test_duration_only(self): + """Test the parser when just the duration argument is passed.""" + ctx = MockContext() + channel, duration = self.cog.parse_silence_args(ctx, 15, 10) + + self.assertEqual(ctx.channel, channel) + self.assertEqual(15, duration) + + async def test_all_args(self): + """Test the parser when both channel and duration are passed.""" + expected_channel = MockTextChannel() + actual_channel, duration = self.cog.parse_silence_args(MockContext(), expected_channel, 15) + + self.assertEqual(expected_channel, actual_channel) + self.assertEqual(15, duration) + + @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) class RescheduleTests(unittest.IsolatedAsyncioTestCase): """Tests for the rescheduling of cached unsilences.""" @@ -387,7 +462,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): with self.subTest(was_silenced=was_silenced, target=target, message=message): with mock.patch.object(self.cog, "send_message") as send_message: ctx = MockContext() - await self.cog.silence.callback(self.cog, ctx, duration, target) + await self.cog.silence.callback(self.cog, ctx, target, duration) send_message.assert_called_once_with( message, ctx.channel, @@ -399,7 +474,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_sync_called(self, ctx, sync, kick): """Tests if silence command calls sync on a voice channel.""" channel = MockVoiceChannel() - await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=False) + await self.cog.silence.callback(self.cog, ctx, channel, 10, kick=False) sync.assert_awaited_once_with(self.cog, channel) kick.assert_not_called() @@ -408,7 +483,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_kick_called(self, ctx, sync, kick): """Tests if silence command calls kick on a voice channel.""" channel = MockVoiceChannel() - await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=True) + await self.cog.silence.callback(self.cog, ctx, channel, 10, kick=True) kick.assert_awaited_once_with(channel) sync.assert_not_called() @@ -417,7 +492,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_sync_not_called(self, ctx, sync, kick): """Tests that silence command does not call sync on a text channel.""" channel = MockTextChannel() - await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=False) + await self.cog.silence.callback(self.cog, ctx, channel, 10, kick=False) sync.assert_not_called() kick.assert_not_called() @@ -426,7 +501,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_kick_not_called(self, ctx, sync, kick): """Tests that silence command does not call kick on a text channel.""" channel = MockTextChannel() - await self.cog.silence.callback(self.cog, ctx, 10, channel, kick=True) + await self.cog.silence.callback(self.cog, ctx, channel, 10, kick=True) sync.assert_not_called() kick.assert_not_called() @@ -515,7 +590,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_indefinite_added_to_notifier(self): """Channel was added to notifier if a duration was not set for the silence.""" with mock.patch.object(self.cog, "_set_silence_overwrites", return_value=True): - await self.cog.silence.callback(self.cog, MockContext(), None) + await self.cog.silence.callback(self.cog, MockContext(), None, None) self.cog.notifier.add_channel.assert_called_once() async def test_silenced_not_added_to_notifier(self): @@ -547,7 +622,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_cached_indefinite_time(self): """A value of -1 was cached for a permanent silence.""" ctx = MockContext(channel=self.text_channel) - await self.cog.silence.callback(self.cog, ctx, None) + await self.cog.silence.callback(self.cog, ctx, None, None) self.cog.unsilence_timestamps.set.assert_awaited_once_with(ctx.channel.id, -1) async def test_scheduled_task(self): @@ -563,7 +638,7 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): async def test_permanent_not_scheduled(self): """A task was not scheduled for a permanent silence.""" ctx = MockContext(channel=self.text_channel) - await self.cog.silence.callback(self.cog, ctx, None) + await self.cog.silence.callback(self.cog, ctx, None, None) self.cog.scheduler.schedule_later.assert_not_called() -- cgit v1.2.3 From 50294816787562abd5f3f984f513d039612d2f3b Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Thu, 13 May 2021 06:13:31 +0300 Subject: Updates Shh Command To Mirror Silence Updates the shh and unshh commands from the error handler to accept channel and kick arguments, to give them the same interface as the silence and unsilence command. Signed-off-by: Hassan Abouelela --- bot/exts/backend/error_handler.py | 27 ++++++++- tests/bot/exts/backend/test_error_handler.py | 84 ++++++++++++++++++++++++---- 2 files changed, 98 insertions(+), 13 deletions(-) (limited to 'tests') diff --git a/bot/exts/backend/error_handler.py b/bot/exts/backend/error_handler.py index d8de177f5..a3c04437f 100644 --- a/bot/exts/backend/error_handler.py +++ b/bot/exts/backend/error_handler.py @@ -3,7 +3,7 @@ import logging import typing as t from discord import Embed -from discord.ext.commands import Cog, Context, errors +from discord.ext.commands import ChannelNotFound, Cog, Context, TextChannelConverter, VoiceChannelConverter, errors from sentry_sdk import push_scope from bot.api import ResponseCodeError @@ -115,8 +115,10 @@ class ErrorHandler(Cog): Return bool depending on success of command. """ command = ctx.invoked_with.lower() + args = ctx.message.content.lower().split(" ") silence_command = self.bot.get_command("silence") ctx.invoked_from_error_handler = True + try: if not await silence_command.can_run(ctx): log.debug("Cancelling attempt to invoke silence/unsilence due to failed checks.") @@ -124,11 +126,30 @@ class ErrorHandler(Cog): except errors.CommandError: log.debug("Cancelling attempt to invoke silence/unsilence due to failed checks.") return False + + # Parse optional args + channel = None + duration = min(command.count("h") * 2, 15) + kick = False + + if len(args) > 1: + # Parse channel + for converter in (TextChannelConverter(), VoiceChannelConverter()): + try: + channel = await converter.convert(ctx, args[1]) + break + except ChannelNotFound: + continue + + if len(args) > 2 and channel is not None: + # Parse kick + kick = args[2].lower() == "true" + if command.startswith("shh"): - await ctx.invoke(silence_command, duration=min(command.count("h")*2, 15)) + await ctx.invoke(silence_command, duration_or_channel=channel, duration=duration, kick=kick) return True elif command.startswith("unshh"): - await ctx.invoke(self.bot.get_command("unsilence")) + await ctx.invoke(self.bot.get_command("unsilence"), channel=channel) return True return False diff --git a/tests/bot/exts/backend/test_error_handler.py b/tests/bot/exts/backend/test_error_handler.py index bd4fb5942..37e8108fc 100644 --- a/tests/bot/exts/backend/test_error_handler.py +++ b/tests/bot/exts/backend/test_error_handler.py @@ -9,7 +9,7 @@ from bot.exts.backend.error_handler import ErrorHandler, setup from bot.exts.info.tags import Tags from bot.exts.moderation.silence import Silence from bot.utils.checks import InWhitelistCheckFailure -from tests.helpers import MockBot, MockContext, MockGuild, MockRole +from tests.helpers import MockBot, MockContext, MockGuild, MockRole, MockTextChannel class ErrorHandlerTests(unittest.IsolatedAsyncioTestCase): @@ -226,8 +226,8 @@ class TrySilenceTests(unittest.IsolatedAsyncioTestCase): self.bot.get_command.return_value.can_run = AsyncMock(side_effect=errors.CommandError()) self.assertFalse(await self.cog.try_silence(self.ctx)) - async def test_try_silence_silencing(self): - """Should run silence command with correct arguments.""" + async def test_try_silence_silence_duration(self): + """Should run silence command with correct duration argument.""" self.bot.get_command.return_value.can_run = AsyncMock(return_value=True) test_cases = ("shh", "shhh", "shhhhhh", "shhhhhhhhhhhhhhhhhhh") @@ -238,21 +238,85 @@ class TrySilenceTests(unittest.IsolatedAsyncioTestCase): self.assertTrue(await self.cog.try_silence(self.ctx)) self.ctx.invoke.assert_awaited_once_with( self.bot.get_command.return_value, - duration=min(case.count("h")*2, 15) + duration_or_channel=None, + duration=min(case.count("h")*2, 15), + kick=False ) + async def test_try_silence_silence_arguments(self): + """Should run silence with the correct channel, duration, and kick arguments.""" + self.bot.get_command.return_value.can_run = AsyncMock(return_value=True) + + test_cases = ( + (MockTextChannel(), None), # None represents the case when no argument is passed + (MockTextChannel(), False), + (MockTextChannel(), True) + ) + + for channel, kick in test_cases: + with self.subTest(kick=kick, channel=channel): + self.ctx.reset_mock() + self.ctx.invoked_with = "shh" + + self.ctx.message.content = f"!shh {channel.name} {kick if kick is not None else ''}" + self.ctx.guild.text_channels = [channel] + + self.assertTrue(await self.cog.try_silence(self.ctx)) + self.ctx.invoke.assert_awaited_once_with( + self.bot.get_command.return_value, + duration_or_channel=channel, + duration=4, + kick=(kick if kick is not None else False) + ) + + async def test_try_silence_silence_message(self): + """If the words after the command could not be converted to a channel, None should be passed as channel.""" + self.bot.get_command.return_value.can_run = AsyncMock(return_value=True) + self.ctx.invoked_with = "shh" + self.ctx.message.content = "!shh not_a_channel true" + + self.assertTrue(await self.cog.try_silence(self.ctx)) + self.ctx.invoke.assert_awaited_once_with( + self.bot.get_command.return_value, + duration_or_channel=None, + duration=4, + kick=False + ) + async def test_try_silence_unsilence(self): - """Should call unsilence command.""" + """Should call unsilence command with correct duration and channel arguments.""" self.silence.silence.can_run = AsyncMock(return_value=True) - test_cases = ("unshh", "unshhhhh", "unshhhhhhhhh") + test_cases = ( + ("unshh", None), + ("unshhhhh", None), + ("unshhhhhhhhh", None), + ("unshh", MockTextChannel()) + ) - for case in test_cases: - with self.subTest(message=case): + for invoke, channel in test_cases: + with self.subTest(message=invoke, channel=channel): self.bot.get_command.side_effect = (self.silence.silence, self.silence.unsilence) self.ctx.reset_mock() - self.ctx.invoked_with = case + + self.ctx.invoked_with = invoke + self.ctx.message.content = f"!{invoke}" + if channel is not None: + self.ctx.message.content += f" {channel.name}" + self.ctx.guild.text_channels = [channel] + self.assertTrue(await self.cog.try_silence(self.ctx)) - self.ctx.invoke.assert_awaited_once_with(self.silence.unsilence) + self.ctx.invoke.assert_awaited_once_with(self.silence.unsilence, channel=channel) + + async def test_try_silence_unsilence_message(self): + """If the words after the command could not be converted to a channel, None should be passed as channel.""" + self.silence.silence.can_run = AsyncMock(return_value=True) + self.bot.get_command.side_effect = (self.silence.silence, self.silence.unsilence) + + self.ctx.invoked_with = "unshh" + self.ctx.message.content = "!unshh not_a_channel" + + self.assertTrue(await self.cog.try_silence(self.ctx)) + self.ctx.invoke.assert_awaited_once_with(self.silence.unsilence, channel=None) async def test_try_silence_no_match(self): """Should return `False` when message don't match.""" -- cgit v1.2.3 From f92338ef18d1bc5d11405a5d9e6ede4f9e080110 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Sat, 5 Jun 2021 14:04:06 +0300 Subject: Properly Handles Indefinite Silences Fixes a bug that stopped the duration `forever` from getting used as a valid duration for silence. Signed-off-by: Hassan Abouelela --- bot/converters.py | 6 +++--- bot/exts/moderation/silence.py | 7 +++++-- tests/bot/exts/moderation/test_silence.py | 7 +++++++ tests/bot/test_converters.py | 2 +- 4 files changed, 16 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/bot/converters.py b/bot/converters.py index 2a3943831..595809517 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -416,11 +416,11 @@ class HushDurationConverter(Converter): MINUTES_RE = re.compile(r"(\d+)(?:M|m|$)") - async def convert(self, ctx: Context, argument: str) -> t.Optional[int]: + async def convert(self, ctx: Context, argument: str) -> int: """ Convert `argument` to a duration that's max 15 minutes or None. - If `"forever"` is passed, None is returned; otherwise an int of the extracted time. + If `"forever"` is passed, -1 is returned; otherwise an int of the extracted time. Accepted formats are: * , * m, @@ -428,7 +428,7 @@ class HushDurationConverter(Converter): * forever. """ if argument == "forever": - return None + return -1 match = self.MINUTES_RE.match(argument) if not match: raise BadArgument(f"{argument} is not a valid minutes duration.") diff --git a/bot/exts/moderation/silence.py b/bot/exts/moderation/silence.py index 8e4ce7ae2..8025f3df6 100644 --- a/bot/exts/moderation/silence.py +++ b/bot/exts/moderation/silence.py @@ -200,9 +200,9 @@ class Silence(commands.Cog): ctx: Context, duration_or_channel: typing.Union[TextOrVoiceChannel, int], duration: HushDurationConverter - ) -> typing.Tuple[TextOrVoiceChannel, int]: + ) -> typing.Tuple[TextOrVoiceChannel, Optional[int]]: """Helper method to parse the arguments of the silence command.""" - duration: int + duration: Optional[int] if duration_or_channel: if isinstance(duration_or_channel, (TextChannel, VoiceChannel)): @@ -213,6 +213,9 @@ class Silence(commands.Cog): else: channel = ctx.channel + if duration == -1: + duration = None + return channel, duration async def _set_silence_overwrites(self, channel: TextOrVoiceChannel, *, kick: bool = False) -> bool: diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index a7ea733c5..59a5893ef 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -641,6 +641,13 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): await self.cog.silence.callback(self.cog, ctx, None, None) self.cog.scheduler.schedule_later.assert_not_called() + async def test_indefinite_silence(self): + """Test silencing a channel forever.""" + with mock.patch.object(self.cog, "_schedule_unsilence") as unsilence: + ctx = MockContext(channel=self.text_channel) + await self.cog.silence.callback(self.cog, ctx, -1) + unsilence.assert_awaited_once_with(ctx, ctx.channel, None) + @autospec(silence.Silence, "unsilence_timestamps", pass_mocks=False) class UnsilenceTests(unittest.IsolatedAsyncioTestCase): diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py index 4af84dde5..2a1c4e543 100644 --- a/tests/bot/test_converters.py +++ b/tests/bot/test_converters.py @@ -291,7 +291,7 @@ class ConverterTests(unittest.IsolatedAsyncioTestCase): ("10", 10), ("5m", 5), ("5M", 5), - ("forever", None), + ("forever", -1), ) converter = HushDurationConverter() for minutes_string, expected_minutes in test_values: -- cgit v1.2.3 From 6f45d6896adb3f05962733cec8e5db199def20bc Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Fri, 2 Jul 2021 10:29:13 +0200 Subject: Bump embed limit to 4096 characters --- bot/exts/backend/branding/_cog.py | 6 +++--- bot/exts/filters/antispam.py | 2 +- bot/exts/info/doc/_parsing.py | 2 +- bot/exts/info/python_news.py | 2 +- bot/exts/moderation/infraction/_utils.py | 4 ++-- bot/exts/moderation/modlog.py | 4 ++-- bot/exts/moderation/watchchannels/_watchchannel.py | 2 +- bot/exts/recruitment/talentpool/_review.py | 4 +++- bot/pagination.py | 18 +++++++++--------- tests/bot/exts/moderation/infraction/test_utils.py | 2 +- tests/bot/exts/moderation/test_modlog.py | 2 +- 11 files changed, 25 insertions(+), 23 deletions(-) (limited to 'tests') diff --git a/bot/exts/backend/branding/_cog.py b/bot/exts/backend/branding/_cog.py index 47c379a34..0ba146635 100644 --- a/bot/exts/backend/branding/_cog.py +++ b/bot/exts/backend/branding/_cog.py @@ -50,7 +50,7 @@ def make_embed(title: str, description: str, *, success: bool) -> discord.Embed: For both `title` and `description`, empty string are valid values ~ fields will be empty. """ colour = Colours.soft_green if success else Colours.soft_red - return discord.Embed(title=title[:256], description=description[:2048], colour=colour) + return discord.Embed(title=title[:256], description=description[:4096], colour=colour) def extract_event_duration(event: Event) -> str: @@ -293,8 +293,8 @@ class Branding(commands.Cog): else: content = "Python Discord is entering a new event!" if is_notification else None - embed = discord.Embed(description=description[:2048], colour=discord.Colour.blurple()) - embed.set_footer(text=duration[:2048]) + embed = discord.Embed(description=description[:4096], colour=discord.Colour.blurple()) + embed.set_footer(text=duration[:4096]) await channel.send(content=content, embed=embed) diff --git a/bot/exts/filters/antispam.py b/bot/exts/filters/antispam.py index 7555e25a2..2f0771396 100644 --- a/bot/exts/filters/antispam.py +++ b/bot/exts/filters/antispam.py @@ -84,7 +84,7 @@ class DeletionContext: mod_alert_message += "Message:\n" [message] = self.messages.values() content = message.clean_content - remaining_chars = 2040 - len(mod_alert_message) + remaining_chars = 4080 - len(mod_alert_message) if len(content) > remaining_chars: content = content[:remaining_chars] + "..." diff --git a/bot/exts/info/doc/_parsing.py b/bot/exts/info/doc/_parsing.py index bf840b96f..1a0d42c47 100644 --- a/bot/exts/info/doc/_parsing.py +++ b/bot/exts/info/doc/_parsing.py @@ -34,7 +34,7 @@ _EMBED_CODE_BLOCK_LINE_LENGTH = 61 # _MAX_SIGNATURE_AMOUNT code block wrapped lines with py syntax highlight _MAX_SIGNATURES_LENGTH = (_EMBED_CODE_BLOCK_LINE_LENGTH + 8) * MAX_SIGNATURE_AMOUNT # Maximum embed description length - signatures on top -_MAX_DESCRIPTION_LENGTH = 2048 - _MAX_SIGNATURES_LENGTH +_MAX_DESCRIPTION_LENGTH = 4096 - _MAX_SIGNATURES_LENGTH _TRUNCATE_STRIP_CHARACTERS = "!?:;." + string.whitespace BracketPair = namedtuple("BracketPair", ["opening_bracket", "closing_bracket"]) diff --git a/bot/exts/info/python_news.py b/bot/exts/info/python_news.py index 0ab5738a4..a7837c93a 100644 --- a/bot/exts/info/python_news.py +++ b/bot/exts/info/python_news.py @@ -173,7 +173,7 @@ class PythonNews(Cog): # Build an embed and send a message to the webhook embed = discord.Embed( title=thread_information["subject"], - description=content[:500] + f"... [continue reading]({link})" if len(content) > 500 else content, + description=content[:1000] + f"... [continue reading]({link})" if len(content) > 1000 else content, timestamp=new_date, url=link, colour=constants.Colours.soft_green diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index e4eb7f79c..cfb238fa3 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -169,8 +169,8 @@ async def notify_infraction( ) # For case when other fields than reason is too long and this reach limit, then force-shorten string - if len(text) > 2048: - text = f"{text[:2045]}..." + if len(text) > 4096: + text = f"{text[:4093]}..." embed = discord.Embed( description=text, diff --git a/bot/exts/moderation/modlog.py b/bot/exts/moderation/modlog.py index be65ade6e..be2245650 100644 --- a/bot/exts/moderation/modlog.py +++ b/bot/exts/moderation/modlog.py @@ -99,7 +99,7 @@ class ModLog(Cog, name="ModLog"): """Generate log embed and send to logging channel.""" # Truncate string directly here to avoid removing newlines embed = discord.Embed( - description=text[:2045] + "..." if len(text) > 2048 else text + description=text[:4093] + "..." if len(text) > 4096 else text ) if title and icon_url: @@ -564,7 +564,7 @@ class ModLog(Cog, name="ModLog"): # Shorten the message content if necessary content = message.clean_content - remaining_chars = 2040 - len(response) + remaining_chars = 4090 - len(response) if len(content) > remaining_chars: botlog_url = await self.upload_log(messages=[message], actor_id=message.author.id) diff --git a/bot/exts/moderation/watchchannels/_watchchannel.py b/bot/exts/moderation/watchchannels/_watchchannel.py index 9f26c34f2..146426569 100644 --- a/bot/exts/moderation/watchchannels/_watchchannel.py +++ b/bot/exts/moderation/watchchannels/_watchchannel.py @@ -295,7 +295,7 @@ class WatchChannel(metaclass=CogABCMeta): footer = f"Added {time_delta} by {actor} | Reason: {reason}" embed = Embed(description=f"{msg.author.mention} {message_jump}") - embed.set_footer(text=textwrap.shorten(footer, width=128, placeholder="...")) + embed.set_footer(text=textwrap.shorten(footer, width=256, placeholder="...")) await self.webhook_send(embed=embed, username=msg.author.display_name, avatar_url=msg.author.avatar_url) diff --git a/bot/exts/recruitment/talentpool/_review.py b/bot/exts/recruitment/talentpool/_review.py index 0cb786e4b..c4c68dbc3 100644 --- a/bot/exts/recruitment/talentpool/_review.py +++ b/bot/exts/recruitment/talentpool/_review.py @@ -31,6 +31,8 @@ MAX_DAYS_IN_POOL = 30 # Maximum amount of characters allowed in a message MAX_MESSAGE_SIZE = 2000 +# Maximum amount of characters allowed in an embed +MAX_EMBED_SIZE = 4000 # Regex finding the user ID of a user mention MENTION_RE = re.compile(r"<@!?(\d+?)>") @@ -199,7 +201,7 @@ class Reviewer: channel = self.bot.get_channel(Channels.nomination_archive) for number, part in enumerate( - textwrap.wrap(embed_content, width=MAX_MESSAGE_SIZE, replace_whitespace=False, placeholder="") + textwrap.wrap(embed_content, width=MAX_EMBED_SIZE, replace_whitespace=False, placeholder="") ): await channel.send(embed=Embed( title=embed_title if number == 0 else None, diff --git a/bot/pagination.py b/bot/pagination.py index 1c5b94b07..865acce41 100644 --- a/bot/pagination.py +++ b/bot/pagination.py @@ -49,8 +49,8 @@ class LinePaginator(Paginator): self, prefix: str = '```', suffix: str = '```', - max_size: int = 2000, - scale_to_size: int = 2000, + max_size: int = 4000, + scale_to_size: int = 4000, max_lines: t.Optional[int] = None, linesep: str = "\n" ) -> None: @@ -59,10 +59,10 @@ class LinePaginator(Paginator): It overrides in order to allow us to configure the maximum number of lines per page. """ - # Embeds that exceed 2048 characters will result in an HTTPException - # (Discord API limit), so we've set a limit of 2000 - if max_size > 2000: - raise ValueError(f"max_size must be <= 2,000 characters. ({max_size} > 2000)") + # Embeds that exceed 4096 characters will result in an HTTPException + # (Discord API limit), so we've set a limit of 4000 + if max_size > 4000: + raise ValueError(f"max_size must be <= 4,000 characters. ({max_size} > 4000)") super().__init__( prefix, @@ -74,8 +74,8 @@ class LinePaginator(Paginator): if scale_to_size < max_size: raise ValueError(f"scale_to_size must be >= max_size. ({scale_to_size} < {max_size})") - if scale_to_size > 2000: - raise ValueError(f"scale_to_size must be <= 2,000 characters. ({scale_to_size} > 2000)") + if scale_to_size > 4000: + raise ValueError(f"scale_to_size must be <= 2,000 characters. ({scale_to_size} > 4000)") self.scale_to_size = scale_to_size - len(suffix) self.max_lines = max_lines @@ -197,7 +197,7 @@ class LinePaginator(Paginator): suffix: str = "", max_lines: t.Optional[int] = None, max_size: int = 500, - scale_to_size: int = 2000, + scale_to_size: int = 4000, empty: bool = True, restrict_to_user: User = None, timeout: int = 300, diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index 50a717bb5..c6ae76984 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -213,7 +213,7 @@ class ModerationUtilsTests(unittest.IsolatedAsyncioTestCase): type="Mute", expires="N/A", reason="foo bar" * 4000 - )[:2045] + "...", + )[:4093] + "...", colour=Colours.soft_red, url=utils.RULES_URL ).set_author( diff --git a/tests/bot/exts/moderation/test_modlog.py b/tests/bot/exts/moderation/test_modlog.py index f8f142484..79e04837d 100644 --- a/tests/bot/exts/moderation/test_modlog.py +++ b/tests/bot/exts/moderation/test_modlog.py @@ -25,5 +25,5 @@ class ModLogTests(unittest.IsolatedAsyncioTestCase): ) embed = self.channel.send.call_args[1]["embed"] self.assertEqual( - embed.description, ("foo bar" * 3000)[:2045] + "..." + embed.description, ("foo bar" * 3000)[:4093] + "..." ) -- cgit v1.2.3 From db410e8def7d1a0d2cc7dde625cabb78958f0af7 Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Fri, 2 Jul 2021 16:48:20 +0200 Subject: Make use of Discord timestamps --- bot/exts/info/information.py | 8 +- bot/exts/moderation/defcon.py | 6 +- bot/exts/moderation/infraction/_utils.py | 2 +- bot/exts/moderation/infraction/management.py | 18 +++-- bot/exts/moderation/stream.py | 13 +--- bot/exts/recruitment/talentpool/_review.py | 9 +-- bot/exts/utils/reminders.py | 27 +++---- bot/exts/utils/utils.py | 2 +- bot/utils/time.py | 85 ++++++++++++++-------- tests/bot/exts/info/test_information.py | 8 +- tests/bot/exts/moderation/infraction/test_utils.py | 4 +- tests/bot/utils/test_time.py | 73 ++++++++----------- 12 files changed, 126 insertions(+), 129 deletions(-) (limited to 'tests') diff --git a/bot/exts/info/information.py b/bot/exts/info/information.py index 1b1243118..2c89d39e8 100644 --- a/bot/exts/info/information.py +++ b/bot/exts/info/information.py @@ -17,7 +17,7 @@ from bot.decorators import in_whitelist from bot.pagination import LinePaginator from bot.utils.channel import is_mod_channel, is_staff_channel from bot.utils.checks import cooldown_with_role_bypass, has_no_roles_check, in_whitelist_check -from bot.utils.time import humanize_delta, time_since +from bot.utils.time import TimestampFormats, discord_timestamp, humanize_delta log = logging.getLogger(__name__) @@ -154,7 +154,7 @@ class Information(Cog): """Returns an embed full of server information.""" embed = Embed(colour=Colour.blurple(), title="Server Information") - created = time_since(ctx.guild.created_at, precision="days") + created = discord_timestamp(ctx.guild.created_at, TimestampFormats.RELATIVE) region = ctx.guild.region num_roles = len(ctx.guild.roles) - 1 # Exclude @everyone @@ -224,7 +224,7 @@ class Information(Cog): """Creates an embed containing information on the `user`.""" on_server = bool(ctx.guild.get_member(user.id)) - created = time_since(user.created_at, max_units=3) + created = discord_timestamp(user.created_at, TimestampFormats.RELATIVE) name = str(user) if on_server and user.nick: @@ -242,7 +242,7 @@ class Information(Cog): badges.append(emoji) if on_server: - joined = time_since(user.joined_at, max_units=3) + joined = discord_timestamp(user.joined_at, TimestampFormats.RELATIVE) roles = ", ".join(role.mention for role in user.roles[1:]) membership = {"Joined": joined, "Verified": not user.pending, "Roles": roles or None} if not is_mod_channel(ctx.channel): diff --git a/bot/exts/moderation/defcon.py b/bot/exts/moderation/defcon.py index dfb1afd19..9801d45ad 100644 --- a/bot/exts/moderation/defcon.py +++ b/bot/exts/moderation/defcon.py @@ -19,7 +19,9 @@ from bot.converters import DurationDelta, Expiry from bot.exts.moderation.modlog import ModLog from bot.utils.messages import format_user from bot.utils.scheduling import Scheduler -from bot.utils.time import humanize_delta, parse_duration_string, relativedelta_to_timedelta +from bot.utils.time import ( + TimestampFormats, discord_timestamp, humanize_delta, parse_duration_string, relativedelta_to_timedelta +) log = logging.getLogger(__name__) @@ -150,7 +152,7 @@ class Defcon(Cog): colour=Colour.blurple(), title="DEFCON Status", description=f""" **Threshold:** {humanize_delta(self.threshold) if self.threshold else "-"} - **Expires in:** {humanize_delta(relativedelta(self.expiry, datetime.utcnow())) if self.expiry else "-"} + **Expires:** {discord_timestamp(self.expiry, TimestampFormats.RELATIVE) if self.expiry else "-"} **Verification level:** {ctx.guild.verification_level.name} """ ) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index cfb238fa3..92e0596df 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -164,7 +164,7 @@ async def notify_infraction( text = INFRACTION_DESCRIPTION_TEMPLATE.format( type=infr_type.title(), - expires=f"{expires_at} UTC" if expires_at else "N/A", + expires=expires_at or "N/A", reason=reason or "No reason provided." ) diff --git a/bot/exts/moderation/infraction/management.py b/bot/exts/moderation/infraction/management.py index b3783cd60..4b0cb78a5 100644 --- a/bot/exts/moderation/infraction/management.py +++ b/bot/exts/moderation/infraction/management.py @@ -3,7 +3,9 @@ import textwrap import typing as t from datetime import datetime +import dateutil.parser import discord +from dateutil.relativedelta import relativedelta from discord.ext import commands from discord.ext.commands import Context from discord.utils import escape_markdown @@ -16,6 +18,7 @@ from bot.exts.moderation.modlog import ModLog from bot.pagination import LinePaginator from bot.utils import messages, time from bot.utils.channel import is_mod_channel +from bot.utils.time import humanize_delta, until_expiration log = logging.getLogger(__name__) @@ -164,8 +167,8 @@ class ModManagement(commands.Cog): self.infractions_cog.schedule_expiration(new_infraction) log_text += f""" - Previous expiry: {infraction['expires_at'] or "Permanent"} - New expiry: {new_infraction['expires_at'] or "Permanent"} + Previous expiry: {until_expiration(infraction['expires_at']) or "Permanent"} + New expiry: {until_expiration(new_infraction['expires_at']) or "Permanent"} """.rstrip() changes = ' & '.join(confirm_messages) @@ -288,10 +291,11 @@ class ModManagement(commands.Cog): remaining = "Inactive" if expires_at is None: - expires = "*Permanent*" + duration = "*Permanent*" else: - date_from = datetime.strptime(created, time.INFRACTION_FORMAT) - expires = time.format_infraction_with_duration(expires_at, date_from) + date_from = datetime.fromtimestamp(float(time.DISCORD_TIMESTAMP_REGEX.match(created).group(1))) + date_to = dateutil.parser.isoparse(expires_at).replace(tzinfo=None) + duration = humanize_delta(relativedelta(date_to, date_from)) lines = textwrap.dedent(f""" {"**===============**" if active else "==============="} @@ -300,8 +304,8 @@ class ModManagement(commands.Cog): Type: **{infraction["type"]}** Shadow: {infraction["hidden"]} Created: {created} - Expires: {expires} - Remaining: {remaining} + Expires: {remaining} + Duration: {duration} Actor: <@{infraction["actor"]["id"]}> ID: `{infraction["id"]}` Reason: {infraction["reason"] or "*None*"} diff --git a/bot/exts/moderation/stream.py b/bot/exts/moderation/stream.py index fd856a7f4..07ee4099e 100644 --- a/bot/exts/moderation/stream.py +++ b/bot/exts/moderation/stream.py @@ -13,7 +13,7 @@ from bot.constants import Colours, Emojis, Guild, MODERATION_ROLES, Roles, STAFF from bot.converters import Expiry from bot.pagination import LinePaginator from bot.utils.scheduling import Scheduler -from bot.utils.time import format_infraction_with_duration +from bot.utils.time import discord_timestamp, format_infraction_with_duration log = logging.getLogger(__name__) @@ -134,16 +134,7 @@ class Stream(commands.Cog): await member.add_roles(discord.Object(Roles.video), reason="Temporary streaming access granted") - # Use embed as embed timestamps do timezone conversions. - embed = discord.Embed( - description=f"{Emojis.check_mark} {member.mention} can now stream.", - colour=Colours.soft_green - ) - embed.set_footer(text=f"Streaming permission has been given to {member} until") - embed.timestamp = duration - - # Mention in content as mentions in embeds don't ping - await ctx.send(content=member.mention, embed=embed) + await ctx.send(f"{Emojis.check_mark} {member.mention} can now stream until {discord_timestamp(duration)}.") # Convert here for nicer logging revoke_time = format_infraction_with_duration(str(duration)) diff --git a/bot/exts/recruitment/talentpool/_review.py b/bot/exts/recruitment/talentpool/_review.py index c4c68dbc3..aebb401e0 100644 --- a/bot/exts/recruitment/talentpool/_review.py +++ b/bot/exts/recruitment/talentpool/_review.py @@ -10,7 +10,6 @@ from datetime import datetime, timedelta from typing import List, Optional, Union from dateutil.parser import isoparse -from dateutil.relativedelta import relativedelta from discord import Embed, Emoji, Member, Message, NoMoreItems, PartialMessage, TextChannel from discord.ext.commands import Context @@ -19,7 +18,7 @@ from bot.bot import Bot from bot.constants import Channels, Colours, Emojis, Guild, Roles from bot.utils.messages import count_unique_users_reaction, pin_no_system_message from bot.utils.scheduling import Scheduler -from bot.utils.time import get_time_delta, humanize_delta, time_since +from bot.utils.time import get_time_delta, time_since if typing.TYPE_CHECKING: from bot.exts.recruitment.talentpool._cog import TalentPool @@ -255,9 +254,9 @@ class Reviewer: last_channel = user_activity["top_channel_activity"][-1] channels += f", and {last_channel[1]} in {last_channel[0]}" - time_on_server = humanize_delta(relativedelta(datetime.utcnow(), member.joined_at), max_units=2) + joined_at_formatted = time_since(member.join_at) review = ( - f"{member.name} has been on the server for **{time_on_server}**" + f"{member.name} joined the server **{joined_at_formatted}**" f" and has **{messages} messages**{channels}." ) @@ -347,7 +346,7 @@ class Reviewer: nomination_times = f"{num_entries} times" if num_entries > 1 else "once" rejection_times = f"{len(history)} times" if len(history) > 1 else "once" - end_time = time_since(isoparse(history[0]['ended_at']).replace(tzinfo=None), max_units=2) + end_time = time_since(isoparse(history[0]['ended_at']).replace(tzinfo=None)) review = ( f"They were nominated **{nomination_times}** before" diff --git a/bot/exts/utils/reminders.py b/bot/exts/utils/reminders.py index 6c21920a1..c7ce8b9e9 100644 --- a/bot/exts/utils/reminders.py +++ b/bot/exts/utils/reminders.py @@ -3,12 +3,11 @@ import logging import random import textwrap import typing as t -from datetime import datetime, timedelta +from datetime import datetime from operator import itemgetter import discord from dateutil.parser import isoparse -from dateutil.relativedelta import relativedelta from discord.ext.commands import Cog, Context, Greedy, group from bot.bot import Bot @@ -19,7 +18,7 @@ from bot.utils.checks import has_any_role_check, has_no_roles_check from bot.utils.lock import lock_arg from bot.utils.messages import send_denial from bot.utils.scheduling import Scheduler -from bot.utils.time import humanize_delta +from bot.utils.time import TimestampFormats, discord_timestamp, time_since log = logging.getLogger(__name__) @@ -62,8 +61,7 @@ class Reminders(Cog): # If the reminder is already overdue ... if remind_at < now: - late = relativedelta(now, remind_at) - await self.send_reminder(reminder, late) + await self.send_reminder(reminder, remind_at) else: self.schedule_reminder(reminder) @@ -174,7 +172,7 @@ class Reminders(Cog): self.schedule_reminder(reminder) @lock_arg(LOCK_NAMESPACE, "reminder", itemgetter("id"), raise_error=True) - async def send_reminder(self, reminder: dict, late: relativedelta = None) -> None: + async def send_reminder(self, reminder: dict, expected_time: datetime = None) -> None: """Send the reminder.""" is_valid, user, channel = self.ensure_valid_reminder(reminder) if not is_valid: @@ -188,16 +186,17 @@ class Reminders(Cog): name="It has arrived!" ) - embed.description = f"Here's your reminder: `{reminder['content']}`." + # Let's not use a codeblock to keep emojis and mentions working. Embeds are safe anyway. + embed.description = f"Here's your reminder: {reminder['content']}." if reminder.get("jump_url"): # keep backward compatibility embed.description += f"\n[Jump back to when you created the reminder]({reminder['jump_url']})" - if late: + if expected_time: embed.colour = discord.Colour.red() embed.set_author( icon_url=Icons.remind_red, - name=f"Sorry it arrived {humanize_delta(late, max_units=2)} late!" + name=f"Sorry it should have arrived {time_since(expected_time)} !" ) additional_mentions = ' '.join( @@ -270,9 +269,7 @@ class Reminders(Cog): } ) - now = datetime.utcnow() - timedelta(seconds=1) - humanized_delta = humanize_delta(relativedelta(expiration, now)) - mention_string = f"Your reminder will arrive in {humanized_delta}" + mention_string = f"Your reminder will arrive {discord_timestamp(expiration, TimestampFormats.RELATIVE)}" if mentions: mention_string += f" and will mention {len(mentions)} other(s)" @@ -297,8 +294,6 @@ class Reminders(Cog): params={'author__id': str(ctx.author.id)} ) - now = datetime.utcnow() - # Make a list of tuples so it can be sorted by time. reminders = sorted( ( @@ -313,7 +308,7 @@ class Reminders(Cog): for content, remind_at, id_, mentions in reminders: # Parse and humanize the time, make it pretty :D remind_datetime = isoparse(remind_at).replace(tzinfo=None) - time = humanize_delta(relativedelta(remind_datetime, now)) + time = discord_timestamp(remind_datetime, TimestampFormats.RELATIVE) mentions = ", ".join( # Both Role and User objects have the `name` attribute @@ -322,7 +317,7 @@ class Reminders(Cog): mention_string = f"\n**Mentions:** {mentions}" if mentions else "" text = textwrap.dedent(f""" - **Reminder #{id_}:** *expires in {time}* (ID: {id_}){mention_string} + **Reminder #{id_}:** *expires {time}* (ID: {id_}){mention_string} {content} """).strip() diff --git a/bot/exts/utils/utils.py b/bot/exts/utils/utils.py index 3b8564aee..2831e30cc 100644 --- a/bot/exts/utils/utils.py +++ b/bot/exts/utils/utils.py @@ -175,7 +175,7 @@ class Utils(Cog): lines = [] for snowflake in snowflakes: created_at = snowflake_time(snowflake) - lines.append(f"**{snowflake}**\nCreated at {created_at} ({time_since(created_at, max_units=3)}).") + lines.append(f"**{snowflake}**\nCreated at {created_at} ({time_since(created_at)}).") await LinePaginator.paginate( lines, diff --git a/bot/utils/time.py b/bot/utils/time.py index d55a0e532..8cf7d623b 100644 --- a/bot/utils/time.py +++ b/bot/utils/time.py @@ -1,12 +1,13 @@ import datetime import re -from typing import Optional +from enum import Enum +from typing import Optional, Union import dateutil.parser from dateutil.relativedelta import relativedelta RFC1123_FORMAT = "%a, %d %b %Y %H:%M:%S GMT" -INFRACTION_FORMAT = "%Y-%m-%d %H:%M" +DISCORD_TIMESTAMP_REGEX = re.compile(r"") _DURATION_REGEX = re.compile( r"((?P\d+?) ?(years|year|Y|y) ?)?" @@ -19,6 +20,25 @@ _DURATION_REGEX = re.compile( ) +ValidTimestamp = Union[int, datetime.datetime, datetime.date, datetime.timedelta, relativedelta] + + +class TimestampFormats(Enum): + """ + Represents the different formats possible for Discord timestamps. + + Examples are given in epoch time. + """ + + DATE_TIME = "f" # January 1, 1970 1:00 AM + DAY_TIME = "F" # Thursday, January 1, 1970 1:00 AM + DATE_SHORT = "d" # 01/01/1970 + DATE = "D" # January 1, 1970 + TIME = "t" # 1:00 AM + TIME_SECONDS = "T" # 1:00:00 AM + RELATIVE = "R" # 52 years ago + + def _stringify_time_unit(value: int, unit: str) -> str: """ Returns a string to represent a value and time unit, ensuring that it uses the right plural form of the unit. @@ -40,6 +60,24 @@ def _stringify_time_unit(value: int, unit: str) -> str: return f"{value} {unit}" +def discord_timestamp(timestamp: ValidTimestamp, format: TimestampFormats = TimestampFormats.DATE_TIME) -> str: + """Create and format a Discord flavored markdown timestamp.""" + if format not in TimestampFormats: + raise ValueError(f"Format can only be one of {', '.join(TimestampFormats.args)}, not {format}.") + + # Convert each possible timestamp class to an integer. + if isinstance(timestamp, datetime.datetime): + timestamp = (timestamp.replace(tzinfo=None) - datetime.datetime.utcfromtimestamp(0)).total_seconds() + elif isinstance(timestamp, datetime.date): + timestamp = (timestamp - datetime.date.fromtimestamp(0)).total_seconds() + elif isinstance(timestamp, datetime.timedelta): + timestamp = timestamp.total_seconds() + elif isinstance(timestamp, relativedelta): + timestamp = timestamp.seconds + + return f"" + + def humanize_delta(delta: relativedelta, precision: str = "seconds", max_units: int = 6) -> str: """ Returns a human-readable version of the relativedelta. @@ -87,7 +125,7 @@ def humanize_delta(delta: relativedelta, precision: str = "seconds", max_units: def get_time_delta(time_string: str) -> str: """Returns the time in human-readable time delta format.""" date_time = dateutil.parser.isoparse(time_string).replace(tzinfo=None) - time_delta = time_since(date_time, precision="minutes", max_units=1) + time_delta = time_since(date_time) return time_delta @@ -123,19 +161,9 @@ def relativedelta_to_timedelta(delta: relativedelta) -> datetime.timedelta: return utcnow + delta - utcnow -def time_since(past_datetime: datetime.datetime, precision: str = "seconds", max_units: int = 6) -> str: - """ - Takes a datetime and returns a human-readable string that describes how long ago that datetime was. - - precision specifies the smallest unit of time to include (e.g. "seconds", "minutes"). - max_units specifies the maximum number of units of time to include (e.g. 1 may include days but not hours). - """ - now = datetime.datetime.utcnow() - delta = abs(relativedelta(now, past_datetime)) - - humanized = humanize_delta(delta, precision, max_units) - - return f"{humanized} ago" +def time_since(past_datetime: datetime.datetime) -> str: + """Takes a datetime and returns a discord timestamp that describes how long ago that datetime was.""" + return discord_timestamp(past_datetime, TimestampFormats.RELATIVE) def parse_rfc1123(stamp: str) -> datetime.datetime: @@ -144,8 +172,8 @@ def parse_rfc1123(stamp: str) -> datetime.datetime: def format_infraction(timestamp: str) -> str: - """Format an infraction timestamp to a more readable ISO 8601 format.""" - return dateutil.parser.isoparse(timestamp).strftime(INFRACTION_FORMAT) + """Format an infraction timestamp to a discord timestamp.""" + return discord_timestamp(dateutil.parser.isoparse(timestamp)) def format_infraction_with_duration( @@ -155,11 +183,7 @@ def format_infraction_with_duration( absolute: bool = True ) -> Optional[str]: """ - Return `date_to` formatted as a readable ISO-8601 with the humanized duration since `date_from`. - - `date_from` must be an ISO-8601 formatted timestamp. The duration is calculated as from - `date_from` until `date_to` with a precision of seconds. If `date_from` is unspecified, the - current time is used. + Return `date_to` formatted as a discord timestamp with the timestamp duration since `date_from`. `max_units` specifies the maximum number of units of time to include in the duration. For example, a value of 1 may include days but not hours. @@ -186,25 +210,22 @@ def format_infraction_with_duration( def until_expiration( - expiry: Optional[str], - now: Optional[datetime.datetime] = None, - max_units: int = 2 + expiry: Optional[str] ) -> Optional[str]: """ - Get the remaining time until infraction's expiration, in a human-readable version of the relativedelta. + Get the remaining time until infraction's expiration, in a discord timestamp. Returns a human-readable version of the remaining duration between datetime.utcnow() and an expiry. - Unlike `humanize_delta`, this function will force the `precision` to be `seconds` by not passing it. - `max_units` specifies the maximum number of units of time to include (e.g. 1 may include days but not hours). - By default, max_units is 2. + Similar to time_since, except that this function doesn't error on a null input + and return null if the expiry is in the paste """ if not expiry: return None - now = now or datetime.datetime.utcnow() + now = datetime.datetime.utcnow() since = dateutil.parser.isoparse(expiry).replace(tzinfo=None, microsecond=0) if since < now: return None - return humanize_delta(relativedelta(since, now), max_units=max_units) + return discord_timestamp(since, TimestampFormats.RELATIVE) diff --git a/tests/bot/exts/info/test_information.py b/tests/bot/exts/info/test_information.py index 770660fe3..ced3a2449 100644 --- a/tests/bot/exts/info/test_information.py +++ b/tests/bot/exts/info/test_information.py @@ -347,7 +347,7 @@ class UserEmbedTests(unittest.IsolatedAsyncioTestCase): self.assertEqual( textwrap.dedent(f""" - Created: {"1 year ago"} + Created: {""} Profile: {user.mention} ID: {user.id} """).strip(), @@ -356,7 +356,7 @@ class UserEmbedTests(unittest.IsolatedAsyncioTestCase): self.assertEqual( textwrap.dedent(f""" - Joined: {"1 year ago"} + Joined: {""} Verified: {"True"} Roles: &Moderators """).strip(), @@ -379,7 +379,7 @@ class UserEmbedTests(unittest.IsolatedAsyncioTestCase): self.assertEqual( textwrap.dedent(f""" - Created: {"1 year ago"} + Created: {""} Profile: {user.mention} ID: {user.id} """).strip(), @@ -388,7 +388,7 @@ class UserEmbedTests(unittest.IsolatedAsyncioTestCase): self.assertEqual( textwrap.dedent(f""" - Joined: {"1 year ago"} + Joined: {""} Roles: &Moderators """).strip(), embed.fields[1].value diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index c6ae76984..5f95ced9f 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -137,7 +137,7 @@ class ModerationUtilsTests(unittest.IsolatedAsyncioTestCase): title=utils.INFRACTION_TITLE, description=utils.INFRACTION_DESCRIPTION_TEMPLATE.format( type="Ban", - expires="2020-02-26 09:20 (23 hours and 59 minutes) UTC", + expires="2020-02-26 09:20 (23 hours and 59 minutes)", reason="No reason provided." ), colour=Colours.soft_red, @@ -193,7 +193,7 @@ class ModerationUtilsTests(unittest.IsolatedAsyncioTestCase): title=utils.INFRACTION_TITLE, description=utils.INFRACTION_DESCRIPTION_TEMPLATE.format( type="Mute", - expires="2020-02-26 09:20 (23 hours and 59 minutes) UTC", + expires="2020-02-26 09:20 (23 hours and 59 minutes)", reason="Test" ), colour=Colours.soft_red, diff --git a/tests/bot/utils/test_time.py b/tests/bot/utils/test_time.py index 115ddfb0d..8edffd1c9 100644 --- a/tests/bot/utils/test_time.py +++ b/tests/bot/utils/test_time.py @@ -52,7 +52,7 @@ class TimeTests(unittest.TestCase): def test_format_infraction(self): """Testing format_infraction.""" - self.assertEqual(time.format_infraction('2019-12-12T00:01:00Z'), '2019-12-12 00:01') + self.assertEqual(time.format_infraction('2019-12-12T00:01:00Z'), '') def test_format_infraction_with_duration_none_expiry(self): """format_infraction_with_duration should work for None expiry.""" @@ -72,10 +72,10 @@ class TimeTests(unittest.TestCase): def test_format_infraction_with_duration_custom_units(self): """format_infraction_with_duration should work for custom max_units.""" test_cases = ( - ('2019-12-12T00:01:00Z', datetime(2019, 12, 11, 12, 5, 5), 6, - '2019-12-12 00:01 (11 hours, 55 minutes and 55 seconds)'), - ('2019-11-23T20:09:00Z', datetime(2019, 4, 25, 20, 15), 20, - '2019-11-23 20:09 (6 months, 28 days, 23 hours and 54 minutes)') + ('3000-12-12T00:01:00Z', datetime(3000, 12, 11, 12, 5, 5), 6, + ' (11 hours, 55 minutes and 55 seconds)'), + ('3000-11-23T20:09:00Z', datetime(3000, 4, 25, 20, 15), 20, + ' (6 months, 28 days, 23 hours and 54 minutes)') ) for expiry, date_from, max_units, expected in test_cases: @@ -85,16 +85,16 @@ class TimeTests(unittest.TestCase): def test_format_infraction_with_duration_normal_usage(self): """format_infraction_with_duration should work for normal usage, across various durations.""" test_cases = ( - ('2019-12-12T00:01:00Z', datetime(2019, 12, 11, 12, 0, 5), 2, '2019-12-12 00:01 (12 hours and 55 seconds)'), - ('2019-12-12T00:01:00Z', datetime(2019, 12, 11, 12, 0, 5), 1, '2019-12-12 00:01 (12 hours)'), - ('2019-12-12T00:00:00Z', datetime(2019, 12, 11, 23, 59), 2, '2019-12-12 00:00 (1 minute)'), - ('2019-11-23T20:09:00Z', datetime(2019, 11, 15, 20, 15), 2, '2019-11-23 20:09 (7 days and 23 hours)'), - ('2019-11-23T20:09:00Z', datetime(2019, 4, 25, 20, 15), 2, '2019-11-23 20:09 (6 months and 28 days)'), - ('2019-11-23T20:58:00Z', datetime(2019, 11, 23, 20, 53), 2, '2019-11-23 20:58 (5 minutes)'), - ('2019-11-24T00:00:00Z', datetime(2019, 11, 23, 23, 59, 0), 2, '2019-11-24 00:00 (1 minute)'), - ('2019-11-23T23:59:00Z', datetime(2017, 7, 21, 23, 0), 2, '2019-11-23 23:59 (2 years and 4 months)'), + ('2019-12-12T00:01:00Z', datetime(2019, 12, 11, 12, 0, 5), 2, ' (12 hours and 55 seconds)'), + ('2019-12-12T00:01:00Z', datetime(2019, 12, 11, 12, 0, 5), 1, ' (12 hours)'), + ('2019-12-12T00:00:00Z', datetime(2019, 12, 11, 23, 59), 2, ' (1 minute)'), + ('2019-11-23T20:09:00Z', datetime(2019, 11, 15, 20, 15), 2, ' (7 days and 23 hours)'), + ('2019-11-23T20:09:00Z', datetime(2019, 4, 25, 20, 15), 2, ' (6 months and 28 days)'), + ('2019-11-23T20:58:00Z', datetime(2019, 11, 23, 20, 53), 2, ' (5 minutes)'), + ('2019-11-24T00:00:00Z', datetime(2019, 11, 23, 23, 59, 0), 2, ' (1 minute)'), + ('2019-11-23T23:59:00Z', datetime(2017, 7, 21, 23, 0), 2, ' (2 years and 4 months)'), ('2019-11-23T23:59:00Z', datetime(2019, 11, 23, 23, 49, 5), 2, - '2019-11-23 23:59 (9 minutes and 55 seconds)'), + ' (9 minutes and 55 seconds)'), (None, datetime(2019, 11, 23, 23, 49, 5), 2, None), ) @@ -104,45 +104,30 @@ class TimeTests(unittest.TestCase): def test_until_expiration_with_duration_none_expiry(self): """until_expiration should work for None expiry.""" - test_cases = ( - (None, None, None, None), - - # To make sure that now and max_units are not touched - (None, 'Why hello there!', None, None), - (None, None, float('inf'), None), - (None, 'Why hello there!', float('inf'), None), - ) - - for expiry, now, max_units, expected in test_cases: - with self.subTest(expiry=expiry, now=now, max_units=max_units, expected=expected): - self.assertEqual(time.until_expiration(expiry, now, max_units), expected) + self.assertEqual(time.until_expiration(None), None) def test_until_expiration_with_duration_custom_units(self): """until_expiration should work for custom max_units.""" test_cases = ( - ('2019-12-12T00:01:00Z', datetime(2019, 12, 11, 12, 5, 5), 6, '11 hours, 55 minutes and 55 seconds'), - ('2019-11-23T20:09:00Z', datetime(2019, 4, 25, 20, 15), 20, '6 months, 28 days, 23 hours and 54 minutes') + ('3000-12-12T00:01:00Z', ''), + ('3000-11-23T20:09:00Z', '') ) - for expiry, now, max_units, expected in test_cases: - with self.subTest(expiry=expiry, now=now, max_units=max_units, expected=expected): - self.assertEqual(time.until_expiration(expiry, now, max_units), expected) + for expiry, expected in test_cases: + with self.subTest(expiry=expiry, expected=expected): + self.assertEqual(time.until_expiration(expiry,), expected) def test_until_expiration_normal_usage(self): """until_expiration should work for normal usage, across various durations.""" test_cases = ( - ('2019-12-12T00:01:00Z', datetime(2019, 12, 11, 12, 0, 5), 2, '12 hours and 55 seconds'), - ('2019-12-12T00:01:00Z', datetime(2019, 12, 11, 12, 0, 5), 1, '12 hours'), - ('2019-12-12T00:00:00Z', datetime(2019, 12, 11, 23, 59), 2, '1 minute'), - ('2019-11-23T20:09:00Z', datetime(2019, 11, 15, 20, 15), 2, '7 days and 23 hours'), - ('2019-11-23T20:09:00Z', datetime(2019, 4, 25, 20, 15), 2, '6 months and 28 days'), - ('2019-11-23T20:58:00Z', datetime(2019, 11, 23, 20, 53), 2, '5 minutes'), - ('2019-11-24T00:00:00Z', datetime(2019, 11, 23, 23, 59, 0), 2, '1 minute'), - ('2019-11-23T23:59:00Z', datetime(2017, 7, 21, 23, 0), 2, '2 years and 4 months'), - ('2019-11-23T23:59:00Z', datetime(2019, 11, 23, 23, 49, 5), 2, '9 minutes and 55 seconds'), - (None, datetime(2019, 11, 23, 23, 49, 5), 2, None), + ('3000-12-12T00:01:00Z', ''), + ('3000-12-12T00:01:00Z', ''), + ('3000-12-12T00:00:00Z', ''), + ('3000-11-23T20:09:00Z', ''), + ('3000-11-23T20:09:00Z', ''), + (None, None), ) - for expiry, now, max_units, expected in test_cases: - with self.subTest(expiry=expiry, now=now, max_units=max_units, expected=expected): - self.assertEqual(time.until_expiration(expiry, now, max_units), expected) + for expiry, expected in test_cases: + with self.subTest(expiry=expiry, expected=expected): + self.assertEqual(time.until_expiration(expiry), expected) -- cgit v1.2.3 From 231f6dc1d7e9fe478c1fcd517c2ec94a54e0763a Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Fri, 2 Jul 2021 16:58:32 +0200 Subject: Tests: remove stale patch of time_since --- tests/bot/exts/info/test_information.py | 1 - 1 file changed, 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/exts/info/test_information.py b/tests/bot/exts/info/test_information.py index ced3a2449..0aa41d889 100644 --- a/tests/bot/exts/info/test_information.py +++ b/tests/bot/exts/info/test_information.py @@ -262,7 +262,6 @@ class UserInfractionHelperMethodTests(unittest.IsolatedAsyncioTestCase): await self._method_subtests(self.cog.user_nomination_counts, test_values, header) -@unittest.mock.patch("bot.exts.info.information.time_since", new=unittest.mock.MagicMock(return_value="1 year ago")) @unittest.mock.patch("bot.exts.info.information.constants.MODERATION_CHANNELS", new=[50]) class UserEmbedTests(unittest.IsolatedAsyncioTestCase): """Tests for the creation of the `!user` embed.""" -- cgit v1.2.3 From 2ff595670b0603fe9e97d16ffcc2c04457fea1c5 Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Wed, 7 Jul 2021 18:21:06 +0300 Subject: Fixes N818 Compliance Renames a couple exceptions to include the error suffix, as enforced by N818. Signed-off-by: Hassan Abouelela --- bot/errors.py | 2 +- bot/exts/backend/error_handler.py | 4 ++-- bot/exts/moderation/infraction/_utils.py | 4 ++-- bot/pagination.py | 4 ++-- tests/bot/exts/backend/test_error_handler.py | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) (limited to 'tests') diff --git a/bot/errors.py b/bot/errors.py index 3544c6320..46efb6d4f 100644 --- a/bot/errors.py +++ b/bot/errors.py @@ -22,7 +22,7 @@ class LockedResourceError(RuntimeError): ) -class InvalidInfractedUser(Exception): +class InvalidInfractedUserError(Exception): """ Exception raised upon attempt of infracting an invalid user. diff --git a/bot/exts/backend/error_handler.py b/bot/exts/backend/error_handler.py index d8de177f5..7ef55af45 100644 --- a/bot/exts/backend/error_handler.py +++ b/bot/exts/backend/error_handler.py @@ -10,7 +10,7 @@ from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Colours, Icons, MODERATION_ROLES from bot.converters import TagNameConverter -from bot.errors import InvalidInfractedUser, LockedResourceError +from bot.errors import InvalidInfractedUserError, LockedResourceError from bot.utils.checks import ContextCheckFailure log = logging.getLogger(__name__) @@ -76,7 +76,7 @@ class ErrorHandler(Cog): await self.handle_api_error(ctx, e.original) elif isinstance(e.original, LockedResourceError): await ctx.send(f"{e.original} Please wait for it to finish and try again later.") - elif isinstance(e.original, InvalidInfractedUser): + elif isinstance(e.original, InvalidInfractedUserError): await ctx.send(f"Cannot infract that user. {e.original.reason}") else: await self.handle_unexpected_error(ctx, e.original) diff --git a/bot/exts/moderation/infraction/_utils.py b/bot/exts/moderation/infraction/_utils.py index e4eb7f79c..adbc641fa 100644 --- a/bot/exts/moderation/infraction/_utils.py +++ b/bot/exts/moderation/infraction/_utils.py @@ -7,7 +7,7 @@ from discord.ext.commands import Context from bot.api import ResponseCodeError from bot.constants import Colours, Icons -from bot.errors import InvalidInfractedUser +from bot.errors import InvalidInfractedUserError log = logging.getLogger(__name__) @@ -85,7 +85,7 @@ async def post_infraction( """Posts an infraction to the API.""" if isinstance(user, (discord.Member, discord.User)) and user.bot: log.trace(f"Posting of {infr_type} infraction for {user} to the API aborted. User is a bot.") - raise InvalidInfractedUser(user) + raise InvalidInfractedUserError(user) log.trace(f"Posting {infr_type} infraction for {user} to the API.") diff --git a/bot/pagination.py b/bot/pagination.py index 1c5b94b07..fbab74021 100644 --- a/bot/pagination.py +++ b/bot/pagination.py @@ -22,7 +22,7 @@ PAGINATION_EMOJI = (FIRST_EMOJI, LEFT_EMOJI, RIGHT_EMOJI, LAST_EMOJI, DELETE_EMO log = logging.getLogger(__name__) -class EmptyPaginatorEmbed(Exception): +class EmptyPaginatorEmbedError(Exception): """Raised when attempting to paginate with empty contents.""" pass @@ -233,7 +233,7 @@ class LinePaginator(Paginator): if not lines: if exception_on_empty_embed: log.exception("Pagination asked for empty lines iterable") - raise EmptyPaginatorEmbed("No lines to paginate") + raise EmptyPaginatorEmbedError("No lines to paginate") log.debug("No lines to add to paginator, adding '(nothing to display)' message") lines.append("(nothing to display)") diff --git a/tests/bot/exts/backend/test_error_handler.py b/tests/bot/exts/backend/test_error_handler.py index bd4fb5942..944cef6ca 100644 --- a/tests/bot/exts/backend/test_error_handler.py +++ b/tests/bot/exts/backend/test_error_handler.py @@ -4,7 +4,7 @@ from unittest.mock import AsyncMock, MagicMock, call, patch from discord.ext.commands import errors from bot.api import ResponseCodeError -from bot.errors import InvalidInfractedUser, LockedResourceError +from bot.errors import InvalidInfractedUserError, LockedResourceError from bot.exts.backend.error_handler import ErrorHandler, setup from bot.exts.info.tags import Tags from bot.exts.moderation.silence import Silence @@ -130,7 +130,7 @@ class ErrorHandlerTests(unittest.IsolatedAsyncioTestCase): "expect_mock_call": "send" }, { - "args": (self.ctx, errors.CommandInvokeError(InvalidInfractedUser(self.ctx.author))), + "args": (self.ctx, errors.CommandInvokeError(InvalidInfractedUserError(self.ctx.author))), "expect_mock_call": "send" } ) -- cgit v1.2.3 From 5bbd29c8957c320ef94beacd191239032eb959ec Mon Sep 17 00:00:00 2001 From: Hassan Abouelela Date: Fri, 9 Jul 2021 03:32:51 +0300 Subject: Properly Handle Fuzzy Matching Help Fixes a bug where calling help with an invalid command would crash out during fuzzy matching. Signed-off-by: Hassan Abouelela --- bot/exts/info/help.py | 4 ++-- tests/bot/exts/info/test_help.py | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 tests/bot/exts/info/test_help.py (limited to 'tests') diff --git a/bot/exts/info/help.py b/bot/exts/info/help.py index bf9ea5986..0235bbaf3 100644 --- a/bot/exts/info/help.py +++ b/bot/exts/info/help.py @@ -125,9 +125,9 @@ class CustomHelpCommand(HelpCommand): Will return an instance of the `HelpQueryNotFound` exception with the error message and possible matches. """ - choices = await self.get_all_help_choices() + choices = list(await self.get_all_help_choices()) result = process.extract(default_process(string), choices, scorer=fuzz.ratio, score_cutoff=60, processor=None) - return HelpQueryNotFound(f'Query "{string}" not found.', dict(result)) + return HelpQueryNotFound(f'Query "{string}" not found.', {choice[0]: choice[1] for choice in result}) async def subcommand_not_found(self, command: Command, string: str) -> "HelpQueryNotFound": """ diff --git a/tests/bot/exts/info/test_help.py b/tests/bot/exts/info/test_help.py new file mode 100644 index 000000000..604c69671 --- /dev/null +++ b/tests/bot/exts/info/test_help.py @@ -0,0 +1,23 @@ +import unittest + +import rapidfuzz + +from bot.exts.info import help +from tests.helpers import MockBot, MockContext, autospec + + +class HelpCogTests(unittest.IsolatedAsyncioTestCase): + def setUp(self) -> None: + """Attach an instance of the cog to the class for tests.""" + self.bot = MockBot() + self.cog = help.Help(self.bot) + self.ctx = MockContext(bot=self.bot) + self.bot.help_command.context = self.ctx + + @autospec(help.CustomHelpCommand, "get_all_help_choices", return_value={"help"}, pass_mocks=False) + async def test_help_fuzzy_matching(self): + """Test fuzzy matching of commands when called from help.""" + result = await self.bot.help_command.command_not_found("holp") + + match = {"help": rapidfuzz.fuzz.ratio("help", "holp")} + self.assertEqual(match, result.possible_matches) -- cgit v1.2.3