From c989eae4d2165c9329379e6ce700a6015b197b8c Mon Sep 17 00:00:00 2001 From: Fabian Williams Date: Fri, 27 Feb 2026 16:52:14 +0100 Subject: [PATCH 1/4] test: add superstarify tests (#14) Add test cases for retrying cog loads and skeleton for new functions --- .../moderation/infraction/superstarify.py | 11 ++- .../infraction/test_superstarify_cog.py | 82 +++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 tests/bot/exts/moderation/infraction/test_superstarify_cog.py diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 006334755d..57b5733a39 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -10,6 +10,7 @@ from bot import constants from bot.bot import Bot +from bot.constants import URLs from bot.converters import Duration, DurationOrExpiry from bot.decorators import ensure_future_timestamp from bot.exts.moderation.infraction import _utils @@ -18,6 +19,7 @@ from bot.utils import time from bot.utils.messages import format_user +MAX_RETRY_ATTEMPTS = URLs.connect_max_retries log = get_logger(__name__) NICKNAME_POLICY_URL = "https://pythondiscord.com/pages/rules/#nickname-policy" SUPERSTARIFY_DEFAULT_DURATION = "1h" @@ -238,7 +240,14 @@ async def cog_check(self, ctx: Context) -> bool: """Only allow moderators to invoke the commands in this cog.""" return await has_any_role(*constants.MODERATION_ROLES).predicate(ctx) - + async def _fetch_with_retries(self, + retries: int = MAX_RETRY_ATTEMPTS, + params: dict[str, str] | None = None) -> list[dict]: + return None + async def _alert_mods_if_loading_failed(self, error: Exception) -> None: + pass + async def _check_error_is_retriable(self, error: Exception) -> bool: + return False async def setup(bot: Bot) -> None: """Load the Superstarify cog.""" await bot.add_cog(Superstarify(bot)) diff --git a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py new file mode 100644 index 0000000000..847ce43ba4 --- /dev/null +++ b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py @@ -0,0 +1,82 @@ +import unittest +from unittest.mock import AsyncMock, MagicMock, patch + +from bot.exts.moderation.infraction.superstarify import Superstarify +from tests.helpers import MockBot + + +class TestSuperstarify(unittest.IsolatedAsyncioTestCase): + + async def asyncSetUp(self): + self.bot = MockBot() + + self.cog = Superstarify(self.bot) + + self.bot.api_client = MagicMock() + self.bot.api_client.get = AsyncMock() + + self.cog._alert_mods_if_loading_failed = AsyncMock() + self.cog._check_error_is_retriable = MagicMock(return_value=True) + + async def test_fetch_from_api_success(self): + """API succeeds on first attempt.""" + expected = [{"id": 1}] + self.bot.api_client.get.return_value = expected + + result = await self.cog._fetch_with_retries( + params={"user__id": "123"} + ) + self.assertEqual(result, expected) + + self.bot.api_client.get.assert_awaited_once_with( + "bot/infractions", + params={"user__id": "123"}, + ) + self.cog._alert_mods_if_loading_failed.assert_not_called() + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_fetch_retries_then_succeeds(self, _): + self.bot.api_client.get.side_effect = [ + OSError("temporary failure"), + [{"id": 42}], + ] + + result = await self.cog._fetch_with_retries( + params={"user__id": "123"} + ) + + self.assertEqual(result, [{"id": 42}]) + self.assertEqual(self.bot.api_client.get.await_count, 2) + + self.cog._alert_mods_if_loading_failed.assert_not_called() + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_fetch_fails_after_max_retries(self, _): + error = OSError("API down") + + self.bot.api_client.get.side_effect = error + + with self.assertRaises(OSError): + await self.cog._fetch_with_retries( + retries=3, + params={"user__id": "123"}, + ) + + self.assertEqual(self.bot.api_client.get.await_count, 3) + + self.cog._alert_mods_if_loading_failed.assert_awaited_once_with(error) + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_non_retriable_error_stops_immediately(self, _): + error = ValueError("bad request") + + self.bot.api_client.get.side_effect = error + self.cog._check_error_is_retriable.return_value = False + + with self.assertRaises(ValueError): + await self.cog._fetch_with_retries() + + # only one attempt + self.bot.api_client.get.assert_awaited_once() + + self.cog._alert_mods_if_loading_failed.assert_awaited_once() From 81f8d34c84e2d81de2ddcad500897e2c02f283fe Mon Sep 17 00:00:00 2001 From: Fabian Williams Date: Fri, 27 Feb 2026 17:05:54 +0100 Subject: [PATCH 2/4] feat: add logic to functions (#14) Implement skeleton functions with code for retrying fetch, alerting mods, and checking if retryable --- .../moderation/infraction/superstarify.py | 51 +++++++++++++++---- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 57b5733a39..8139eaa37d 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -1,16 +1,19 @@ +import asyncio import json import random import textwrap from pathlib import Path +import discord from discord import Embed, Member from discord.ext.commands import Cog, Context, command, has_any_role from discord.utils import escape_markdown +from pydis_core.site_api import ResponseCodeError from pydis_core.utils.members import get_or_fetch_member from bot import constants from bot.bot import Bot -from bot.constants import URLs +from bot.constants import Icons, URLs from bot.converters import Duration, DurationOrExpiry from bot.decorators import ensure_future_timestamp from bot.exts.moderation.infraction import _utils @@ -18,8 +21,10 @@ from bot.log import get_logger from bot.utils import time from bot.utils.messages import format_user +from bot.utils.modlog import send_log_message MAX_RETRY_ATTEMPTS = URLs.connect_max_retries +BACKOFF_INITIAL_DELAY = 5 # seconds log = get_logger(__name__) NICKNAME_POLICY_URL = "https://pythondiscord.com/pages/rules/#nickname-policy" SUPERSTARIFY_DEFAULT_DURATION = "1h" @@ -45,9 +50,7 @@ async def on_member_update(self, before: Member, after: Member) -> None: f"{after.display_name}. Checking if the user is in superstar-prison..." ) - active_superstarifies = await self.bot.api_client.get( - "bot/infractions", - params={ + active_superstarifies = await self._fetch_with_retries(params={ "active": "true", "type": "superstar", "user__id": str(before.id) @@ -86,9 +89,7 @@ async def on_member_update(self, before: Member, after: Member) -> None: @Cog.listener() async def on_member_join(self, member: Member) -> None: """Reapply active superstar infractions for returning members.""" - active_superstarifies = await self.bot.api_client.get( - "bot/infractions", - params={ + active_superstarifies = await self._fetch_with_retries(params={ "active": "true", "type": "superstar", "user__id": member.id @@ -243,11 +244,43 @@ async def cog_check(self, ctx: Context) -> bool: async def _fetch_with_retries(self, retries: int = MAX_RETRY_ATTEMPTS, params: dict[str, str] | None = None) -> list[dict]: + """Fetch infractions from the API with retries and exponential backoff.""" + for attempt in range(retries): + try: + return await self.bot.api_client.get("bot/infractions", params=params) + except Exception as e: + if attempt == retries - 1 or not self._check_error_is_retriable(e): + await self._alert_mods_if_loading_failed(e) + raise + await asyncio.sleep(BACKOFF_INITIAL_DELAY * (2 ** (attempt - 1))) return None + async def _alert_mods_if_loading_failed(self, error: Exception) -> None: - pass + """Alert moderators that loading the superstarify cog failed after retries.""" + message = textwrap.dedent( + f""" + An error occurred while loading the Superstarify Cog, and it failed to initialize properly. + + Error details: + {error} + """ + ) + + await send_log_message( + self.bot, + title="Error: Failed to initialize the Superstarify Cog", + text=message, + ping_everyone=True, + icon_url=Icons.token_removed, + colour=discord.Color.red() + ) async def _check_error_is_retriable(self, error: Exception) -> bool: - return False + """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" + if isinstance(error, ResponseCodeError): + return error.status in (408, 429) or error.status >= 500 + + return isinstance(error, (TimeoutError, OSError)) + async def setup(bot: Bot) -> None: """Load the Superstarify cog.""" await bot.add_cog(Superstarify(bot)) From 3b8d7eb9f64aa8997d5d018a8ad12a2ce91d9c6a Mon Sep 17 00:00:00 2001 From: Fabian Williams Date: Fri, 27 Feb 2026 17:20:30 +0100 Subject: [PATCH 3/4] test: more unit tests for superstarify (#14) Add unit test for on_member_update and unit test to check _alert_mods_if_loading_failed is being called --- .../infraction/test_superstarify_cog.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py index 847ce43ba4..4e3004ce6b 100644 --- a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py +++ b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py @@ -80,3 +80,33 @@ async def test_non_retriable_error_stops_immediately(self, _): self.bot.api_client.get.assert_awaited_once() self.cog._alert_mods_if_loading_failed.assert_awaited_once() + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_member_update_recovers_from_api_failure(self, _): + before = MagicMock(display_name="Old", id=123) + after = MagicMock(display_name="New", id=123) + after.edit = AsyncMock() + + self.bot.api_client.get.side_effect = [ + OSError(), + [{"id": 42}], + ] + + self.cog.get_nick = MagicMock(return_value="Taylor Swift") + + with patch( + "bot.exts.moderation.infraction._utils.notify_infraction", + new=AsyncMock(return_value=True), + ): + await self.cog.on_member_update(before, after) + + after.edit.assert_awaited_once() + + @patch("asyncio.sleep", new_callable=AsyncMock) + async def test_alert_triggered_after_total_failure(self, _): + self.bot.api_client.get.side_effect = OSError("down") + + with self.assertRaises(OSError): + await self.cog._fetch_with_retries(retries=3) + + self.cog._alert_mods_if_loading_failed.assert_awaited_once() From 4e203c2bb0d8b55b183196ac905a9eddbe2782ce Mon Sep 17 00:00:00 2001 From: Fabian Williams Date: Sun, 1 Mar 2026 14:53:15 +0100 Subject: [PATCH 4/4] refactor: remove _alert_mods_if_loading_failed() (#14) Remove redundant code and corresponding parts of unit tests. --- .../moderation/infraction/superstarify.py | 24 +------------------ .../infraction/test_superstarify_cog.py | 7 ------ 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/bot/exts/moderation/infraction/superstarify.py b/bot/exts/moderation/infraction/superstarify.py index 8139eaa37d..180a49d304 100644 --- a/bot/exts/moderation/infraction/superstarify.py +++ b/bot/exts/moderation/infraction/superstarify.py @@ -4,7 +4,6 @@ import textwrap from pathlib import Path -import discord from discord import Embed, Member from discord.ext.commands import Cog, Context, command, has_any_role from discord.utils import escape_markdown @@ -13,7 +12,7 @@ from bot import constants from bot.bot import Bot -from bot.constants import Icons, URLs +from bot.constants import URLs from bot.converters import Duration, DurationOrExpiry from bot.decorators import ensure_future_timestamp from bot.exts.moderation.infraction import _utils @@ -21,7 +20,6 @@ from bot.log import get_logger from bot.utils import time from bot.utils.messages import format_user -from bot.utils.modlog import send_log_message MAX_RETRY_ATTEMPTS = URLs.connect_max_retries BACKOFF_INITIAL_DELAY = 5 # seconds @@ -250,30 +248,10 @@ async def _fetch_with_retries(self, return await self.bot.api_client.get("bot/infractions", params=params) except Exception as e: if attempt == retries - 1 or not self._check_error_is_retriable(e): - await self._alert_mods_if_loading_failed(e) raise await asyncio.sleep(BACKOFF_INITIAL_DELAY * (2 ** (attempt - 1))) return None - async def _alert_mods_if_loading_failed(self, error: Exception) -> None: - """Alert moderators that loading the superstarify cog failed after retries.""" - message = textwrap.dedent( - f""" - An error occurred while loading the Superstarify Cog, and it failed to initialize properly. - - Error details: - {error} - """ - ) - - await send_log_message( - self.bot, - title="Error: Failed to initialize the Superstarify Cog", - text=message, - ping_everyone=True, - icon_url=Icons.token_removed, - colour=discord.Color.red() - ) async def _check_error_is_retriable(self, error: Exception) -> bool: """Return whether loading filter lists failed due to some temporary error, thus retrying could help.""" if isinstance(error, ResponseCodeError): diff --git a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py index 4e3004ce6b..54473c7064 100644 --- a/tests/bot/exts/moderation/infraction/test_superstarify_cog.py +++ b/tests/bot/exts/moderation/infraction/test_superstarify_cog.py @@ -15,7 +15,6 @@ async def asyncSetUp(self): self.bot.api_client = MagicMock() self.bot.api_client.get = AsyncMock() - self.cog._alert_mods_if_loading_failed = AsyncMock() self.cog._check_error_is_retriable = MagicMock(return_value=True) async def test_fetch_from_api_success(self): @@ -32,7 +31,6 @@ async def test_fetch_from_api_success(self): "bot/infractions", params={"user__id": "123"}, ) - self.cog._alert_mods_if_loading_failed.assert_not_called() @patch("asyncio.sleep", new_callable=AsyncMock) async def test_fetch_retries_then_succeeds(self, _): @@ -48,7 +46,6 @@ async def test_fetch_retries_then_succeeds(self, _): self.assertEqual(result, [{"id": 42}]) self.assertEqual(self.bot.api_client.get.await_count, 2) - self.cog._alert_mods_if_loading_failed.assert_not_called() @patch("asyncio.sleep", new_callable=AsyncMock) async def test_fetch_fails_after_max_retries(self, _): @@ -64,7 +61,6 @@ async def test_fetch_fails_after_max_retries(self, _): self.assertEqual(self.bot.api_client.get.await_count, 3) - self.cog._alert_mods_if_loading_failed.assert_awaited_once_with(error) @patch("asyncio.sleep", new_callable=AsyncMock) async def test_non_retriable_error_stops_immediately(self, _): @@ -79,7 +75,6 @@ async def test_non_retriable_error_stops_immediately(self, _): # only one attempt self.bot.api_client.get.assert_awaited_once() - self.cog._alert_mods_if_loading_failed.assert_awaited_once() @patch("asyncio.sleep", new_callable=AsyncMock) async def test_member_update_recovers_from_api_failure(self, _): @@ -108,5 +103,3 @@ async def test_alert_triggered_after_total_failure(self, _): with self.assertRaises(OSError): await self.cog._fetch_with_retries(retries=3) - - self.cog._alert_mods_if_loading_failed.assert_awaited_once()