From 1e0c0cfe37eb9a868454508fcb813d7cf19e12cc Mon Sep 17 00:00:00 2001 From: Izan Date: Wed, 29 Dec 2021 15:09:22 +0000 Subject: Fix tests --- tests/bot/exts/moderation/test_incidents.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_incidents.py b/tests/bot/exts/moderation/test_incidents.py index cfe0c4b03..ef33aa62b 100644 --- a/tests/bot/exts/moderation/test_incidents.py +++ b/tests/bot/exts/moderation/test_incidents.py @@ -1,4 +1,5 @@ import asyncio +import datetime import enum import logging import typing as t @@ -13,6 +14,7 @@ from async_rediscache import RedisSession from bot.constants import Colours from bot.exts.moderation import incidents from bot.utils.messages import format_user +from bot.utils.time import TimestampFormats, discord_timestamp from tests.helpers import ( MockAsyncWebhook, MockAttachment, MockBot, MockMember, MockMessage, MockReaction, MockRole, MockTextChannel, MockUser @@ -114,10 +116,19 @@ class TestMakeEmbed(unittest.IsolatedAsyncioTestCase): async def test_make_embed_content(self): """Incident content appears as embed description.""" - incident = MockMessage(content="this is an incident") + current_time = datetime.datetime(2022, 1, 1, 0, 0, 0, tzinfo=datetime.timezone.utc) + incident = MockMessage(content="this is an incident", created_at=current_time) + + day_timestamp = discord_timestamp(current_time, TimestampFormats.DATE) + time_timestamp = discord_timestamp(current_time, TimestampFormats.TIME) + relative_timestamp = discord_timestamp(current_time, TimestampFormats.RELATIVE) + embed, file = await incidents.make_embed(incident, incidents.Signal.ACTIONED, MockMember()) - self.assertEqual(incident.content, embed.description) + self.assertEqual( + f"{incident.content}\n\n__*Reported {day_timestamp} at {time_timestamp} ({relative_timestamp}).*__", + embed.description + ) async def test_make_embed_with_attachment_succeeds(self): """Incident's attachment is downloaded and displayed in the embed's image field.""" @@ -391,7 +402,7 @@ class TestArchive(TestIncidents): # Define our own `incident` to be archived incident = MockMessage( content="this is an incident", - author=MockUser(name="author_name", display_avatar=Mock(url="author_avatar")), + author=MockUser(display_name="author_name", display_avatar=Mock(url="author_avatar")), id=123, ) built_embed = MagicMock(discord.Embed, id=123) # We patch `make_embed` to return this @@ -422,7 +433,7 @@ class TestArchive(TestIncidents): webhook = MockAsyncWebhook() self.cog_instance.bot.fetch_webhook = AsyncMock(return_value=webhook) - message_from_clyde = MockMessage(author=MockUser(name="clyde the great")) + message_from_clyde = MockMessage(author=MockUser(display_name="clyde the great")) await self.cog_instance.archive(message_from_clyde, MagicMock(incidents.Signal), MockMember()) self.assertNotIn("clyde", webhook.send.call_args.kwargs["username"]) @@ -521,12 +532,13 @@ class TestProcessEvent(TestIncidents): async def test_process_event_confirmation_task_is_awaited(self): """Task given by `Incidents.make_confirmation_task` is awaited before method exits.""" mock_task = AsyncMock() + mock_member = MockMember(display_name="Bobby Johnson", roles=[MockRole(id=1)]) with patch("bot.exts.moderation.incidents.Incidents.make_confirmation_task", mock_task): await self.cog_instance.process_event( reaction=incidents.Signal.ACTIONED.value, - incident=MockMessage(id=123), - member=MockMember(roles=[MockRole(id=1)]) + incident=MockMessage(author=mock_member, id=123), + member=mock_member ) mock_task.assert_awaited() -- cgit v1.2.3 From b7e03616ac3fc0b5e8a5a77a352df593983d187a Mon Sep 17 00:00:00 2001 From: Izan Date: Thu, 14 Jul 2022 22:21:34 +0100 Subject: Address Reviews - Use the more concise DATETIME timestamp instead of both a DATE and a TIME timestamp. - Remove underline from the "Reported ..." section at the bottom of the embed. - Re-add time of action/rejection timestamp to footer of embed. --- bot/exts/moderation/incidents.py | 7 ++++--- tests/bot/exts/moderation/test_incidents.py | 5 ++--- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/bot/exts/moderation/incidents.py b/bot/exts/moderation/incidents.py index bd9e5b88e..f29cfcdd6 100644 --- a/bot/exts/moderation/incidents.py +++ b/bot/exts/moderation/incidents.py @@ -1,5 +1,6 @@ import asyncio import re +from datetime import datetime, timezone from enum import Enum from typing import Optional @@ -97,10 +98,9 @@ async def make_embed(incident: discord.Message, outcome: Signal, actioned_by: di colour = Colours.soft_red footer = f"Rejected by {actioned_by}" - day_timestamp = discord_timestamp(incident.created_at, TimestampFormats.DATE) - time_timestamp = discord_timestamp(incident.created_at, TimestampFormats.TIME) + reported_timestamp = discord_timestamp(incident.created_at) relative_timestamp = discord_timestamp(incident.created_at, TimestampFormats.RELATIVE) - reported_on_msg = f"__*Reported {day_timestamp} at {time_timestamp} ({relative_timestamp}).*__" + reported_on_msg = f"*Reported {reported_timestamp} ({relative_timestamp}).*" # If the description will be too long (>4096 total characters), truncate the incident content if len(incident.content) > (allowed_content_chars := 4096-len(reported_on_msg)-2): # -2 for the newlines @@ -111,6 +111,7 @@ async def make_embed(incident: discord.Message, outcome: Signal, actioned_by: di embed = discord.Embed( description=description, colour=colour, + timestamp=datetime.now(timezone.utc) ) embed.set_footer(text=footer, icon_url=actioned_by.display_avatar.url) diff --git a/tests/bot/exts/moderation/test_incidents.py b/tests/bot/exts/moderation/test_incidents.py index ef33aa62b..da0a79ce8 100644 --- a/tests/bot/exts/moderation/test_incidents.py +++ b/tests/bot/exts/moderation/test_incidents.py @@ -119,14 +119,13 @@ class TestMakeEmbed(unittest.IsolatedAsyncioTestCase): current_time = datetime.datetime(2022, 1, 1, 0, 0, 0, tzinfo=datetime.timezone.utc) incident = MockMessage(content="this is an incident", created_at=current_time) - day_timestamp = discord_timestamp(current_time, TimestampFormats.DATE) - time_timestamp = discord_timestamp(current_time, TimestampFormats.TIME) + reported_timestamp = discord_timestamp(current_time) relative_timestamp = discord_timestamp(current_time, TimestampFormats.RELATIVE) embed, file = await incidents.make_embed(incident, incidents.Signal.ACTIONED, MockMember()) self.assertEqual( - f"{incident.content}\n\n__*Reported {day_timestamp} at {time_timestamp} ({relative_timestamp}).*__", + f"{incident.content}\n\n*Reported {reported_timestamp} ({relative_timestamp}).*", embed.description ) -- cgit v1.2.3 From 1df6034a1723fd3ff1bd88047ca6a62f920767e6 Mon Sep 17 00:00:00 2001 From: Izan Date: Mon, 15 Aug 2022 12:04:33 +0100 Subject: Fix incident tests. --- tests/bot/exts/moderation/test_incidents.py | 38 +++++++++++++++++++---------- 1 file changed, 25 insertions(+), 13 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_incidents.py b/tests/bot/exts/moderation/test_incidents.py index 11fe565fc..53d98360c 100644 --- a/tests/bot/exts/moderation/test_incidents.py +++ b/tests/bot/exts/moderation/test_incidents.py @@ -20,6 +20,8 @@ from tests.helpers import ( MockUser ) +CURRENT_TIME = datetime.datetime(2022, 1, 1, tzinfo=datetime.timezone.utc) + class MockAsyncIterable: """ @@ -102,25 +104,32 @@ class TestMakeEmbed(unittest.IsolatedAsyncioTestCase): async def test_make_embed_actioned(self): """Embed is coloured green and footer contains 'Actioned' when `outcome=Signal.ACTIONED`.""" - embed, file = await incidents.make_embed(MockMessage(), incidents.Signal.ACTIONED, MockMember()) + embed, file = await incidents.make_embed( + incident=MockMessage(created_at=CURRENT_TIME), + outcome=incidents.Signal.ACTIONED, + actioned_by=MockMember() + ) self.assertEqual(embed.colour.value, Colours.soft_green) self.assertIn("Actioned", embed.footer.text) async def test_make_embed_not_actioned(self): """Embed is coloured red and footer contains 'Rejected' when `outcome=Signal.NOT_ACTIONED`.""" - embed, file = await incidents.make_embed(MockMessage(), incidents.Signal.NOT_ACTIONED, MockMember()) + embed, file = await incidents.make_embed( + incident=MockMessage(created_at=CURRENT_TIME), + outcome=incidents.Signal.NOT_ACTIONED, + actioned_by=MockMember() + ) self.assertEqual(embed.colour.value, Colours.soft_red) self.assertIn("Rejected", embed.footer.text) async def test_make_embed_content(self): """Incident content appears as embed description.""" - current_time = datetime.datetime(2022, 1, 1, 0, 0, 0, tzinfo=datetime.timezone.utc) - incident = MockMessage(content="this is an incident", created_at=current_time) + incident = MockMessage(content="this is an incident", created_at=CURRENT_TIME) - reported_timestamp = discord_timestamp(current_time) - relative_timestamp = discord_timestamp(current_time, TimestampFormats.RELATIVE) + reported_timestamp = discord_timestamp(CURRENT_TIME) + relative_timestamp = discord_timestamp(CURRENT_TIME, TimestampFormats.RELATIVE) embed, file = await incidents.make_embed(incident, incidents.Signal.ACTIONED, MockMember()) @@ -133,7 +142,7 @@ class TestMakeEmbed(unittest.IsolatedAsyncioTestCase): """Incident's attachment is downloaded and displayed in the embed's image field.""" file = MagicMock(discord.File, filename="bigbadjoe.jpg") attachment = MockAttachment(filename="bigbadjoe.jpg") - incident = MockMessage(content="this is an incident", attachments=[attachment]) + incident = MockMessage(content="this is an incident", attachments=[attachment], created_at=CURRENT_TIME) # Patch `download_file` to return our `file` with patch("bot.exts.moderation.incidents.download_file", AsyncMock(return_value=file)): @@ -145,7 +154,7 @@ class TestMakeEmbed(unittest.IsolatedAsyncioTestCase): async def test_make_embed_with_attachment_fails(self): """Incident's attachment fails to download, proxy url is linked instead.""" attachment = MockAttachment(proxy_url="discord.com/bigbadjoe.jpg") - incident = MockMessage(content="this is an incident", attachments=[attachment]) + incident = MockMessage(content="this is an incident", attachments=[attachment], created_at=CURRENT_TIME) # Patch `download_file` to return None as if the download failed with patch("bot.exts.moderation.incidents.download_file", AsyncMock(return_value=None)): @@ -359,7 +368,6 @@ class TestCrawlIncidents(TestIncidents): class TestArchive(TestIncidents): """Tests for the `Incidents.archive` coroutine.""" - async def test_archive_webhook_not_found(self): """ Method recovers and returns False when the webhook is not found. @@ -369,7 +377,11 @@ class TestArchive(TestIncidents): """ self.cog_instance.bot.fetch_webhook = AsyncMock(side_effect=mock_404) self.assertFalse( - await self.cog_instance.archive(incident=MockMessage(), outcome=MagicMock(), actioned_by=MockMember()) + await self.cog_instance.archive( + incident=MockMessage(created_at=CURRENT_TIME), + outcome=MagicMock(), + actioned_by=MockMember() + ) ) async def test_archive_relays_incident(self): @@ -416,7 +428,7 @@ class TestArchive(TestIncidents): webhook = MockAsyncWebhook() self.cog_instance.bot.fetch_webhook = AsyncMock(return_value=webhook) - message_from_clyde = MockMessage(author=MockUser(display_name="clyde the great")) + message_from_clyde = MockMessage(author=MockUser(display_name="clyde the great"), created_at=CURRENT_TIME) await self.cog_instance.archive(message_from_clyde, MagicMock(incidents.Signal), MockMember()) self.assertNotIn("clyde", webhook.send.call_args.kwargs["username"]) @@ -520,7 +532,7 @@ class TestProcessEvent(TestIncidents): with patch("bot.exts.moderation.incidents.Incidents.make_confirmation_task", mock_task): await self.cog_instance.process_event( reaction=incidents.Signal.ACTIONED.value, - incident=MockMessage(author=mock_member, id=123), + incident=MockMessage(author=mock_member, id=123, created_at=CURRENT_TIME), member=mock_member ) @@ -540,7 +552,7 @@ class TestProcessEvent(TestIncidents): with patch("bot.exts.moderation.incidents.Incidents.make_confirmation_task", mock_task): await self.cog_instance.process_event( reaction=incidents.Signal.ACTIONED.value, - incident=MockMessage(id=123), + incident=MockMessage(id=123, created_at=CURRENT_TIME), member=MockMember(roles=[MockRole(id=1)]) ) except asyncio.TimeoutError: -- cgit v1.2.3 From e0b593318eba77d6fe93f2145b43838d6eb09278 Mon Sep 17 00:00:00 2001 From: Chris Lovering Date: Mon, 15 Aug 2022 22:14:32 +0100 Subject: Correctly initialise redis tests Calling the cog_load from within the setUp function resulted in interaction with a RedisSession before it was initialised. This wasn't noticed in CI as it only error under certain concurrency timings due to xdist. To resolve this, we moved the setup and async setup logic to a base class. Co-authored-by: Hassan Abouelela --- tests/bot/exts/moderation/test_silence.py | 79 +++++++++++++++---------------- 1 file changed, 37 insertions(+), 42 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/test_silence.py b/tests/bot/exts/moderation/test_silence.py index 98547e2bc..2622f46a7 100644 --- a/tests/bot/exts/moderation/test_silence.py +++ b/tests/bot/exts/moderation/test_silence.py @@ -1,4 +1,3 @@ -import asyncio import itertools import unittest from datetime import datetime, timezone @@ -23,8 +22,24 @@ class PatchedDatetime(datetime): now = mock.create_autospec(datetime, "now") -class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): +class SilenceTest(RedisTestCase): + """A base class for Silence tests that correctly sets up the cog and redis.""" + + @autospec(silence, "Scheduler", pass_mocks=False) + @autospec(silence.Silence, "_reschedule", pass_mocks=False) + def setUp(self) -> None: + self.bot = MockBot(get_channel=lambda _id: MockTextChannel(id=_id)) + self.cog = silence.Silence(self.bot) + + @autospec(silence, "SilenceNotifier", pass_mocks=False) + async def asyncSetUp(self) -> None: + await super().asyncSetUp() + await self.cog.cog_load() # Populate instance attributes. + + +class SilenceNotifierTests(SilenceTest): def setUp(self) -> None: + super().setUp() self.alert_channel = MockTextChannel() self.notifier = silence.SilenceNotifier(self.alert_channel) self.notifier.stop = self.notifier_stop_mock = Mock() @@ -89,34 +104,24 @@ class SilenceNotifierTests(unittest.IsolatedAsyncioTestCase): @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) -class SilenceCogTests(RedisTestCase): +class SilenceCogTests(SilenceTest): """Tests for the general functionality of the Silence cog.""" - @autospec(silence, "Scheduler", pass_mocks=False) - def setUp(self) -> None: - self.bot = MockBot() - self.cog = silence.Silence(self.bot) - @autospec(silence, "SilenceNotifier", pass_mocks=False) async def test_cog_load_got_guild(self): """Bot got guild after it became available.""" - await self.cog.cog_load() self.bot.wait_until_guild_available.assert_awaited_once() self.bot.get_guild.assert_called_once_with(Guild.id) @autospec(silence, "SilenceNotifier", pass_mocks=False) async def test_cog_load_got_channels(self): """Got channels from bot.""" - self.bot.get_channel.side_effect = lambda id_: MockTextChannel(id=id_) - await self.cog.cog_load() self.assertEqual(self.cog._mod_alerts_channel.id, Channels.mod_alerts) @autospec(silence, "SilenceNotifier") async def test_cog_load_got_notifier(self, notifier): """Notifier was started with channel.""" - self.bot.get_channel.side_effect = lambda id_: MockTextChannel(id=id_) - await self.cog.cog_load() notifier.assert_called_once_with(MockTextChannel(id=Channels.mod_log)) self.assertEqual(self.cog.notifier, notifier.return_value) @@ -229,13 +234,9 @@ class SilenceCogTests(RedisTestCase): self.assertEqual(member.move_to.call_count, 1 if member == failing_member else 2) -class SilenceArgumentParserTests(RedisTestCase): +class SilenceArgumentParserTests(SilenceTest): """Tests for the silence argument parser utility function.""" - def setUp(self): - self.bot = MockBot() - self.cog = silence.Silence(self.bot) - @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") @@ -303,17 +304,19 @@ class SilenceArgumentParserTests(RedisTestCase): @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) -class RescheduleTests(unittest.IsolatedAsyncioTestCase): +class RescheduleTests(RedisTestCase): """Tests for the rescheduling of cached unsilences.""" - @autospec(silence, "Scheduler", "SilenceNotifier", pass_mocks=False) - def setUp(self): + @autospec(silence, "Scheduler", pass_mocks=False) + def setUp(self) -> None: self.bot = MockBot() self.cog = silence.Silence(self.bot) self.cog._unsilence_wrapper = mock.create_autospec(self.cog._unsilence_wrapper) - with mock.patch.object(self.cog, "_reschedule", autospec=True): - asyncio.run(self.cog.cog_load()) # Populate instance attributes. + @autospec(silence, "SilenceNotifier", pass_mocks=False) + async def asyncSetUp(self) -> None: + await super().asyncSetUp() + await self.cog.cog_load() # Populate instance attributes. async def test_skipped_missing_channel(self): """Did nothing because the channel couldn't be retrieved.""" @@ -388,20 +391,14 @@ def voice_sync_helper(function): @autospec(silence.Silence, "previous_overwrites", "unsilence_timestamps", pass_mocks=False) -class SilenceTests(RedisTestCase): +class SilenceTests(SilenceTest): """Tests for the silence command and its related helper methods.""" - @autospec(silence.Silence, "_reschedule", pass_mocks=False) - @autospec(silence, "Scheduler", "SilenceNotifier", pass_mocks=False) def setUp(self) -> None: - self.bot = MockBot(get_channel=lambda _: MockTextChannel()) - self.cog = silence.Silence(self.bot) + super().setUp() # Avoid unawaited coroutine warnings. self.cog.scheduler.schedule_later.side_effect = lambda delay, task_id, coro: coro.close() - - asyncio.run(self.cog.cog_load()) # Populate instance attributes. - self.text_channel = MockTextChannel() self.text_overwrite = PermissionOverwrite( send_messages=True, @@ -659,22 +656,13 @@ class SilenceTests(RedisTestCase): @autospec(silence.Silence, "unsilence_timestamps", pass_mocks=False) -class UnsilenceTests(unittest.IsolatedAsyncioTestCase): +class UnsilenceTests(SilenceTest): """Tests for the unsilence command and its related helper methods.""" - @autospec(silence.Silence, "_reschedule", pass_mocks=False) - @autospec(silence, "Scheduler", "SilenceNotifier", pass_mocks=False) def setUp(self) -> None: - self.bot = MockBot(get_channel=lambda _: MockTextChannel()) - self.cog = silence.Silence(self.bot) - - overwrites_cache = mock.create_autospec(self.cog.previous_overwrites, spec_set=True) - self.cog.previous_overwrites = overwrites_cache - - asyncio.run(self.cog.cog_load()) # Populate instance attributes. + super().setUp() self.cog.scheduler.__contains__.return_value = True - overwrites_cache.get.return_value = '{"send_messages": true, "add_reactions": false}' self.text_channel = MockTextChannel() self.text_overwrite = PermissionOverwrite(send_messages=False, add_reactions=False) self.text_channel.overwrites_for.return_value = self.text_overwrite @@ -683,6 +671,13 @@ class UnsilenceTests(unittest.IsolatedAsyncioTestCase): self.voice_overwrite = PermissionOverwrite(connect=True, speak=True) self.voice_channel.overwrites_for.return_value = self.voice_overwrite + async def asyncSetUp(self) -> None: + await super().asyncSetUp() + overwrites_cache = mock.create_autospec(self.cog.previous_overwrites, spec_set=True) + self.cog.previous_overwrites = overwrites_cache + + overwrites_cache.get.return_value = '{"send_messages": true, "add_reactions": false}' + async def test_sent_correct_message(self): """Appropriate failure/success message was sent by the command.""" unsilenced_overwrite = PermissionOverwrite(send_messages=True, add_reactions=True) -- cgit v1.2.3 From 0e6242f7f4c41329e9270724a9780511f7165240 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 04:54:26 -0400 Subject: Updated tests - Refactored tests for new time duration arguments --- .../exts/moderation/infraction/test_infractions.py | 11 ++++---- tests/bot/exts/moderation/infraction/test_utils.py | 29 +++++++++++++--------- 2 files changed, 23 insertions(+), 17 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/infraction/test_infractions.py b/tests/bot/exts/moderation/infraction/test_infractions.py index 052048053..a18a4d23b 100644 --- a/tests/bot/exts/moderation/infraction/test_infractions.py +++ b/tests/bot/exts/moderation/infraction/test_infractions.py @@ -79,13 +79,13 @@ class VoiceMuteTests(unittest.IsolatedAsyncioTestCase): """Should call voice mute applying function without expiry.""" self.cog.apply_voice_mute = AsyncMock() self.assertIsNone(await self.cog.voicemute(self.cog, self.ctx, self.user, reason="foobar")) - self.cog.apply_voice_mute.assert_awaited_once_with(self.ctx, self.user, "foobar", expires_at=None) + self.cog.apply_voice_mute.assert_awaited_once_with(self.ctx, self.user, "foobar", duration_or_expiry=None) async def test_temporary_voice_mute(self): """Should call voice mute applying function with expiry.""" self.cog.apply_voice_mute = AsyncMock() self.assertIsNone(await self.cog.tempvoicemute(self.cog, self.ctx, self.user, "baz", reason="foobar")) - self.cog.apply_voice_mute.assert_awaited_once_with(self.ctx, self.user, "foobar", expires_at="baz") + self.cog.apply_voice_mute.assert_awaited_once_with(self.ctx, self.user, "foobar", duration_or_expiry="baz") async def test_voice_unmute(self): """Should call infraction pardoning function.""" @@ -189,7 +189,8 @@ class VoiceMuteTests(unittest.IsolatedAsyncioTestCase): user = MockUser() await self.cog.voicemute(self.cog, self.ctx, user, reason=None) - post_infraction_mock.assert_called_once_with(self.ctx, user, "voice_mute", None, active=True, expires_at=None) + post_infraction_mock.assert_called_once_with(self.ctx, user, "voice_mute", None, active=True, + duration_or_expiry=None) apply_infraction_mock.assert_called_once_with(self.cog, self.ctx, infraction, user, ANY) # Test action @@ -273,7 +274,7 @@ class CleanBanTests(unittest.IsolatedAsyncioTestCase): self.user, "FooBar", purge_days=1, - expires_at=None, + duration_or_expiry=None, ) async def test_cleanban_doesnt_purge_messages_if_clean_cog_available(self): @@ -285,7 +286,7 @@ class CleanBanTests(unittest.IsolatedAsyncioTestCase): self.ctx, self.user, "FooBar", - expires_at=None, + duration_or_expiry=None, ) @patch("bot.exts.moderation.infraction.infractions.Age") diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index 5cf02033d..4c78c0bd8 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -318,14 +318,17 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "user": self.member.id, "active": False, "expires_at": now.isoformat(), - "dm_sent": False + "dm_sent": False, + "last_applied": datetime(2020, 1, 1).isoformat(), } - self.ctx.bot.api_client.post.return_value = "foo" - actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) + # Patch the time.now(tz=timezone.utc) function to return a specific time + with patch("bot.exts.moderation.infraction._utils.datetime.now", return_value=datetime(2020, 1, 1)): + self.ctx.bot.api_client.post.return_value = "foo" + actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) - self.assertEqual(actual, "foo") - self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) + self.assertEqual(actual, "foo") + self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) async def test_unknown_error_post_infraction(self): """Should send an error message to chat when a non-400 error occurs.""" @@ -356,12 +359,14 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "type": "mute", "user": self.user.id, "active": True, - "dm_sent": False + "dm_sent": False, + "last_applied": datetime(2020, 1, 1), } - self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] - - actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") - self.assertEqual(actual, "foo") - self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) - post_user_mock.assert_awaited_once_with(self.ctx, self.user) + # Patch the time.now(tz=timezone.utc) function to return a specific time + with patch("bot.exts.moderation.infraction._utils.datetime.now", return_value=datetime(2020, 1, 1)): + self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] + actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") + self.assertEqual(actual, "foo") + self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) + post_user_mock.assert_awaited_once_with(self.ctx, self.user) -- cgit v1.2.3 From 035b5accf78f8623b40e0612e3c057f0ef2b93a7 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 05:04:38 -0400 Subject: Fixed test patches --- tests/bot/exts/moderation/infraction/test_utils.py | 32 ++++++++++++---------- 1 file changed, 17 insertions(+), 15 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index 4c78c0bd8..def06932b 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -307,7 +307,8 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.user = MockUser(id=1234) self.ctx = MockContext(bot=self.bot, author=self.member) - async def test_normal_post_infraction(self): + @patch("bot.exts.moderation.infraction._utils.datetime", wraps=datetime) + async def test_normal_post_infraction(self, mock_datetime): """Should return response from POST request if there are no errors.""" now = datetime.now() payload = { @@ -322,13 +323,13 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "last_applied": datetime(2020, 1, 1).isoformat(), } - # Patch the time.now(tz=timezone.utc) function to return a specific time - with patch("bot.exts.moderation.infraction._utils.datetime.now", return_value=datetime(2020, 1, 1)): - self.ctx.bot.api_client.post.return_value = "foo" - actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) + # Patch the datetime.now function to return a specific time + mock_datetime.now.return_value = datetime(2020, 1, 1) + self.ctx.bot.api_client.post.return_value = "foo" + actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) - self.assertEqual(actual, "foo") - self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) + self.assertEqual(actual, "foo") + self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) async def test_unknown_error_post_infraction(self): """Should send an error message to chat when a non-400 error occurs.""" @@ -349,8 +350,9 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.assertIsNone(actual) post_user_mock.assert_awaited_once_with(self.ctx, self.user) + @patch("bot.exts.moderation.infraction._utils.datetime", wraps=datetime) @patch("bot.exts.moderation.infraction._utils.post_user", return_value="bar") - async def test_first_fail_second_success_user_post_infraction(self, post_user_mock): + async def test_first_fail_second_success_user_post_infraction(self, post_user_mock, mock_datetime): """Should post the user if they don't exist, POST infraction again, and return the response if successful.""" payload = { "actor": self.ctx.author.id, @@ -363,10 +365,10 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "last_applied": datetime(2020, 1, 1), } - # Patch the time.now(tz=timezone.utc) function to return a specific time - with patch("bot.exts.moderation.infraction._utils.datetime.now", return_value=datetime(2020, 1, 1)): - self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] - actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") - self.assertEqual(actual, "foo") - self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) - post_user_mock.assert_awaited_once_with(self.ctx, self.user) + # Patch the datetime.now function to return a specific time + mock_datetime.now.return_value = datetime(2020, 1, 1) + self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] + actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") + self.assertEqual(actual, "foo") + self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) + post_user_mock.assert_awaited_once_with(self.ctx, self.user) -- cgit v1.2.3 From 8db7ef5df087e43804d07dabe2037af18adcb0d6 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 05:19:30 -0400 Subject: Added isoformat for test payload --- tests/bot/exts/moderation/infraction/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index def06932b..d3a908b28 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -362,7 +362,7 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "user": self.user.id, "active": True, "dm_sent": False, - "last_applied": datetime(2020, 1, 1), + "last_applied": datetime(2020, 1, 1).isoformat(), } # Patch the datetime.now function to return a specific time -- cgit v1.2.3 From 3280ac48a9031b727bdde69909729093593bd967 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Thu, 28 Jul 2022 06:11:11 -0400 Subject: Fixed tests - Corrected datetime patching --- tests/bot/exts/moderation/infraction/test_utils.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index d3a908b28..b1f23e31c 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -307,8 +307,7 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.user = MockUser(id=1234) self.ctx = MockContext(bot=self.bot, author=self.member) - @patch("bot.exts.moderation.infraction._utils.datetime", wraps=datetime) - async def test_normal_post_infraction(self, mock_datetime): + async def test_normal_post_infraction(self): """Should return response from POST request if there are no errors.""" now = datetime.now() payload = { @@ -320,16 +319,18 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "active": False, "expires_at": now.isoformat(), "dm_sent": False, - "last_applied": datetime(2020, 1, 1).isoformat(), } - # Patch the datetime.now function to return a specific time - mock_datetime.now.return_value = datetime(2020, 1, 1) self.ctx.bot.api_client.post.return_value = "foo" actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) self.assertEqual(actual, "foo") - self.ctx.bot.api_client.post.assert_awaited_once_with("bot/infractions", json=payload) + self.ctx.bot.api_client.post.assert_awaited_once() + await_args = str(self.ctx.bot.api_client.post.await_args) + # Check existing keys present, allow for additional keys (e.g. `last_applied`) + for key, value in payload.items(): + self.assertTrue(key in await_args) + self.assertTrue(str(value) in await_args) async def test_unknown_error_post_infraction(self): """Should send an error message to chat when a non-400 error occurs.""" -- cgit v1.2.3 From e74ff62122935da349d38a8c06eef14d1e3ba9aa Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 6 Aug 2022 13:47:49 -0400 Subject: Refactored test to not use datetime patch - Used new method of dict subset comparison instead of datetime patching for better compat. with argument types --- tests/bot/exts/moderation/infraction/test_utils.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index b1f23e31c..5ba0f4273 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -1,7 +1,7 @@ import unittest from collections import namedtuple from datetime import datetime -from unittest.mock import AsyncMock, MagicMock, call, patch +from unittest.mock import AsyncMock, MagicMock, patch from botcore.site_api import ResponseCodeError from discord import Embed, Forbidden, HTTPException, NotFound @@ -351,11 +351,10 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.assertIsNone(actual) post_user_mock.assert_awaited_once_with(self.ctx, self.user) - @patch("bot.exts.moderation.infraction._utils.datetime", wraps=datetime) @patch("bot.exts.moderation.infraction._utils.post_user", return_value="bar") - async def test_first_fail_second_success_user_post_infraction(self, post_user_mock, mock_datetime): + async def test_first_fail_second_success_user_post_infraction(self, post_user_mock): """Should post the user if they don't exist, POST infraction again, and return the response if successful.""" - payload = { + expected = { "actor": self.ctx.author.id, "hidden": False, "reason": "Test reason", @@ -363,13 +362,17 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): "user": self.user.id, "active": True, "dm_sent": False, - "last_applied": datetime(2020, 1, 1).isoformat(), } - # Patch the datetime.now function to return a specific time - mock_datetime.now.return_value = datetime(2020, 1, 1) self.bot.api_client.post.side_effect = [ResponseCodeError(MagicMock(status=400), {"user": "foo"}), "foo"] actual = await utils.post_infraction(self.ctx, self.user, "mute", "Test reason") self.assertEqual(actual, "foo") - self.bot.api_client.post.assert_has_awaits([call("bot/infractions", json=payload)] * 2) + await_args = self.bot.api_client.post.await_args_list + self.assertEqual(len(await_args), 2, "Expected 2 awaits") + + # Since `last_applied` is based on current time, just check if expected is a subset of payload + for args in await_args: + payload: dict = args.kwargs["json"] + self.assertEqual(payload, payload | expected) + post_user_mock.assert_awaited_once_with(self.ctx, self.user) -- cgit v1.2.3 From d93afa3dbe16404a60e23acaf394affcda5aff89 Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 6 Aug 2022 13:57:51 -0400 Subject: Updated previous tests to use subset method --- tests/bot/exts/moderation/infraction/test_utils.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index 5ba0f4273..6c9af2555 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -310,7 +310,7 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): async def test_normal_post_infraction(self): """Should return response from POST request if there are no errors.""" now = datetime.now() - payload = { + expected = { "actor": self.ctx.author.id, "hidden": True, "reason": "Test reason", @@ -323,14 +323,12 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): self.ctx.bot.api_client.post.return_value = "foo" actual = await utils.post_infraction(self.ctx, self.member, "ban", "Test reason", now, True, False) - self.assertEqual(actual, "foo") self.ctx.bot.api_client.post.assert_awaited_once() - await_args = str(self.ctx.bot.api_client.post.await_args) - # Check existing keys present, allow for additional keys (e.g. `last_applied`) - for key, value in payload.items(): - self.assertTrue(key in await_args) - self.assertTrue(str(value) in await_args) + + # Since `last_applied` is based on current time, just check if expected is a subset of payload + payload: dict = self.ctx.bot.api_client.post.await_args_list[0].kwargs["json"] + self.assertEqual(payload, payload | expected) async def test_unknown_error_post_infraction(self): """Should send an error message to chat when a non-400 error occurs.""" -- cgit v1.2.3 From 08a6bbc306d1ec54998b690eeeb258548e8e08fa Mon Sep 17 00:00:00 2001 From: ionite34 Date: Sat, 13 Aug 2022 15:45:52 -0400 Subject: Corrected test use of utcnow Corrected test case to use `datetime.utcnow()` to be consistent with target --- tests/bot/exts/moderation/infraction/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/bot/exts/moderation/infraction/test_utils.py b/tests/bot/exts/moderation/infraction/test_utils.py index 6c9af2555..29dadf372 100644 --- a/tests/bot/exts/moderation/infraction/test_utils.py +++ b/tests/bot/exts/moderation/infraction/test_utils.py @@ -309,7 +309,7 @@ class TestPostInfraction(unittest.IsolatedAsyncioTestCase): async def test_normal_post_infraction(self): """Should return response from POST request if there are no errors.""" - now = datetime.now() + now = datetime.utcnow() expected = { "actor": self.ctx.author.id, "hidden": True, -- cgit v1.2.3 From dada405211eac996196cdfb0496f4ff22f9a656a Mon Sep 17 00:00:00 2001 From: arl Date: Thu, 18 Aug 2022 19:01:22 -0400 Subject: fix: don't include replied mentions in mention filter (#2017) Co-authored-by: Izan Co-authored-by: TizzySaurus <47674925+TizzySaurus@users.noreply.github.com> Co-authored-by: Xithrius <15021300+Xithrius@users.noreply.github.com> --- bot/rules/mentions.py | 56 +++++++++++++++++++++++++++++++++----- tests/bot/rules/test_mentions.py | 58 ++++++++++++++++++++++++++++++++++++---- tests/helpers.py | 22 +++++++++++++++ 3 files changed, 124 insertions(+), 12 deletions(-) (limited to 'tests') diff --git a/bot/rules/mentions.py b/bot/rules/mentions.py index 6f5addad1..ca1d0c01c 100644 --- a/bot/rules/mentions.py +++ b/bot/rules/mentions.py @@ -1,23 +1,65 @@ from typing import Dict, Iterable, List, Optional, Tuple -from discord import Member, Message +from discord import DeletedReferencedMessage, Member, Message, MessageType, NotFound + +import bot +from bot.log import get_logger + +log = get_logger(__name__) async def apply( last_message: Message, recent_messages: List[Message], config: Dict[str, int] ) -> Optional[Tuple[str, Iterable[Member], Iterable[Message]]]: - """Detects total mentions exceeding the limit sent by a single user.""" + """ + Detects total mentions exceeding the limit sent by a single user. + + Excludes mentions that are bots, themselves, or replied users. + + In very rare cases, may not be able to determine a + mention was to a reply, in which case it is not ignored. + """ relevant_messages = tuple( msg for msg in recent_messages if msg.author == last_message.author ) + # We use `msg.mentions` here as that is supplied by the api itself, to determine who was mentioned. + # Additionally, `msg.mentions` includes the user replied to, even if the mention doesn't occur in the body. + # In order to exclude users who are mentioned as a reply, we check if the msg has a reference + # + # While we could use regex to parse the message content, and get a list of + # the mentions, that solution is very prone to breaking. + # We would need to deal with codeblocks, escaping markdown, and any discrepancies between + # our implementation and discord's markdown parser which would cause false positives or false negatives. + total_recent_mentions = 0 + for msg in relevant_messages: + # We check if the message is a reply, and if it is try to get the author + # since we ignore mentions of a user that we're replying to + reply_author = None - total_recent_mentions = sum( - not user.bot - for msg in relevant_messages - for user in msg.mentions - ) + if msg.type == MessageType.reply: + ref = msg.reference + + if not (resolved := ref.resolved): + # It is possible, in a very unusual situation, for a message to have a reference + # that is both not in the cache, and deleted while running this function. + # In such a situation, this will throw an error which we catch. + try: + resolved = await bot.instance.get_partial_messageable(resolved.channel_id).fetch_message( + resolved.message_id + ) + except NotFound: + log.info('Could not fetch the reference message as it has been deleted.') + + if resolved and not isinstance(resolved, DeletedReferencedMessage): + reply_author = resolved.author + + for user in msg.mentions: + # Don't count bot or self mentions, or the user being replied to (if applicable) + if user.bot or user in {msg.author, reply_author}: + continue + total_recent_mentions += 1 if total_recent_mentions > config['max']: return ( diff --git a/tests/bot/rules/test_mentions.py b/tests/bot/rules/test_mentions.py index f8805ac48..e1f904917 100644 --- a/tests/bot/rules/test_mentions.py +++ b/tests/bot/rules/test_mentions.py @@ -1,15 +1,32 @@ -from typing import Iterable +from typing import Iterable, Optional + +import discord from bot.rules import mentions from tests.bot.rules import DisallowedCase, RuleTest -from tests.helpers import MockMember, MockMessage +from tests.helpers import MockMember, MockMessage, MockMessageReference -def make_msg(author: str, total_user_mentions: int, total_bot_mentions: int = 0) -> MockMessage: - """Makes a message with `total_mentions` mentions.""" +def make_msg( + author: str, + total_user_mentions: int, + total_bot_mentions: int = 0, + *, + reference: Optional[MockMessageReference] = None +) -> MockMessage: + """Makes a message from `author` with `total_user_mentions` user mentions and `total_bot_mentions` bot mentions.""" user_mentions = [MockMember() for _ in range(total_user_mentions)] bot_mentions = [MockMember(bot=True) for _ in range(total_bot_mentions)] - return MockMessage(author=author, mentions=user_mentions+bot_mentions) + + mentions = user_mentions + bot_mentions + if reference is not None: + # For the sake of these tests we assume that all references are mentions. + mentions.append(reference.resolved.author) + msg_type = discord.MessageType.reply + else: + msg_type = discord.MessageType.default + + return MockMessage(author=author, mentions=mentions, reference=reference, type=msg_type) class TestMentions(RuleTest): @@ -56,6 +73,16 @@ class TestMentions(RuleTest): ("bob",), 3, ), + DisallowedCase( + [make_msg("bob", 3, reference=MockMessageReference())], + ("bob",), + 3, + ), + DisallowedCase( + [make_msg("bob", 3, reference=MockMessageReference(reference_author_is_bot=True))], + ("bob",), + 3 + ) ) await self.run_disallowed(cases) @@ -71,6 +98,27 @@ class TestMentions(RuleTest): await self.run_allowed(cases) + async def test_ignore_reply_mentions(self): + """Messages with an allowed amount of mentions in the content, also containing reply mentions.""" + cases = ( + [ + make_msg("bob", 2, reference=MockMessageReference()) + ], + [ + make_msg("bob", 2, reference=MockMessageReference(reference_author_is_bot=True)) + ], + [ + make_msg("bob", 2, reference=MockMessageReference()), + make_msg("bob", 0, reference=MockMessageReference()) + ], + [ + make_msg("bob", 2, reference=MockMessageReference(reference_author_is_bot=True)), + make_msg("bob", 0, reference=MockMessageReference(reference_author_is_bot=True)) + ] + ) + + await self.run_allowed(cases) + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: last_message = case.recent_messages[0] return tuple( diff --git a/tests/helpers.py b/tests/helpers.py index 17214553c..687e15b96 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -492,6 +492,28 @@ class MockAttachment(CustomMockMixin, unittest.mock.MagicMock): spec_set = attachment_instance +message_reference_instance = discord.MessageReference( + message_id=unittest.mock.MagicMock(id=1), + channel_id=unittest.mock.MagicMock(id=2), + guild_id=unittest.mock.MagicMock(id=3) +) + + +class MockMessageReference(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock MessageReference objects. + + Instances of this class will follow the specification of `discord.MessageReference` instances. + For more information, see the `MockGuild` docstring. + """ + spec_set = message_reference_instance + + def __init__(self, *, reference_author_is_bot: bool = False, **kwargs): + super().__init__(**kwargs) + referenced_msg_author = MockMember(name="bob", bot=reference_author_is_bot) + self.resolved = MockMessage(author=referenced_msg_author) + + class MockMessage(CustomMockMixin, unittest.mock.MagicMock): """ A MagicMock subclass to mock Message objects. -- cgit v1.2.3 From 8028690d4eae27e57dfe1429b8067eabaa94eef9 Mon Sep 17 00:00:00 2001 From: Aleksey Zasorin Date: Fri, 16 Sep 2022 10:42:16 -0700 Subject: Removed "redis_ready" from additional_spec_asyncs in MockBot (#2275) The attribute was removed from Bot in fc05849 --- tests/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/helpers.py b/tests/helpers.py index 687e15b96..a4b919dcb 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -317,7 +317,7 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): guild_id=1, intents=discord.Intents.all(), ) - additional_spec_asyncs = ("wait_for", "redis_ready") + additional_spec_asyncs = ("wait_for",) def __init__(self, **kwargs) -> None: super().__init__(**kwargs) -- cgit v1.2.3