From fb834f35822aba50ce0bbdb81e491cd9bef9dd8e Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 05:38:12 -0500 Subject: [PATCH 01/26] Move Memebot code to `memebot` package Python projects typically do not have a `src` directory. The code is typically all located in a root pacakge under the same name as the project. --- {src/integrations => memebot}/__init__.py | 0 src/memebot.py => memebot/client.py | 20 ++++++------- {src => memebot}/commands/__init__.py | 0 {src => memebot}/commands/hello.py | 0 {src => memebot}/commands/poll.py | 5 ++-- {src => memebot}/commands/role.py | 30 +++++++++++++------ {src => memebot}/config/__init__.py | 5 +--- {src => memebot}/config/mongod.yaml | 0 {src => memebot}/config/validators.py | 0 {src => memebot}/db/__init__.py | 2 +- {src => memebot}/db/internals.py | 2 +- {src/lib => memebot/integrations}/__init__.py | 0 {src => memebot}/integrations/twitter.py | 29 ++++++++---------- memebot/lib/__init__.py | 0 {src => memebot}/lib/constants.py | 0 {src => memebot}/lib/exception.py | 0 {src => memebot}/lib/util.py | 24 ++++++++------- {src => memebot}/log/__init__.py | 2 +- {src => memebot}/log/formatter.py | 0 {src => memebot}/log/logger.py | 2 +- {src => memebot}/main.py | 10 +++---- 21 files changed, 70 insertions(+), 61 deletions(-) rename {src/integrations => memebot}/__init__.py (100%) rename src/memebot.py => memebot/client.py (89%) rename {src => memebot}/commands/__init__.py (100%) rename {src => memebot}/commands/hello.py (100%) rename {src => memebot}/commands/poll.py (97%) rename {src => memebot}/commands/role.py (90%) rename {src => memebot}/config/__init__.py (98%) rename {src => memebot}/config/mongod.yaml (100%) rename {src => memebot}/config/validators.py (100%) rename {src => memebot}/db/__init__.py (94%) rename {src => memebot}/db/internals.py (96%) rename {src/lib => memebot/integrations}/__init__.py (100%) rename {src => memebot}/integrations/twitter.py (89%) create mode 100644 memebot/lib/__init__.py rename {src => memebot}/lib/constants.py (100%) rename {src => memebot}/lib/exception.py (100%) rename {src => memebot}/lib/util.py (77%) rename {src => memebot}/log/__init__.py (98%) rename {src => memebot}/log/formatter.py (100%) rename {src => memebot}/log/logger.py (99%) rename {src => memebot}/main.py (67%) diff --git a/src/integrations/__init__.py b/memebot/__init__.py similarity index 100% rename from src/integrations/__init__.py rename to memebot/__init__.py diff --git a/src/memebot.py b/memebot/client.py similarity index 89% rename from src/memebot.py rename to memebot/client.py index e99f62d..6379685 100644 --- a/src/memebot.py +++ b/memebot/client.py @@ -3,12 +3,12 @@ import discord import discord.ext.commands -import commands -import config -import db -import log -from integrations import twitter -from lib import exception, util +from memebot import commands +from memebot import config +from memebot import db +from memebot import log +from memebot.integrations import twitter +from memebot.lib import exception, util async def on_ready() -> None: @@ -36,7 +36,7 @@ async def on_interaction(interaction: discord.Interaction) -> None: """ log.interaction( interaction, - f"{util.parse_invocation(interaction.data)} from {interaction.user}", + f"{util.parse_invocation(interaction)} from {interaction.user}", ) @@ -48,7 +48,7 @@ async def on_command_error( log.exception(error) return - invocation = util.parse_invocation(interaction.data) + invocation = util.parse_invocation(interaction) if isinstance(error, exception.MemebotInternalError): # For intentionally thrown internal errors @@ -85,7 +85,7 @@ async def on_command_error( memebot = discord.ext.commands.Bot( - command_prefix="!", + command_prefix="/", intents=discord.Intents().all(), activity=discord.Game(name="• /hello"), ) @@ -99,5 +99,3 @@ async def on_command_error( memebot.tree.error(on_command_error) if config.twitter_enabled: memebot.add_listener(twitter.process_message_for_interaction, "on_message") - -run = memebot.run diff --git a/src/commands/__init__.py b/memebot/commands/__init__.py similarity index 100% rename from src/commands/__init__.py rename to memebot/commands/__init__.py diff --git a/src/commands/hello.py b/memebot/commands/hello.py similarity index 100% rename from src/commands/hello.py rename to memebot/commands/hello.py diff --git a/src/commands/poll.py b/memebot/commands/poll.py similarity index 97% rename from src/commands/poll.py rename to memebot/commands/poll.py index 18adbf9..8fd7b41 100644 --- a/src/commands/poll.py +++ b/memebot/commands/poll.py @@ -6,7 +6,7 @@ import discord.ext.commands import emoji -from lib import constants, exception +from memebot.lib import constants, exception @discord.app_commands.command( @@ -35,7 +35,8 @@ async def poll( yes_no = False if len(choices) == 1: raise exception.MemebotUserError( - f"_Only 1 choice provided. {interaction.command.qualified_name} requires either 0 or 2+ choices!_" + f"_Only 1 choice provided. {interaction.command.qualified_name} " + f"requires either 0 or 2+ choices!_" ) elif len(choices) == 0 or [c.lower() for c in choices] in ( ["yes", "no"], diff --git a/src/commands/role.py b/memebot/commands/role.py similarity index 90% rename from src/commands/role.py rename to memebot/commands/role.py index 5b07e1b..b9b019a 100644 --- a/src/commands/role.py +++ b/memebot/commands/role.py @@ -1,9 +1,8 @@ -from code import interact -from typing import Optional, Any, Union, Annotated +from typing import Optional, Any, Union import discord -from lib import exception +from memebot.lib import exception class RoleActionError(exception.MemebotUserError): @@ -37,6 +36,18 @@ def __init__(self, action: str, target_name: str, *args: Any) -> None: ) +class RoleFailure(exception.MemebotInternalError): + def __init__(self, action: str, target_name: str, *args: Any) -> None: + """ + Unexpected, but handled internal failure + """ + super(RoleFailure, self).__init__( + f"Failed to {action} role `@{target_name}. " + f"It seems that Discord's API is having problems.", + *args, + ) + + class RoleLocationError(exception.MemebotUserError): """ Generic message for role commands which are executed in DMs @@ -106,8 +117,8 @@ async def create(interaction: discord.Interaction, role_name: str) -> None: ) except discord.Forbidden: raise RolePermissionError("create", target_name) - except (discord.HTTPException, TypeError): - raise RoleActionError("create", target_name) + except discord.HTTPException: + raise RoleFailure("create", target_name) await interaction.response.send_message(f"Created new role {new_role.mention}!") @@ -133,7 +144,7 @@ async def delete( except discord.Forbidden: raise RolePermissionError("delete", target_role.name) except discord.HTTPException: - raise RoleActionError("delete", target_role.name) + raise RoleFailure("delete", target_role.name) await interaction.response.send_message(f"Deleted role `@{target_role.name}`") @@ -161,7 +172,7 @@ async def join( except discord.Forbidden: raise RolePermissionError("join", target_role.name) except discord.HTTPException: - raise RoleActionError("join", target_role.name) + raise RoleFailure("join", target_role.name) await interaction.response.send_message( f"{author.name} successfully joined `@{target_role.name}`" @@ -192,7 +203,7 @@ async def leave( except discord.Forbidden: raise RolePermissionError("leave", target_role.name) except discord.HTTPException: - raise RoleActionError("leave", target_role.name) + raise RoleFailure("leave", target_role.name) await interaction.response.send_message( f"{author.name} successfully left `@{target_role.name}`" @@ -205,7 +216,8 @@ async def role_list( target: Optional[Union[discord.Role, discord.Member]], ) -> None: """ - List all roles managed by Memebot, or all members of a role managed by Memebot. + List all roles managed by Memebot, all managed roles of which a user is a member, + or all members of a role managed by Memebot. """ if not interaction.guild: raise RoleLocationError diff --git a/src/config/__init__.py b/memebot/config/__init__.py similarity index 98% rename from src/config/__init__.py rename to memebot/config/__init__.py index 04716e5..f15a87c 100644 --- a/src/config/__init__.py +++ b/memebot/config/__init__.py @@ -3,7 +3,7 @@ import os import urllib.parse -from config import validators +from memebot.config import validators # Discord API token discord_api_token: str @@ -148,6 +148,3 @@ def populate_config_from_command_line() -> None: global database_uri database_enabled = args.database_enabled database_uri = args.database_uri - - -populate_config_from_command_line() diff --git a/src/config/mongod.yaml b/memebot/config/mongod.yaml similarity index 100% rename from src/config/mongod.yaml rename to memebot/config/mongod.yaml diff --git a/src/config/validators.py b/memebot/config/validators.py similarity index 100% rename from src/config/validators.py rename to memebot/config/validators.py diff --git a/src/db/__init__.py b/memebot/db/__init__.py similarity index 94% rename from src/db/__init__.py rename to memebot/db/__init__.py index 15cddd1..456c961 100644 --- a/src/db/__init__.py +++ b/memebot/db/__init__.py @@ -1,4 +1,4 @@ -import config +from memebot import config from .internals import DatabaseInternals db_internals = DatabaseInternals() diff --git a/src/db/internals.py b/memebot/db/internals.py similarity index 96% rename from src/db/internals.py rename to memebot/db/internals.py index 92f2aa2..5a68969 100644 --- a/src/db/internals.py +++ b/memebot/db/internals.py @@ -2,7 +2,7 @@ import pymongo as mongo -import config +from memebot import config class DatabaseInternals: diff --git a/src/lib/__init__.py b/memebot/integrations/__init__.py similarity index 100% rename from src/lib/__init__.py rename to memebot/integrations/__init__.py diff --git a/src/integrations/twitter.py b/memebot/integrations/twitter.py similarity index 89% rename from src/integrations/twitter.py rename to memebot/integrations/twitter.py index 90be155..0c6a8cc 100644 --- a/src/integrations/twitter.py +++ b/memebot/integrations/twitter.py @@ -1,10 +1,11 @@ """ -This is a module for managing our integration with Twitter. It owns all metadata related to Twitter, and the only state -it maintains is whether or not the API has been initialized. +This is a module for managing our integration with Twitter. +It owns all metadata related to Twitter, +and the only state it maintains is whether the API has been initialized. -All functions that make any network communication, except for init, should be asynchronous and stateless. +All functions that make any network communication, except for init, +should be asynchronous and stateless. """ -import asyncio import re from typing import Tuple @@ -12,9 +13,9 @@ import emoji import tweepy -import config -from lib import util -from lib.exception import MemebotInternalError +from memebot import config +from memebot.lib import util +from memebot.lib.exception import MemebotInternalError # Regular expression that describes the pattern of a Tweet URL twitter_url_pattern = re.compile( @@ -155,9 +156,7 @@ async def process_message_for_interaction(message: discord.Message) -> None: # React with a numeric emoji to Tweets containing multiple images if (n_images := len(tweet_media)) > 1: - asyncio.create_task( - message.add_reaction(emoji.emojize(f":keycap_{n_images}:")) - ) + await message.add_reaction(emoji.emojize(f":keycap_{n_images}:")) # For tweets containing only a single video, we embed the video in Discord. elif n_images == 1 and tweet_media[0].type in ("video", "animated_gif"): video_url = max( @@ -170,14 +169,12 @@ async def process_message_for_interaction(message: discord.Message) -> None: # Choose the variant with the highest bitrate key=lambda v: int(v.get("bitrate", -1)), )["url"] - asyncio.create_task( - message.channel.send( - f"embedded video:\n" - f"{util.maybe_make_link_spoiler(video_url, spoiled)}" - ) + await message.channel.send( + f"embedded video:\n" + f"{util.maybe_make_link_spoiler(video_url, spoiled)}" ) # Post quote tweet links. if is_quote_tweet(tweet_info) and message.author != bot_user: quote_tweet_urls = get_quote_tweet_urls(tweet_info, spoiled) - asyncio.create_task(message.channel.send(quote_tweet_urls)) + await message.channel.send(quote_tweet_urls) diff --git a/memebot/lib/__init__.py b/memebot/lib/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/lib/constants.py b/memebot/lib/constants.py similarity index 100% rename from src/lib/constants.py rename to memebot/lib/constants.py diff --git a/src/lib/exception.py b/memebot/lib/exception.py similarity index 100% rename from src/lib/exception.py rename to memebot/lib/exception.py diff --git a/src/lib/util.py b/memebot/lib/util.py similarity index 77% rename from src/lib/util.py rename to memebot/lib/util.py index 643ecca..9d66433 100644 --- a/src/lib/util.py +++ b/memebot/lib/util.py @@ -1,7 +1,10 @@ from typing import Tuple, List, Optional, cast, TYPE_CHECKING +import discord +import discord.ext.commands + if TYPE_CHECKING: - from discord.types.interactions import InteractionData + pass def is_spoil(message: str, idx: int) -> bool: @@ -48,27 +51,28 @@ def maybe_make_link_spoiler(message: str, spoil: bool) -> str: return f"|| {message} ||" if spoil else message -def parse_invocation(interaction_data: Optional["InteractionData"]) -> str: +def parse_invocation(interaction: discord.Interaction) -> str: """Returns the command involacion parsed from the raw interaction data""" def impl(data: dict) -> str: out = [] name: Optional[str] = data.get("name") - options: Optional[list[dict]] = data.get("options") + options: list[dict] = data.get("options", []) value: Optional[str] = data.get("value") if value: out.append(value) elif name: out.append(name) - if options: - for option in options: - out.append(impl(option)) + for option in options: + out.append(impl(option)) return " ".join(out) - if not interaction_data: + if not interaction: return "" - # The interaction data is a TypedDict, which behaves like a dict at runtime, but - # due to discord.py's internal type checking, mypy doesn't like to view it as a dict. - return "/" + impl(cast(dict, interaction_data)) + if interaction.command: + prefix = cast(discord.ext.commands.Bot, interaction.client).command_prefix + else: + prefix = "" + return f"{prefix}{impl(cast(dict, interaction.data))}" diff --git a/src/log/__init__.py b/memebot/log/__init__.py similarity index 98% rename from src/log/__init__.py rename to memebot/log/__init__.py index 63f8580..e503dfe 100644 --- a/src/log/__init__.py +++ b/memebot/log/__init__.py @@ -3,7 +3,7 @@ import logging from typing import cast, Any, TYPE_CHECKING -import config +from memebot import config from . import formatter, logger config.log_location.setFormatter(formatter.MemeBotLogFormatter()) diff --git a/src/log/formatter.py b/memebot/log/formatter.py similarity index 100% rename from src/log/formatter.py rename to memebot/log/formatter.py diff --git a/src/log/logger.py b/memebot/log/logger.py similarity index 99% rename from src/log/logger.py rename to memebot/log/logger.py index 72a6e6a..04c8d7b 100644 --- a/src/log/logger.py +++ b/memebot/log/logger.py @@ -6,7 +6,7 @@ import sys from typing import Union, Mapping, Optional -import config +from memebot import config memebot_context = os.getcwd() diff --git a/src/main.py b/memebot/main.py similarity index 67% rename from src/main.py rename to memebot/main.py index cc659eb..813ed9e 100644 --- a/src/main.py +++ b/memebot/main.py @@ -1,9 +1,9 @@ -# In order to ensure consistent logging, -# log must always be the first thing imported by the main module -import log +from memebot import config -import config -import memebot +config.populate_config_from_command_line() + +from memebot.client import memebot +from memebot import log def main() -> None: From 3d6e720aa808341931c682302037d15a5f2fc92b Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 05:38:44 -0500 Subject: [PATCH 02/26] Update test requirements to support new tests --- tests/requirements.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/requirements.txt b/tests/requirements.txt index 47df967..e7045e8 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -1,2 +1,5 @@ +dpytest==0.6.1 mypy==0.961 -pytest==7.1.2 +pytest==7.2.1 +pytest-asyncio==0.20.3 +pytest-mock==3.10.0 From 3129334a432ca4c05a1d9f863d673da311ef451f Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 05:39:49 -0500 Subject: [PATCH 03/26] Update docker-compose.yaml and README to reflect new `memebot` package --- README.md | 12 ++++++------ docker-compose.yaml | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 7fbe0f9..7083c61 100644 --- a/README.md +++ b/README.md @@ -90,31 +90,31 @@ To run mypy locally, ensure it is installed to the same python environment as al Memebot dependencies, and then run it using the proper interpreter. ```shell -$ venv/bin/mypy src +$ venv/bin/mypy memebot # OR $ source venv/bin/activate -$ mypy src +$ mypy memebot ``` To run mypy in Docker, ensure you are using an image built from the `test` target. ```shell -$ docker run --rm -it --entrypoint mypy memebot:test src +$ docker run --rm -it --entrypoint mypy memebot:test memebot # OR -$ docker-compose run --rm --entrypoint mypy bot src +$ docker-compose run --rm --entrypoint mypy bot memebot ``` You can speed up subsequent runs of mypy by mounting the `.mypy-cache` directory as a volume. This way, mypy can reuse the cache it generates inside the container on the next run. ```shell -$ docker run --rm --volume "$(pwd)/.mypy_cache:/opt/memebot/.mypy_cache" --entrypoint mypy -it memebot:test src +$ docker run --rm --volume "$(pwd)/.mypy_cache:/opt/memebot/.mypy_cache" --entrypoint mypy -it memebot:test memebot # OR -$ docker-compose run --rm --volume "$(pwd)/.mypy_cache:/opt/memebot/.mypy_cache" --entrypoint mypy bot src +$ docker-compose run --rm --volume "$(pwd)/.mypy_cache:/opt/memebot/.mypy_cache" --entrypoint mypy bot memebot ``` diff --git a/docker-compose.yaml b/docker-compose.yaml index eb3e7bc..4189712 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -27,7 +27,7 @@ services: restart: always volumes: - ./data/db:/data/db - - ./src/config/mongod.yaml:/etc/mongo/mongod.yaml:ro + - ./memebot/config/mongod.yaml:/etc/mongo/mongod.yaml:ro networks: default: dns: From d61e6bf688aa430e21e14084a8ca28b0e31a94ac Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 05:40:23 -0500 Subject: [PATCH 04/26] Update Dockerfile to use new `memebot` package --- docker/Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker/Dockerfile b/docker/Dockerfile index 4e94f98..0d1bf14 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -21,8 +21,8 @@ RUN python3 -m pip install --no-cache-dir -r tests/requirements.txt FROM python:3.9.6-slim as run-base WORKDIR /opt/memebot/ ENV PATH="/opt/venv/bin:$PATH" -COPY src src -ENTRYPOINT ["python3", "src/main.py"] +COPY memebot memebot +ENTRYPOINT ["python3", "memebot/main.py"] # Run release build. FROM run-base as release From 7e0b103e56801f84f4f53f1f23d1ffd293e50396 Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 05:46:20 -0500 Subject: [PATCH 05/26] Add initial test fixtures --- tests/__init__.py | 0 tests/conftest.py | 4 ++++ tests/fixtures/__init__.py | 0 tests/fixtures/general_fixtures.py | 33 ++++++++++++++++++++++++++++++ 4 files changed, 37 insertions(+) create mode 100644 tests/__init__.py create mode 100644 tests/conftest.py create mode 100644 tests/fixtures/__init__.py create mode 100644 tests/fixtures/general_fixtures.py diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..9eea984 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,4 @@ +pytest_plugins = [ + "pytest_asyncio", + "tests.fixtures.general_fixtures", +] diff --git a/tests/fixtures/__init__.py b/tests/fixtures/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/fixtures/general_fixtures.py b/tests/fixtures/general_fixtures.py new file mode 100644 index 0000000..165ae0f --- /dev/null +++ b/tests/fixtures/general_fixtures.py @@ -0,0 +1,33 @@ +import os +from unittest import mock + +import pytest + +from memebot import config +from memebot.integrations import twitter + +DEFAULT_ENVIRONMENT = os.environ | { + "MEMEBOT_DISCORD_CLIENT_TOKEN": "MOCK_TOKEN", + "MEMEBOT_TWITTER_ENABLED": "False", +} + + +@pytest.fixture(autouse=True) +def setup_and_teardown(mock_twitter_client: mock.Mock) -> None: + """ + This is a universal setup and teardown function which is automatically run + surrounding each test. The function contains both the setup and teardown behavior. + The ``yield`` statement is where the test is run. + """ + # Set up + # This creates a simple base environment. Tests that require specific configuration + # can overwrite the variables they need + os.environ = DEFAULT_ENVIRONMENT + + with mock.patch("argparse.ArgumentParser", mock.MagicMock()): + config.populate_config_from_command_line() + + # Run test + yield + + # Tear down From fe0de15a319d6e96e4ba113c0b7f47a8cdcac553 Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 05:47:25 -0500 Subject: [PATCH 06/26] Add tests for `/hello` command --- tests/commands/__init__.py | 0 tests/commands/test_hello.py | 14 +++ tests/conftest.py | 1 + tests/fixtures/discord_fixtures.py | 137 +++++++++++++++++++++++++++++ 4 files changed, 152 insertions(+) create mode 100644 tests/commands/__init__.py create mode 100644 tests/commands/test_hello.py create mode 100644 tests/fixtures/discord_fixtures.py diff --git a/tests/commands/__init__.py b/tests/commands/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/commands/test_hello.py b/tests/commands/test_hello.py new file mode 100644 index 0000000..6014cfb --- /dev/null +++ b/tests/commands/test_hello.py @@ -0,0 +1,14 @@ +from unittest import mock + +import pytest + +from memebot import commands + + +@pytest.mark.asyncio() +async def test_hello(mock_interaction: mock.Mock) -> None: + await commands.hello.callback(mock_interaction) + + mock_interaction.response.send_message.assert_awaited_once_with( + f"Hello, {mock_interaction.user.display_name}!" + ) diff --git a/tests/conftest.py b/tests/conftest.py index 9eea984..690f32d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,5 @@ pytest_plugins = [ "pytest_asyncio", + "tests.fixtures.discord_fixtures", "tests.fixtures.general_fixtures", ] diff --git a/tests/fixtures/discord_fixtures.py b/tests/fixtures/discord_fixtures.py new file mode 100644 index 0000000..9959d5a --- /dev/null +++ b/tests/fixtures/discord_fixtures.py @@ -0,0 +1,137 @@ +from typing import Type, Optional +from unittest import mock + +import discord.utils +import pytest + + +@pytest.fixture +@mock.patch("discord.Client", spec=True) +def mock_discord_client(MockClient: type[mock.Mock]) -> mock.Mock: + return MockClient() + +@pytest.fixture +@mock.patch("discord.Interaction", spec=True) +def mock_interaction( + MockInteraction: Type[mock.Mock], mock_message: mock.Mock, mock_member: mock.Mock +) -> mock.Mock: + """ + Generates a mock interaction with basic behavior. + """ + mock_interaction = MockInteraction() + + mock_interaction.response.send_message = mock.AsyncMock() + mock_interaction.original_response = mock.AsyncMock(return_value=mock_message) + mock_interaction.user = mock_member + + return mock_interaction + + +@pytest.fixture +@mock.patch("discord.Message", spec=True) +def mock_message(MockMessage: Type[mock.Mock]) -> mock.Mock: + """ + Generates a mock message with basic behavior. + """ + mock_message = MockMessage() + + mock_message.add_reaction = mock.AsyncMock() + mock_message.channel.send = mock.AsyncMock() + + return mock_message + + +@pytest.fixture +@mock.patch("discord.Member", spec=True) +def mock_member(MockMember: Type[mock.Mock]) -> mock.Mock: + mock_member = MockMember() + mock_member.roles = [] + mock_member.add_roles = mock.AsyncMock() + return mock_member + + +@pytest.fixture +@mock.patch("discord.Guild", spec=True) +def mock_guild(MockGuild: Type[mock.Mock]) -> mock.Mock: + """ + Creates a default mock Guild + """ + mock_guild = MockGuild() + + mock_guild.create_role = mock.AsyncMock() + + return mock_guild + + +@pytest.fixture +def mock_guild_empty(mock_guild: mock.Mock) -> mock.Mock: + """ + Generates a guild with empty iterables + """ + mock_guild.roles = discord.utils.SequenceProxy([]) + + return mock_guild + + +def mock_role_with_attrs( + role_type: Type[mock.Mock], name: Optional[str] = None +) -> mock.Mock: + """ + Generates a unique mock role with specified attributes + """ + role = role_type(name=name) + role.delete = mock.AsyncMock() + role.members = discord.utils.SequenceProxy([]) + if name is not None: + role.name = name + return role + + +@pytest.fixture +@mock.patch("discord.Role", spec=True) +def mock_role_everyone(MockRoleEveryone: Type[mock.Mock], mock_interaction: mock.Mock) -> mock.Mock: + everyone = mock_role_with_attrs(MockRoleEveryone, "everyone") + everyone.members = discord.utils.SequenceProxy([mock_interaction.client.user]) + return everyone + + +@pytest.fixture +@mock.patch("discord.Role", spec=True) +def mock_role_bot(MockRoleBot: Type[mock.Mock], mock_interaction: mock.Mock) -> mock.Mock: + bot_role = mock_role_with_attrs(MockRoleBot, "bot") + bot_role.members = discord.utils.SequenceProxy([mock_interaction.client.user]) + return bot_role + + +@pytest.fixture +@mock.patch("discord.Role", spec=True) +def mock_role_foo(MockRoleFoo: Type[mock.Mock]) -> list[mock.Mock]: + return mock_role_with_attrs(MockRoleFoo, "foo") + + +@pytest.fixture +@mock.patch("discord.Role", spec=True) +def mock_role_bar(MockRoleBar: Type[mock.Mock]) -> list[mock.Mock]: + return mock_role_with_attrs(MockRoleBar, "bar") + + +@pytest.fixture +@mock.patch("discord.Role", spec=True) +def mock_role_baz(MockRoleBaz: Type[mock.Mock]) -> list[mock.Mock]: + return mock_role_with_attrs(MockRoleBaz, "baz") + + +@pytest.fixture +def mock_guild_populated( + mock_guild: mock.Mock, + mock_role_everyone: mock.Mock, + mock_role_bot: mock.Mock, + mock_role_foo: mock.Mock, + mock_role_bar: mock.Mock, + mock_role_baz: mock.Mock, +) -> mock.Mock: + mock_guild.roles = discord.utils.SequenceProxy( + # Roles ordered in reverse priority + [mock_role_everyone, mock_role_baz, mock_role_bar, mock_role_foo, mock_role_bot] + ) + return mock_guild From f936cc89c45ff48830f7fdf41ba9036e93237175 Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 05:47:42 -0500 Subject: [PATCH 07/26] Add tests for `/poll` command --- tests/commands/test_poll.py | 262 ++++++++++++++++++++++++++++++++++++ 1 file changed, 262 insertions(+) create mode 100644 tests/commands/test_poll.py diff --git a/tests/commands/test_poll.py b/tests/commands/test_poll.py new file mode 100644 index 0000000..d2baaad --- /dev/null +++ b/tests/commands/test_poll.py @@ -0,0 +1,262 @@ +import itertools +from typing import Optional +from unittest import mock + +import discord.embeds +import emoji +import pytest + +from memebot import commands +from memebot.lib import constants, exception + + +async def do_poll_votes_test( + mock_interaction: mock.Mock, + expected_answers: list[str], + yes_no: bool = False, +): + """ + Tests that the vote buttons of a poll work properly + """ + actual_view: discord.ui.View = ( + mock_interaction.response.send_message.call_args.kwargs.get("view") + ) + initial_embed: discord.Embed = ( + mock_interaction.response.send_message.call_args.kwargs.get("embed") + ) + + assert actual_view is not None, "/poll did not create a view" + + # Assert that the number of buttons created is correct + if yes_no: + assert len(actual_view.children) == 2, "Didn't create 2 buttons for yes/no" + else: + assert len(actual_view.children) == len( + expected_answers + ), "Created wrong number of buttons" + + if yes_no: + expected_emojis = ["👍", "👎"] + else: + expected_emojis = ["🇦", "🇧", "🇨", "🇩", "🇪"][: len(actual_view.children)] + + # Map button index -> number of expected votes + votes = {k: 0 for k in range(len(expected_emojis))} + + # Function to generate embed fields based on the current voting + def gen_fields(): + if yes_no: + return [ + { + "name": f"{expected_emojis[k]} ({votes[k]})", + "value": "\n".join(u.display_name for u in mock_users[: votes[k]]), + } + for k in votes + ] + else: + return [ + { + "name": f"{expected_emojis[k]} {expected_answers[k]} ({votes[k]})", + "value": "\n".join(u.display_name for u in mock_users[: votes[k]]), + } + for k in votes + ] + + for i, button in enumerate(actual_view.children): + assert isinstance( + button, discord.ui.Button + ), "/poll created non-button children" + + # Test that button attributes are correct + assert button.emoji.name == expected_emojis[i] + if yes_no: + assert button.label is None, "Added extraneous label for yes/no button" + else: + assert button.label == expected_answers[i], "Button is mislabled" + + # Try voting 5 times + n_votes = 5 + mock_users = [] + for j in range(n_votes): + with mock.patch("discord.Member", spec=True) as MockMember: + user = MockMember() + user.display_name = str(j) + mock_users.append(user) + + for vote_count in range(1, n_votes + 1): + with mock.patch("discord.Interaction", spec=True) as MockInteraction: + vote_interaction = MockInteraction() + vote_interaction.response.edit_message = mock.AsyncMock() + vote_interaction.user = mock_users[vote_count - 1] + # Click a vote button + await button.callback(vote_interaction) + votes[i] += 1 + + do_poll_embed_test( + vote_interaction, + initial_embed.description.strip("_"), + gen_fields(), + is_vote=True, + ) + + # Test un-voting + for unvote_count in reversed(range(1, n_votes + 1)): + with mock.patch("discord.Interaction", spec=True) as MockInteraction: + vote_interaction = MockInteraction() + vote_interaction.response.edit_message = mock.AsyncMock() + vote_interaction.user = mock_users[unvote_count - 1] + # Click a vote button + await button.callback(vote_interaction) + votes[i] -= 1 + + do_poll_embed_test( + vote_interaction, + initial_embed.description.strip("_"), + gen_fields(), + is_vote=True, + ) + + +def do_poll_embed_test( + mock_interaction: mock.Mock, + question: str, + expected_fields: list[dict[str, str]], + is_vote: bool = False, +) -> None: + """ + Tests that the embed for a poll is created correctly + """ + + if is_vote: + expected_call = mock_interaction.response.edit_message + else: + expected_call = mock_interaction.response.send_message + + expected_call.assert_awaited_once() + actual_embed: discord.Embed = expected_call.call_args.kwargs.get("embed") + + assert actual_embed is not None, "/poll did not send an embed" + + expected_title = ":bar_chart: **New Poll!**" + expected_description = f"_{question}_" + expected_color = constants.COLOR + + # Check embed attributes + assert actual_embed.title == expected_title + assert actual_embed.description == expected_description + assert actual_embed.color == expected_color + + # Ensure all the desired choices are present and correctly ordered + assert len(actual_embed.fields) == len(expected_fields) + for actual, expected in zip(actual_embed.fields, expected_fields): + proxy = discord.embeds.EmbedProxy(expected) + assert actual.name == proxy.name + assert actual.value == proxy.value + + +@pytest.mark.asyncio +async def test_poll_no_answers(mock_interaction: mock.Mock) -> None: + """ + Test creation of a poll message with no answers as input. The poll message should + contain the regular poll embed, with the only choices being automatically thumbs up + and thumbs down. + """ + question = "Mock question no args" + await commands.poll.callback(mock_interaction, question, *[None] * 5) + + # Check that only 2 fields (positive, negative) were added to the embed + expected_fields = [ + {"name": f"{emoji.emojize(':thumbs_up:')} (0)", "value": ""}, + {"name": f"{emoji.emojize(':thumbs_down:')} (0)", "value": ""}, + ] + + mock_interaction.response.send_message.assert_awaited_once() + do_poll_embed_test(mock_interaction, question, expected_fields) + await do_poll_votes_test(mock_interaction, [], yes_no=True) + + +@pytest.mark.parametrize( + ["a1", "a2", "a3", "a4", "a5"], + # Create a matrix of all possible answer inputs that contain only 1 real answer + [["Mock answer" if i == j else None for i in range(5)] for j in range(5)], +) +@pytest.mark.asyncio +async def test_poll_one_answer( + mock_interaction: mock.Mock, + a1: Optional[str], + a2: Optional[str], + a3: Optional[str], + a4: Optional[str], + a5: Optional[str], +) -> None: + """ + Test a poll with only one answer parameter provided. This is an illegal call and + should result in an immediate throw without creating the poll + """ + question = "Mock question 1 arg" + with pytest.raises(exception.MemebotUserError): + await commands.poll.callback(mock_interaction, question, a1, a2, a3, a4, a5) + + mock_interaction.response.send_message.assert_not_called() + mock_interaction.original_response.assert_not_called() + + +def valid_answer_combinations() -> list[list[Optional[str]]]: + """ + Generates a list of all possible valid answer configurations. The simple definition + for a valid answer combination is any copy of ``mock_answers`` such that, at most, + 3 of the values are replaced with ``None``. + """ + mock_answers = [ + "Mock answer 1", + "Mock answer 2", + "Mock answer 3", + "Mock answer 4", + "Mock answer 5", + ] + replace_max = 3 + return [ + [ + # 3. Generate an answer configuration by grabbing all the indexes from + # mock_answers that we're not going to replace with None + mock_answers[i] if i not in idxs_to_replace else None + for i in range(len(mock_answers)) + ] + # 1. Iterate through the number of replacements we can make + for n_replacements in range(replace_max + 1) + # 2. Iterate through all possible combinations of indices we can replace for + # the n_replacements we're looking at + for idxs_to_replace in itertools.combinations( + range(len(mock_answers)), n_replacements + ) + ] + + +@pytest.mark.parametrize(["a1", "a2", "a3", "a4", "a5"], valid_answer_combinations()) +@pytest.mark.asyncio +async def test_poll_n_answers( + mock_interaction: mock.Mock, + a1: Optional[str], + a2: Optional[str], + a3: Optional[str], + a4: Optional[str], + a5: Optional[str], +) -> None: + """ + Test a poll with any valid non-zero number of answers + """ + possible_answers = [a1, a2, a3, a4, a5] + question = f"Mock question with args: {possible_answers}" + + await commands.poll.callback(mock_interaction, question, *possible_answers) + + provided_answers = [a for a in possible_answers if a is not None] + letter_emojis = ["🇦", "🇧", "🇨", "🇩", "🇪"] + + expected_fields = [ + {"name": f"{letter_emojis[i]} {answer} (0)", "value": ""} + for i, answer in enumerate(provided_answers) + ] + + do_poll_embed_test(mock_interaction, question, expected_fields) + await do_poll_votes_test(mock_interaction, provided_answers) From 57d626cf0ab4ecac546b96c7c581631873f593fe Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 05:47:59 -0500 Subject: [PATCH 08/26] Add tests for `/role` command --- tests/commands/test_role.py | 626 ++++++++++++++++++++++++++++++++++++ 1 file changed, 626 insertions(+) create mode 100644 tests/commands/test_role.py diff --git a/tests/commands/test_role.py b/tests/commands/test_role.py new file mode 100644 index 0000000..8c5b323 --- /dev/null +++ b/tests/commands/test_role.py @@ -0,0 +1,626 @@ +from typing import Optional, Type, Any +from unittest import mock + +import discord.ext.commands +import pytest + +from memebot import commands +from memebot.lib import exception + +role_create: discord.ext.commands.Command +role_delete: discord.ext.commands.Command +role_join: discord.ext.commands.Command +role_leave: discord.ext.commands.Command +role_list: discord.ext.commands.Command + + +@pytest.fixture(autouse=True) +def resolve_role_functions() -> None: + global role_create + global role_delete + global role_join + global role_leave + global role_list + role_create = commands.role.get_command("create") + role_delete = commands.role.get_command("delete") + role_join = commands.role.get_command("join") + role_leave = commands.role.get_command("leave") + role_list = commands.role.get_command("list") + + +async def do_role_test( + mock_interaction: mock.Mock, + mock_guild: Optional[mock.Mock], + command: discord.ext.commands.Command, + *args: Any, + **kwargs: Any, +) -> None: + """ + Helper function for common role test pattern + """ + mock_interaction.guild = mock_guild + + await command.callback(mock_interaction, *args, **kwargs) + mock_interaction.response.send_message.assert_awaited_once() + + +async def do_role_test_failure( + mock_interaction: mock.Mock, + mock_guild: Optional[mock.Mock], + exc_type: Type[Exception], + command: discord.ext.commands.Command, + *args: Any, + **kwargs: Any, +) -> None: + """ + Helper function for common role test pattern + Only for test cases which throw + """ + with pytest.raises(exc_type): + await do_role_test(mock_interaction, mock_guild, command, *args, **kwargs) + mock_interaction.response.send_message.assert_not_awaited() + + +async def do_role_create_failure( + mock_interaction: mock.Mock, + mock_guild: Optional[mock.Mock], + role_name: str, + exc_type: Type[Exception], +) -> None: + """ + Helper function for common role_create failure test pattern + """ + await do_role_test_failure( + mock_interaction, mock_guild, exc_type, role_create, role_name + ) + + +async def do_role_create_success( + mock_interaction: mock.Mock, mock_guild: Optional[mock.Mock], role_name: str +) -> None: + """ + Helper function for common role_create test pattern + """ + await do_role_test(mock_interaction, mock_guild, role_create, role_name) + + mock_interaction.guild.create_role.assert_awaited_once_with( + name=role_name.lower(), mentionable=True, reason=mock.ANY + ) + + +@pytest.mark.parametrize("role_name", ["newrole", "nEWRole"]) +@pytest.mark.asyncio +async def test_role_create_empty( + mock_interaction: mock.Mock, mock_guild_empty: mock.Mock, role_name: str +) -> None: + """ + Test basic successful creation of a new role + """ + await do_role_create_success(mock_interaction, mock_guild_empty, role_name) + + +@pytest.mark.asyncio +async def test_role_create_populated( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Tests successful creation of a new role when there exist other roles + """ + await do_role_create_success(mock_interaction, mock_guild_populated, "anewrole") + + +@pytest.mark.parametrize("invalid_role_name", ["@newrole", "new@role"]) +@pytest.mark.asyncio +async def test_role_create_invalid_name( + mock_interaction: mock.Mock, mock_guild_empty: mock.Mock, invalid_role_name: str +) -> None: + """ + Test failure to create a role with an invalid name + """ + await do_role_create_failure( + mock_interaction, + mock_guild_empty, + invalid_role_name, + exception.MemebotUserError, + ) + mock_guild_empty.create_role.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_role_create_in_dm(mock_interaction: mock.Mock) -> None: + """ + Test creating a role in DMs fails gracefully + """ + await do_role_create_failure( + mock_interaction, None, "dmrole", exception.MemebotUserError + ) + + +@pytest.mark.parametrize("conflict_name", ["conflictrole", "ConFlictROLE"]) +@pytest.mark.asyncio +async def test_role_create_conflict( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock, conflict_name: str +) -> None: + """ + Test creating a role fails when there already exists a role with the same name + """ + mock_guild_populated.roles[1].name = conflict_name.lower() + await do_role_create_failure( + mock_interaction, + mock_guild_populated, + conflict_name, + exception.MemebotUserError, + ) + mock_guild_populated.create_role.assert_not_called() + + +@pytest.mark.asyncio +async def test_role_create_permission_error( + mock_interaction: mock.Mock, + mock_guild_populated: mock.Mock, +) -> None: + """ + Test attempting to create a role when Memebot doesn't have permission to do so + """ + with mock.patch("requests.Response") as MockResponse: + role_name = "forbiddenrole" + # Force create_role to throw a dummy exception + mock_guild_populated.create_role = mock.AsyncMock( + side_effect=discord.Forbidden(MockResponse(), None) + ) + await do_role_create_failure( + mock_interaction, + mock_guild_populated, + role_name, + exception.MemebotInternalError, + ) + + +@pytest.mark.asyncio +async def test_role_create_http_failure( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Test role creation HTTP failures are handled gracefully + """ + with mock.patch("requests.Response") as MockResponse: + role_name = "failurerole" + # Force create_role to throw a dummy exception + mock_guild_populated.create_role = mock.AsyncMock( + side_effect=discord.HTTPException(MockResponse(), None) + ) + await do_role_create_failure( + mock_interaction, + mock_guild_populated, + role_name, + exception.MemebotInternalError, + ) + + +@pytest.mark.asyncio +async def test_role_delete_happy( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Test deleting role from a guild which contains the role + and the role only has 0 members + """ + target_role = mock_guild_populated.roles[0] + target_role.members = [] + await do_role_test(mock_interaction, mock_guild_populated, role_delete, target_role) + + # Check that delete was only called on the desired role + for role in mock_guild_populated.roles: + if role != target_role: + role.delete.assert_not_awaited() + else: + role.delete.assert_awaited_once() + + +@pytest.mark.parametrize("n_members", [1, 10]) +@pytest.mark.asyncio +async def test_role_delete_too_many_members( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock, n_members: int +) -> None: + """ + Test attempting to delete a role with any members fails gracefully + """ + with mock.patch("discord.User") as MockUser: + mock_users = [MockUser() for _ in range(n_members)] + target_role = mock_guild_populated.roles[0] + target_role.members = mock_users + + await do_role_test_failure( + mock_interaction, + mock_guild_populated, + exception.MemebotUserError, + role_delete, + target_role, + ) + + target_role.delete.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_role_delete_forbidden( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Test attempting to delete a role when Memebot doesn't have permission to do so + """ + with mock.patch("requests.Response") as MockResponse: + target_role = mock_guild_populated.roles[2] + # Force role.delete to throw a dummy exception + target_role.delete = mock.AsyncMock( + side_effect=discord.Forbidden(MockResponse(), None) + ) + await do_role_test_failure( + mock_interaction, + mock_guild_populated, + exception.MemebotInternalError, + role_delete, + target_role, + ) + + +@pytest.mark.asyncio +async def test_role_delete_http_failure( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Test that HTTP failure during role deletion is handled gracefully + """ + with mock.patch("requests.Response") as MockResponse: + target_role = mock_guild_populated.roles[2] + # Force role.delete to throw a dummy exception + target_role.delete = mock.AsyncMock( + side_effect=discord.HTTPException(MockResponse(), None) + ) + await do_role_test_failure( + mock_interaction, + mock_guild_populated, + exception.MemebotInternalError, + role_delete, + target_role, + ) + + +@pytest.mark.asyncio +async def test_role_join_happy( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Tests successfully joining an existing role for a user with no roles + """ + target_role = mock_guild_populated.roles[0] + await do_role_test(mock_interaction, mock_guild_populated, role_join, target_role) + mock_interaction.user.add_roles.assert_awaited_once_with( + target_role, reason=mock.ANY + ) + + +@pytest.mark.asyncio +async def test_role_join_conflict( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Test failure to join a role, because the caller is already in the role + """ + target_role = mock_guild_populated.roles[0] + mock_interaction.user.roles = [target_role] + await do_role_test_failure( + mock_interaction, + mock_guild_populated, + exception.MemebotUserError, + role_join, + target_role, + ) + mock_interaction.user.add_roles.assert_not_awaited() + mock_interaction.response.send_message.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_role_join_forbidden( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Test that permission errors for role join are handled gracefully + """ + with mock.patch("requests.Response") as MockResponse: + target_role = mock_guild_populated.roles[2] + # Force role.delete to throw a dummy exception + mock_interaction.user.add_roles = mock.AsyncMock( + side_effect=discord.Forbidden(MockResponse(), None) + ) + await do_role_test_failure( + mock_interaction, + mock_guild_populated, + exception.MemebotInternalError, + role_join, + target_role, + ) + mock_interaction.user.add_roles.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_role_join_failure( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Test that permission errors for role join are handled gracefully + """ + with mock.patch("requests.Response") as MockResponse: + target_role = mock_guild_populated.roles[2] + # Force role.delete to throw a dummy exception + mock_interaction.user.add_roles = mock.AsyncMock( + side_effect=discord.HTTPException(MockResponse(), None) + ) + await do_role_test_failure( + mock_interaction, + mock_guild_populated, + exception.MemebotInternalError, + role_join, + target_role, + ) + mock_interaction.user.add_roles.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_role_leave_with_membership( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Test ability to leave a role the user is a member of + """ + target_role = mock_guild_populated.roles[1] + target_role.members = [mock_interaction.user] + + await do_role_test(mock_interaction, mock_guild_populated, role_leave, target_role) + + mock_interaction.user.remove_roles.assert_awaited_once_with( + target_role, reason=mock.ANY + ) + + +@pytest.mark.asyncio +async def test_role_leave_no_membership( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Test graceful failure for trying to leave a role the user is not a member of + """ + target_role = mock_guild_populated.roles[1] + target_role.members = [] + + await do_role_test_failure( + mock_interaction, + mock_guild_populated, + exception.MemebotUserError, + role_leave, + target_role, + ) + mock_interaction.user.remove_roles.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_role_leave_forbidden( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Test graceful failure of calling role leave when memebot has bad permissions + """ + with mock.patch("requests.Response") as MockResponse: + target_role = mock_guild_populated.roles[2] + target_role.members = [mock_interaction.user] + # Force role.delete to throw a dummy exception + mock_interaction.user.remove_roles = mock.AsyncMock( + side_effect=discord.Forbidden(MockResponse(), None) + ) + await do_role_test_failure( + mock_interaction, + mock_guild_populated, + exception.MemebotInternalError, + role_leave, + target_role, + ) + mock_interaction.user.remove_roles.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_role_leave_failure( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Test graceful failure of HTTP failure when calling role leave + """ + with mock.patch("requests.Response") as MockResponse: + target_role = mock_guild_populated.roles[2] + target_role.members = [mock_interaction.user] + # Force role.delete to throw a dummy exception + mock_interaction.user.remove_roles = mock.AsyncMock( + side_effect=discord.HTTPException(MockResponse(), None) + ) + await do_role_test_failure( + mock_interaction, + mock_guild_populated, + exception.MemebotInternalError, + role_leave, + target_role, + ) + mock_interaction.user.remove_roles.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_role_list_all_roles( + mock_interaction: mock.Mock, + mock_guild_populated: mock.Mock, + mock_role_bot: mock.Mock, + mock_role_everyone: mock.Mock, +) -> None: + """ + Test ability to list all roles + """ + await do_role_test(mock_interaction, mock_guild_populated, role_list, None) + expected_output = f"Roles managed through `/role` command:\n- bar\n- baz\n- foo" + mock_interaction.response.send_message.assert_awaited_once_with(expected_output) + + +@pytest.mark.asyncio +async def test_role_list_no_roles( + mock_interaction: mock.Mock, + mock_guild: mock.Mock, + mock_role_bot: mock.Mock, + mock_role_everyone: mock.Mock, +) -> None: + """ + Test that role list works when there are no manageable roles + """ + mock_guild.roles = discord.utils.SequenceProxy([mock_role_everyone, mock_role_bot]) + await do_role_test(mock_interaction, mock_guild, role_list, None) + expected_output = f"Roles managed through `/role` command:" + mock_interaction.response.send_message.assert_awaited_once_with(expected_output) + + +@pytest.mark.asyncio +async def test_role_list_user_own_roles( + mock_interaction: mock.Mock, + mock_guild_populated: mock.Mock, + mock_role_bar: mock.Mock, +) -> None: + """ + Test role list when retrieving roles of the user calling the command + """ + target = mock_interaction.user + mock_role_bar.members = discord.utils.SequenceProxy([target]) + await do_role_test(mock_interaction, mock_guild_populated, role_list, target) + expected_output = ( + f"Roles for user {target.name} managed through `/role` command:\n- bar" + ) + mock_interaction.response.send_message.assert_awaited_once_with(expected_output) + + +@pytest.mark.asyncio +async def test_role_list_other_user_roles( + mock_interaction: mock.Mock, + mock_guild_populated: mock.Mock, + mock_role_bar: mock.Mock, +) -> None: + """ + Test role list when retrieving roles of a user other than the one making the call + """ + with mock.patch("discord.Member", spec=True) as MockMember: + target = MockMember() + mock_role_bar.members = discord.utils.SequenceProxy([target]) + await do_role_test(mock_interaction, mock_guild_populated, role_list, target) + expected_output = ( + f"Roles for user {target.name} managed through `/role` command:\n- bar" + ) + mock_interaction.response.send_message.assert_awaited_once_with(expected_output) + + +@pytest.mark.asyncio +async def test_role_list_user_no_roles( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Test role list when retrieving roles of a user who is not in any manageable roles + """ + target = mock_interaction.user + await do_role_test(mock_interaction, mock_guild_populated, role_list, target) + expected_output = f"Roles for user {target.name} managed through `/role` command:" + mock_interaction.response.send_message.assert_awaited_once_with(expected_output) + + +@pytest.mark.asyncio +async def test_role_list_role( + mock_interaction: mock.Mock, + mock_guild_populated: mock.Mock, + mock_role_bar: mock.Mock, +) -> None: + """ + Test role list when retrieving members of a managed role + """ + user = mock_interaction.user + mock_role_bar.members = discord.utils.SequenceProxy([user]) + await do_role_test(mock_interaction, mock_guild_populated, role_list, mock_role_bar) + expected = f"Members of `@{mock_role_bar.name}`:\n- {user.nick} ({user.name})" + mock_interaction.response.send_message.assert_awaited_once_with(expected) + + +@pytest.mark.asyncio +async def test_role_list_role_no_members( + mock_interaction: mock.Mock, + mock_guild_populated: mock.Mock, + mock_role_bar: mock.Mock, +) -> None: + """ + Test role list when retrieving members of a role with no members + """ + with pytest.raises(exception.MemebotUserError): + await do_role_test( + mock_interaction, mock_guild_populated, role_list, mock_role_bar + ) + mock_interaction.response.send_message.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_role_list_role_not_managed( + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock +) -> None: + """ + Test role list when retrieving members of a non-managed role + """ + with mock.patch("discord.Role", spec=True) as MockRolePrivileged: + mock_role_privileged = MockRolePrivileged() + mock_role_privileged.members = discord.utils.SequenceProxy([]) + mock_guild_populated.roles = discord.utils.SequenceProxy( + [ + # Append a role to the end of the list, + # making it higher priority than the bot role + *mock_guild_populated.roles, + mock_role_privileged, + ] + ) + with pytest.raises(exception.MemebotUserError): + # TODO I believe this only throws because the role is empty. + # Memebot *does* check non-managed roles via role list. + # Should it? + await do_role_test( + mock_interaction, mock_guild_populated, role_list, mock_role_privileged + ) + mock_interaction.response.send_message.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_role_list_member( + mock_interaction: mock.Mock, + mock_guild_populated: mock.Mock, + mock_member: mock.Mock, + mock_role_bar: mock.Mock, +) -> None: + """ + Test listing roles of a particular user + """ + mock_role_bar.members = discord.utils.SequenceProxy([mock_member]) + await do_role_test(mock_interaction, mock_guild_populated, role_list, mock_member) + mock_interaction.response.send_message.assert_awaited_once_with( + f"Roles for user {mock_member.name} managed through `/role` command:" + f"\n- {mock_role_bar.name}" + ) + + +@pytest.mark.asyncio +async def test_role_list_member_no_roles( + mock_interaction: mock.Mock, + mock_guild_populated: mock.Mock, + mock_member: mock.Mock, +) -> None: + """ + Test listing roles of a particular user which is not a member of any role + """ + await do_role_test(mock_interaction, mock_guild_populated, role_list, mock_member) + # TODO This does not respond with an error. Should it? + # Should the message be different if not? + mock_interaction.response.send_message.assert_awaited_once_with( + f"Roles for user {mock_member.name} managed through `/role` command:" + ) From 57a89b88c5a5140208a29f8ab0a2e38e913118aa Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 05:48:17 -0500 Subject: [PATCH 09/26] Add tests for Twitter integration --- tests/conftest.py | 1 + tests/fixtures/general_fixtures.py | 4 +- tests/fixtures/twitter_fixtures.py | 140 ++++++++++++++++++ tests/integrations/__init__.py | 0 tests/integrations/test_twitter.py | 220 +++++++++++++++++++++++++++++ 5 files changed, 364 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/twitter_fixtures.py create mode 100644 tests/integrations/__init__.py create mode 100644 tests/integrations/test_twitter.py diff --git a/tests/conftest.py b/tests/conftest.py index 690f32d..e71afce 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,4 +2,5 @@ "pytest_asyncio", "tests.fixtures.discord_fixtures", "tests.fixtures.general_fixtures", + "tests.fixtures.twitter_fixtures", ] diff --git a/tests/fixtures/general_fixtures.py b/tests/fixtures/general_fixtures.py index 165ae0f..e2c2cd2 100644 --- a/tests/fixtures/general_fixtures.py +++ b/tests/fixtures/general_fixtures.py @@ -25,7 +25,9 @@ def setup_and_teardown(mock_twitter_client: mock.Mock) -> None: os.environ = DEFAULT_ENVIRONMENT with mock.patch("argparse.ArgumentParser", mock.MagicMock()): - config.populate_config_from_command_line() + with mock.patch("tweepy.Client", lambda *_, **__: mock_twitter_client): + config.populate_config_from_command_line() + twitter.init(mock.MagicMock()) # Run test yield diff --git a/tests/fixtures/twitter_fixtures.py b/tests/fixtures/twitter_fixtures.py new file mode 100644 index 0000000..107ad47 --- /dev/null +++ b/tests/fixtures/twitter_fixtures.py @@ -0,0 +1,140 @@ +from typing import Any +from unittest import mock + +import pytest + + +@pytest.fixture +def mock_twitter_url() -> str: + return "https://twitter.com/dril/status/922321981" + + +@pytest.fixture +@mock.patch("tweepy.ReferencedTweet") +def mock_referenced_tweet(MockReferencedTweet: type[mock.Mock]) -> mock.Mock: + return MockReferencedTweet() + + +@pytest.fixture +@mock.patch("tweepy.ReferencedTweet") +def mock_referenced_tweet_2(MockReferencedTweet: type[mock.Mock]) -> mock.Mock: + return MockReferencedTweet() + + +@pytest.fixture +@mock.patch("tweepy.ReferencedTweet") +def mock_referenced_tweet_3(MockReferencedTweet: type[mock.Mock]) -> mock.Mock: + return MockReferencedTweet() + + +@pytest.fixture +@mock.patch("tweepy.ReferencedTweet") +def mock_referenced_tweet_4(MockReferencedTweet: type[mock.Mock]) -> mock.Mock: + return MockReferencedTweet() + + +@pytest.fixture +@mock.patch("tweepy.Tweet", spec=True) +def mock_tweet(MockTweet: type[mock.Mock], mock_twitter_user: mock.Mock) -> mock.Mock: + tweet = MockTweet() + tweet.referenced_tweets = [] + tweet.author_id = mock_twitter_user.id + return tweet + + +@pytest.fixture +@mock.patch("tweepy.Tweet", spec=True) +def mock_quote_tweet( + MockTweet: type[mock.Mock], + mock_referenced_tweet: mock.Mock, + mock_twitter_user: mock.Mock, +) -> mock.Mock: + tweet = MockTweet() + mock_referenced_tweet.type = "quoted" + tweet.referenced_tweets = [mock_referenced_tweet] + tweet.author_id = mock_twitter_user.id + return tweet + + +@pytest.fixture +@mock.patch("tweepy.Tweet", spec=True) +def mock_quote_tweet_2( + MockTweet: mock.Mock, + mock_referenced_tweet_2: mock.Mock, + mock_twitter_user: mock.Mock, +) -> mock.Mock: + tweet = MockTweet() + mock_referenced_tweet_2.type = "quoted" + tweet.referenced_tweets = [mock_referenced_tweet_2] + tweet.author_id = mock_twitter_user.id + return tweet + + +@pytest.fixture +@mock.patch("tweepy.Tweet", spec=True) +def mock_quote_tweet_3( + MockTweet: mock.Mock, + mock_referenced_tweet_3: mock.Mock, + mock_twitter_user: mock.Mock, +) -> mock.Mock: + tweet = MockTweet() + mock_referenced_tweet_3.type = "quoted" + tweet.referenced_tweets = [mock_referenced_tweet_3] + tweet.author_id = mock_twitter_user.id + return tweet + + +@pytest.fixture +@mock.patch("tweepy.Tweet", spec=True) +def mock_quote_tweet_4( + MockTweet: mock.Mock, + mock_referenced_tweet_4: mock.Mock, + mock_twitter_user: mock.Mock, +) -> mock.Mock: + tweet = MockTweet() + mock_referenced_tweet_4.type = "quoted" + tweet.referenced_tweets = [mock_referenced_tweet_4] + tweet.author_id = mock_twitter_user.id + return tweet + + +@pytest.fixture +@mock.patch("tweepy.User", new_callable=mock.MagicMock) +def mock_twitter_user(MockTwitterUser: type[mock.Mock]) -> mock.Mock: + return MockTwitterUser() + + +@pytest.fixture +@mock.patch("tweepy.Response", autospec=True) +@mock.patch("tweepy.Client", autospec=True) +def mock_twitter_client( + MockTwitterClient: type[mock.Mock], + MockTwitterResponse: type[mock.Mock], + mock_twitter_user: mock.Mock, + mock_tweet: mock.Mock, + mock_quote_tweet: mock.Mock, + mock_quote_tweet_2: mock.Mock, + mock_quote_tweet_3: mock.Mock, + mock_quote_tweet_4: mock.Mock, +) -> mock.Mock: + def _get_tweet(tweet_id: str, *_: Any, **__: Any) -> mock.Mock: + resp = MockTwitterResponse() + resp.data = next( + ( + t + for t in ( + mock_quote_tweet, + mock_quote_tweet_2, + mock_quote_tweet_3, + mock_quote_tweet_4, + ) + if t.id == tweet_id + ), + mock_tweet, + ) + resp.includes = {"users": [mock_twitter_user]} + return resp + + client = MockTwitterClient() + client.get_tweet = _get_tweet + return client diff --git a/tests/integrations/__init__.py b/tests/integrations/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/integrations/test_twitter.py b/tests/integrations/test_twitter.py new file mode 100644 index 0000000..9bd03a3 --- /dev/null +++ b/tests/integrations/test_twitter.py @@ -0,0 +1,220 @@ +from unittest import mock + +import emoji +import pytest + +from memebot.integrations import twitter +from memebot.lib import exception + + +def test_twitter_url_regex(mock_twitter_url: str) -> None: + """ + Tests that the Twitter regex only works on valid tweet URLs + """ + assert twitter.twitter_url_pattern.search(mock_twitter_url) is not None + assert twitter.twitter_url_pattern.search(f"{mock_twitter_url}?s=19") is not None + assert ( + twitter.twitter_url_pattern.search(f"{mock_twitter_url}?s=19&t=12345") + is not None + ) + assert twitter.twitter_url_pattern.search("https://memebot.com") is None + assert twitter.twitter_url_pattern.search("https://twitter.com/foo/bar") is None + assert ( + twitter.twitter_url_pattern.search("https://twitter.com/i/status/12345") + is not None + ) + assert ( + twitter.twitter_url_pattern.search("https://twitter.com/i/web/status/12345") + is not None + ) + assert twitter.twitter_url_pattern.search("https://twitter.com") is None + assert twitter.twitter_url_pattern.search("twitter.com") is None + assert twitter.twitter_url_pattern.search("twitter.com/i/status/12345") is None + + +def test_get_twitter_url_from_message_content(mock_twitter_url: str) -> None: + """ + Test that we're able to successfully pull a Tweet URL from a discord message + """ + assert twitter.get_twitter_url_from_message_content( + f"foo bar baz {mock_twitter_url} 1 2 3" + ) == (mock_twitter_url, False) + assert twitter.get_twitter_url_from_message_content( + f"foo bar baz {mock_twitter_url}" + ) == (mock_twitter_url, False) + assert twitter.get_twitter_url_from_message_content( + f"{mock_twitter_url} 1 2 3" + ) == (mock_twitter_url, False) + + # Test spoiler + assert twitter.get_twitter_url_from_message_content( + f"foo bar baz ||{mock_twitter_url}|| 1 2 3" + ) == (mock_twitter_url, True) + assert twitter.get_twitter_url_from_message_content( + f"foo bar baz ||{mock_twitter_url}||" + ) == (mock_twitter_url, True) + assert twitter.get_twitter_url_from_message_content( + f"||{mock_twitter_url}|| 1 2 3" + ) == (mock_twitter_url, True) + assert twitter.get_twitter_url_from_message_content( + f"||foo bar baz {mock_twitter_url}|| 1 2 3" + ) == (mock_twitter_url, True) + + +def test_is_quote_tweet(mock_quote_tweet: mock.Mock, mock_tweet: mock.Mock) -> None: + """ + Test if we properly evaluate quote tweets + """ + assert twitter.is_quote_tweet(mock_quote_tweet) + assert not twitter.is_quote_tweet(mock_tweet) + + +def test_get_quote_tweet_urls( + mock_quote_tweet: mock.Mock, mock_tweet: mock.Mock, mock_twitter_user: mock.Mock +) -> None: + """ + Test if we properly construct the string which displays nested quote tweet URLs + """ + actual = twitter.get_quote_tweet_urls(mock_quote_tweet, False) + expected = ( + f"quoted tweet(s): \n" + f"https://twitter.com/{mock_twitter_user.username}" + f"/status/{mock_tweet.id}" + ) + assert actual == expected + + +def test_nested_quote_tweet( + mock_quote_tweet: mock.Mock, + mock_quote_tweet_2: mock.Mock, + mock_quote_tweet_3: mock.Mock, + mock_quote_tweet_4: mock.Mock, + mock_twitter_user: mock.Mock, +) -> None: + """ + Test that multiple nested quote tweets come out properly + """ + mock_quote_tweet.referenced_tweets[0].id = mock_quote_tweet_2.id + mock_quote_tweet_2.referenced_tweets[0].id = mock_quote_tweet_3.id + mock_quote_tweet_3.referenced_tweets[0].id = mock_quote_tweet_4.id + actual = twitter.get_quote_tweet_urls(mock_quote_tweet, False) + expected = ( + "quoted tweet(s): \n" + f"https://twitter.com/{mock_twitter_user.username}" + f"/status/{mock_quote_tweet_2.id}\n" + f"https://twitter.com/{mock_twitter_user.username}" + f"/status/{mock_quote_tweet_3.id}\n" + f"https://twitter.com/{mock_twitter_user.username}" + f"/status/{mock_quote_tweet_4.id}" + ) + assert actual == expected + + +def test_quote_tweet_non_quote_failure(mock_tweet: mock.Mock) -> None: + """ + Test that attempting to unroll a non-quote tweet fails + """ + with pytest.raises(exception.MemebotInternalError): + twitter.get_quote_tweet_urls(mock_tweet, False) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("num_media", [2, 3, 4]) +async def test_tweet_media_reaction( + mock_twitter_url: str, + mock_message: mock.Mock, + num_media: int, +) -> None: + """ + Tests that tweets with more than 1 media item get reacted to + """ + old_get_tweet = twitter.twitter_api.get_tweet + + def patched_get_tweet(*args, **kwargs): + response = old_get_tweet(*args, **kwargs) + response.includes |= {"media": [mock.MagicMock()] * num_media} + return response + + with mock.patch.object(twitter.twitter_api, "get_tweet", patched_get_tweet): + mock_message.content = mock_twitter_url + await twitter.process_message_for_interaction(mock_message) + mock_message.add_reaction.assert_awaited_once_with( + emoji.emojize(f":keycap_{num_media}:") + ) + + +@pytest.mark.asyncio +async def test_tweet_media_no_reaction( + mock_twitter_url: str, mock_message: mock.Mock +) -> None: + """ + Tests that tweets with a single piece of media do not get reacted to + """ + old_get_tweet = twitter.twitter_api.get_tweet + + def patched_get_tweet(*args, **kwargs): + response = old_get_tweet(*args, **kwargs) + response.includes |= {"media": [mock.MagicMock()]} + return response + + with mock.patch.object(twitter.twitter_api, "get_tweet", patched_get_tweet): + mock_message.content = mock_twitter_url + await twitter.process_message_for_interaction(mock_message) + mock_message.add_reaction.assert_not_awaited() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("media_type", ["video", "animated_gif"]) +@pytest.mark.parametrize("content_type", ["video/mp4", "video/mpeg"]) +async def test_tweet_embedded_video( + mock_twitter_url: str, mock_message: mock.Mock, media_type: str, content_type: str +) -> None: + """ + Tests that tweets with a video have the video embedded + """ + old_get_tweet = twitter.twitter_api.get_tweet + mock_variant_1 = mock.MagicMock() + mock_variant_2 = mock.MagicMock() + mock_variant_3 = mock.MagicMock() + + with mock.patch("tweepy.media.Media", spec=True) as MockMedia: + + def patched_get_tweet(*args, **kwargs): + response = old_get_tweet(*args, **kwargs) + mock_media = MockMedia() + mock_media.type = media_type + mock_media.variants = [ + {"bitrate": n, "url": variant, "content_type": content_type} + for n, variant in enumerate( + (mock_variant_1, mock_variant_2, mock_variant_3) + ) + ] + response.includes |= {"media": [mock_media]} + return response + + with mock.patch.object(twitter.twitter_api, "get_tweet", patched_get_tweet): + mock_message.content = mock_twitter_url + await twitter.process_message_for_interaction(mock_message) + mock_message.channel.send.assert_awaited_once_with( + f"embedded video:\n{mock_variant_3}" + ) + + +@pytest.mark.asyncio +async def test_single_media_no_embed( + mock_message: mock.Mock, mock_twitter_url: str +) -> None: + """ + Tests that a tweet with a single non-video media does not attempt to embed + """ + old_get_tweet = twitter.twitter_api.get_tweet + + def patched_get_tweet(*args, **kwargs): + response = old_get_tweet(*args, **kwargs) + response.includes |= {"media": [mock.MagicMock()]} + return response + + with mock.patch.object(twitter.twitter_api, "get_tweet", patched_get_tweet): + mock_message.content = mock_twitter_url + await twitter.process_message_for_interaction(mock_message) + mock_message.channel.send.assert_not_awaited() From 9e29794134788b0f9a744cc944c2a1b1bc45958e Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 05:49:11 -0500 Subject: [PATCH 10/26] Update mypy config to ignore `discord.ext.commands` --- pyproject.toml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index cf2c752..a2718af 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,7 +15,11 @@ exclude = [ "venv" ] -# tweepy does not provide type annotations, but it is desired for 4.x [[tool.mypy.overrides]] -module = "tweepy" +module = [ + # tweepy does not provide type annotations, but it is desired for 4.x + "tweepy", + # This module in discord.py does not have full stubs for some reason + "discord.ext.commands", +] ignore_missing_imports = true From 29789c6208b512cae5dba7a6406c5bc1c05f06f2 Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 05:50:12 -0500 Subject: [PATCH 11/26] Update pytest workflow * Remove extraneous `pytest` install * Remove unused secrets * Use more detailed test runner output --- .github/workflows/pytest.yaml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/pytest.yaml b/.github/workflows/pytest.yaml index fe3215a..09ad91e 100644 --- a/.github/workflows/pytest.yaml +++ b/.github/workflows/pytest.yaml @@ -27,12 +27,8 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - pip install pytest pip install -r requirements.txt -r tests/requirements.txt - name: Test with pytest env: - MEMEBOT_DISCORD_CLIENT_TOKEN: ${{ secrets.MEMEBOT_DISCORD_CLIENT_TOKEN }} - MEMEBOT_TWITTER_CONSUMER_KEY: ${{ secrets.MEMEBOT_TWITTER_CONSUMER_KEY }} - MEMEBOT_TWITTER_CONSUMER_SECRET: ${{ secrets.MEMEBOT_TWITTER_CONSUMER_SECRET }} run: | - pytest + pytest --tb=long -v From 3933fdf38182c43849ad83c01b00a81749e365fe Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 06:03:19 -0500 Subject: [PATCH 12/26] Update README with instructions on running tests --- README.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/README.md b/README.md index 7083c61..60d47ec 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,28 @@ Leaving variables empty just means that default values will be used. ## Tests +### pytest + +Memebot has a suite of unit tests based on [`pytest`](https://pytest.org). The test code +is located in the [tests](./tests) directory. Running the tests is straightforward: + +```shell +$ python3 -m pytest [/path/to/test/package/or/module] +``` + +Running the above from the root of the repository with no path(s) specified will run +all the tests. + +The tests can also be run from the _test_ Docker image: + +```shell +$ docker run --rm -it --entrypoint python3 memebot:test -m pytest [/path/to/test/package/or/module] + +# OR + +$ docker-compose run --rm --entrypoint python3 bot -m pytest [/path/to/test/package/or/module] +``` + ### mypy Memebot uses static type checking from [mypy](http://mypy-lang.org) to improve code correctness. The config From 5df38fbaca0b4b96757c9a269911ae7faf9d59bd Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 06:09:50 -0500 Subject: [PATCH 13/26] Update pytest action to remove unnecessary permission --- .github/workflows/pytest.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/pytest.yaml b/.github/workflows/pytest.yaml index 09ad91e..c7973bd 100644 --- a/.github/workflows/pytest.yaml +++ b/.github/workflows/pytest.yaml @@ -10,9 +10,6 @@ on: pull_request: branches: ["master"] -permissions: - contents: read - jobs: build: From 154044295e0dddc0a70687e4b92ce2ebef3d7b09 Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 06:09:59 -0500 Subject: [PATCH 14/26] Fix lint --- tests/fixtures/discord_fixtures.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/fixtures/discord_fixtures.py b/tests/fixtures/discord_fixtures.py index 9959d5a..3a7f264 100644 --- a/tests/fixtures/discord_fixtures.py +++ b/tests/fixtures/discord_fixtures.py @@ -10,6 +10,7 @@ def mock_discord_client(MockClient: type[mock.Mock]) -> mock.Mock: return MockClient() + @pytest.fixture @mock.patch("discord.Interaction", spec=True) def mock_interaction( @@ -89,7 +90,9 @@ def mock_role_with_attrs( @pytest.fixture @mock.patch("discord.Role", spec=True) -def mock_role_everyone(MockRoleEveryone: Type[mock.Mock], mock_interaction: mock.Mock) -> mock.Mock: +def mock_role_everyone( + MockRoleEveryone: Type[mock.Mock], mock_interaction: mock.Mock +) -> mock.Mock: everyone = mock_role_with_attrs(MockRoleEveryone, "everyone") everyone.members = discord.utils.SequenceProxy([mock_interaction.client.user]) return everyone @@ -97,7 +100,9 @@ def mock_role_everyone(MockRoleEveryone: Type[mock.Mock], mock_interaction: mock @pytest.fixture @mock.patch("discord.Role", spec=True) -def mock_role_bot(MockRoleBot: Type[mock.Mock], mock_interaction: mock.Mock) -> mock.Mock: +def mock_role_bot( + MockRoleBot: Type[mock.Mock], mock_interaction: mock.Mock +) -> mock.Mock: bot_role = mock_role_with_attrs(MockRoleBot, "bot") bot_role.members = discord.utils.SequenceProxy([mock_interaction.client.user]) return bot_role From 328b108a94b7a6a556e114056d90046de1d56ac6 Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 06:14:00 -0500 Subject: [PATCH 15/26] Exclude tests from mypy With mocking, type checking becomes very difficult and annoying --- pyproject.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index a2718af..490a83b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,7 +12,8 @@ warn_unreachable = true pretty = true exclude = [ "data", - "venv" + "venv", + "tests", ] [[tool.mypy.overrides]] From c399be5fd74ea6f1a71658cd0b40f99fd5a6fe23 Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 06:16:08 -0500 Subject: [PATCH 16/26] Rename pytest job --- .github/workflows/pytest.yaml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/pytest.yaml b/.github/workflows/pytest.yaml index c7973bd..cefcc34 100644 --- a/.github/workflows/pytest.yaml +++ b/.github/workflows/pytest.yaml @@ -11,10 +11,8 @@ on: branches: ["master"] jobs: - build: - + test: runs-on: ubuntu-latest - steps: - uses: actions/checkout@v3 - name: Set up Python 3.9 From 0bea15f82c594b93ff011c219988427d5e8a4f21 Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 7 Jun 2023 06:21:39 -0500 Subject: [PATCH 17/26] Fix broken pytest job --- .github/workflows/pytest.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/pytest.yaml b/.github/workflows/pytest.yaml index cefcc34..d938989 100644 --- a/.github/workflows/pytest.yaml +++ b/.github/workflows/pytest.yaml @@ -24,6 +24,5 @@ jobs: python -m pip install --upgrade pip pip install -r requirements.txt -r tests/requirements.txt - name: Test with pytest - env: run: | pytest --tb=long -v From 4a7be89400a6765eb2ee396aac9b4cbc5b90e00f Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Sun, 5 May 2024 20:21:08 -0400 Subject: [PATCH 18/26] Lazily instantiate Discord client Instead of creating the client on load of `client.py` module, the client is instead instantiated with the first call to a new function `get_memebot()`. This eliminates side effects related to module load order. --- memebot/client.py | 32 +++++++++++++++++++------------- memebot/main.py | 7 +++---- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/memebot/client.py b/memebot/client.py index 6379685..f5c7211 100644 --- a/memebot/client.py +++ b/memebot/client.py @@ -1,3 +1,4 @@ +import functools import logging import discord @@ -15,6 +16,7 @@ async def on_ready() -> None: """ Determines what the bot does as soon as it is logged into discord """ + memebot = get_memebot() if not memebot.user: raise exception.MemebotInternalError("Memebot is not logged in to Discord") log.info(f"Logged in as {memebot.user}") @@ -84,18 +86,22 @@ async def on_command_error( ) -memebot = discord.ext.commands.Bot( - command_prefix="/", - intents=discord.Intents().all(), - activity=discord.Game(name="• /hello"), -) +@functools.cache +def get_memebot() -> discord.ext.commands.Bot: + new_memebot = discord.ext.commands.Bot( + command_prefix="/", + intents=discord.Intents().all(), + activity=discord.Game(name="• /hello"), + ) + + new_memebot.tree.add_command(commands.hello) + new_memebot.tree.add_command(commands.poll) + new_memebot.tree.add_command(commands.role) -memebot.tree.add_command(commands.hello) -memebot.tree.add_command(commands.poll) -memebot.tree.add_command(commands.role) + new_memebot.add_listener(on_ready) + new_memebot.add_listener(on_interaction) + new_memebot.tree.error(on_command_error) + if config.twitter_enabled: + new_memebot.add_listener(twitter.process_message_for_interaction, "on_message") -memebot.add_listener(on_ready) -memebot.add_listener(on_interaction) -memebot.tree.error(on_command_error) -if config.twitter_enabled: - memebot.add_listener(twitter.process_message_for_interaction, "on_message") + return new_memebot diff --git a/memebot/main.py b/memebot/main.py index 813ed9e..ce918fa 100644 --- a/memebot/main.py +++ b/memebot/main.py @@ -1,9 +1,6 @@ from memebot import config - -config.populate_config_from_command_line() - -from memebot.client import memebot from memebot import log +from memebot.client import get_memebot def main() -> None: @@ -12,6 +9,8 @@ def main() -> None: :return: Exit status of discord.Client.run() """ log.set_third_party_logging() + config.populate_config_from_command_line() + memebot = get_memebot() # !! DO NOT HARDCODE THE TOKEN !! memebot.run(config.discord_api_token) From 821e07ed201b579ea27b6cca5873d0f92d1dd9ce Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Sun, 5 May 2024 20:21:32 -0400 Subject: [PATCH 19/26] Run all pytests by default --- docker/Dockerfile | 1 + pyproject.toml | 3 +++ 2 files changed, 4 insertions(+) diff --git a/docker/Dockerfile b/docker/Dockerfile index 0d1bf14..e52affb 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -30,4 +30,5 @@ COPY --from=build /opt/venv /opt/venv # Run test build. FROM run-base as test +COPY tests tests COPY --from=build-test /opt/venv /opt/venv \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml index 490a83b..edcaf7c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,3 +1,6 @@ +[tool.pytest.ini_options] +testpaths = ["tests"] + [tool.mypy] python_version = "3.9" warn_return_any = true From 48aa9252436df552656562b9f663dc264024673c Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Sun, 5 May 2024 20:21:52 -0400 Subject: [PATCH 20/26] Use constant display name in test for `hello.py` --- tests/commands/test_hello.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/commands/test_hello.py b/tests/commands/test_hello.py index 6014cfb..940f5ad 100644 --- a/tests/commands/test_hello.py +++ b/tests/commands/test_hello.py @@ -7,8 +7,9 @@ @pytest.mark.asyncio() async def test_hello(mock_interaction: mock.Mock) -> None: + mock_interaction.user.display_name = "Test Name" await commands.hello.callback(mock_interaction) mock_interaction.response.send_message.assert_awaited_once_with( - f"Hello, {mock_interaction.user.display_name}!" + f"Hello, Test Name!" ) From fbc39933c5a6f8572dcee9b6fc377c2b9d205b52 Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Sun, 5 May 2024 20:22:12 -0400 Subject: [PATCH 21/26] Use static answer matrix in single-answer test for `poll.py` --- tests/commands/test_poll.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/commands/test_poll.py b/tests/commands/test_poll.py index d2baaad..5810154 100644 --- a/tests/commands/test_poll.py +++ b/tests/commands/test_poll.py @@ -178,7 +178,13 @@ async def test_poll_no_answers(mock_interaction: mock.Mock) -> None: @pytest.mark.parametrize( ["a1", "a2", "a3", "a4", "a5"], # Create a matrix of all possible answer inputs that contain only 1 real answer - [["Mock answer" if i == j else None for i in range(5)] for j in range(5)], + [ + ["Mock answer", None, None, None, None], + [None, "Mock answer", None, None, None], + [None, None, "Mock answer", None, None], + [None, None, None, "Mock answer", None], + [None, None, None, None, "Mock answer"], + ], ) @pytest.mark.asyncio async def test_poll_one_answer( From 19179ad7e2ef21c298fbe67cc93e185d45a87e85 Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Sun, 5 May 2024 20:22:26 -0400 Subject: [PATCH 22/26] Remove redundant check for typing module --- memebot/lib/util.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/memebot/lib/util.py b/memebot/lib/util.py index 9d66433..5ab6e91 100644 --- a/memebot/lib/util.py +++ b/memebot/lib/util.py @@ -1,11 +1,8 @@ -from typing import Tuple, List, Optional, cast, TYPE_CHECKING +from typing import Tuple, List, Optional, cast import discord import discord.ext.commands -if TYPE_CHECKING: - pass - def is_spoil(message: str, idx: int) -> bool: """Returns whether idx of message is within a spoiler text""" From 7cd0adfa7cc6cbe9677f94bd2f79e3aa1ceeb486 Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Sun, 5 May 2024 20:22:48 -0400 Subject: [PATCH 23/26] Remove stale script entry from `.dockerignore` --- .dockerignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.dockerignore b/.dockerignore index 2b13f65..3942a0e 100644 --- a/.dockerignore +++ b/.dockerignore @@ -5,5 +5,4 @@ .mypy_cache/ data/ docker/ -install.bash venv/ \ No newline at end of file From 4bfd634b1e1ab3e961c9e43497e946dbbe5c771f Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Sun, 5 May 2024 20:25:20 -0400 Subject: [PATCH 24/26] Fix lint --- memebot/config/validators.py | 1 + memebot/integrations/twitter.py | 1 + 2 files changed, 2 insertions(+) diff --git a/memebot/config/validators.py b/memebot/config/validators.py index c5dd0a6..94081f0 100644 --- a/memebot/config/validators.py +++ b/memebot/config/validators.py @@ -2,6 +2,7 @@ Functions that will validate string input from the command line and convert them into appropriate data for use by the config module. """ + import collections import logging import logging.handlers diff --git a/memebot/integrations/twitter.py b/memebot/integrations/twitter.py index 0c6a8cc..57d3a88 100644 --- a/memebot/integrations/twitter.py +++ b/memebot/integrations/twitter.py @@ -6,6 +6,7 @@ All functions that make any network communication, except for init, should be asynchronous and stateless. """ + import re from typing import Tuple From 74ff612bb6b544efa950e7c3772b9eecccd6ad7d Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Tue, 12 Nov 2024 17:25:27 -0500 Subject: [PATCH 25/26] Remove helpers from `/role` tests --- tests/commands/test_role.py | 315 +++++++++++++----------------------- 1 file changed, 114 insertions(+), 201 deletions(-) diff --git a/tests/commands/test_role.py b/tests/commands/test_role.py index 8c5b323..4abc3bb 100644 --- a/tests/commands/test_role.py +++ b/tests/commands/test_role.py @@ -1,4 +1,3 @@ -from typing import Optional, Type, Any from unittest import mock import discord.ext.commands @@ -28,66 +27,6 @@ def resolve_role_functions() -> None: role_list = commands.role.get_command("list") -async def do_role_test( - mock_interaction: mock.Mock, - mock_guild: Optional[mock.Mock], - command: discord.ext.commands.Command, - *args: Any, - **kwargs: Any, -) -> None: - """ - Helper function for common role test pattern - """ - mock_interaction.guild = mock_guild - - await command.callback(mock_interaction, *args, **kwargs) - mock_interaction.response.send_message.assert_awaited_once() - - -async def do_role_test_failure( - mock_interaction: mock.Mock, - mock_guild: Optional[mock.Mock], - exc_type: Type[Exception], - command: discord.ext.commands.Command, - *args: Any, - **kwargs: Any, -) -> None: - """ - Helper function for common role test pattern - Only for test cases which throw - """ - with pytest.raises(exc_type): - await do_role_test(mock_interaction, mock_guild, command, *args, **kwargs) - mock_interaction.response.send_message.assert_not_awaited() - - -async def do_role_create_failure( - mock_interaction: mock.Mock, - mock_guild: Optional[mock.Mock], - role_name: str, - exc_type: Type[Exception], -) -> None: - """ - Helper function for common role_create failure test pattern - """ - await do_role_test_failure( - mock_interaction, mock_guild, exc_type, role_create, role_name - ) - - -async def do_role_create_success( - mock_interaction: mock.Mock, mock_guild: Optional[mock.Mock], role_name: str -) -> None: - """ - Helper function for common role_create test pattern - """ - await do_role_test(mock_interaction, mock_guild, role_create, role_name) - - mock_interaction.guild.create_role.assert_awaited_once_with( - name=role_name.lower(), mentionable=True, reason=mock.ANY - ) - - @pytest.mark.parametrize("role_name", ["newrole", "nEWRole"]) @pytest.mark.asyncio async def test_role_create_empty( @@ -96,7 +35,12 @@ async def test_role_create_empty( """ Test basic successful creation of a new role """ - await do_role_create_success(mock_interaction, mock_guild_empty, role_name) + mock_interaction.guild = mock_guild_empty + await role_create.callback(mock_interaction, role_name) + mock_interaction.response.send_message.assert_awaited_once() + mock_interaction.guild.create_role.assert_awaited_once_with( + name=role_name.lower(), mentionable=True, reason=mock.ANY + ) @pytest.mark.asyncio @@ -106,7 +50,13 @@ async def test_role_create_populated( """ Tests successful creation of a new role when there exist other roles """ - await do_role_create_success(mock_interaction, mock_guild_populated, "anewrole") + role_name = "anewrole" + mock_interaction.guild = mock_guild_populated + await role_create.callback(mock_interaction, role_name) + mock_interaction.response.send_message.assert_awaited_once() + mock_interaction.guild.create_role.assert_awaited_once_with( + name=role_name.lower(), mentionable=True, reason=mock.ANY + ) @pytest.mark.parametrize("invalid_role_name", ["@newrole", "new@role"]) @@ -117,13 +67,11 @@ async def test_role_create_invalid_name( """ Test failure to create a role with an invalid name """ - await do_role_create_failure( - mock_interaction, - mock_guild_empty, - invalid_role_name, - exception.MemebotUserError, - ) + with pytest.raises(exception.MemebotUserError): + mock_interaction.guild = mock_guild_empty + await role_create.callback(mock_interaction, invalid_role_name) mock_guild_empty.create_role.assert_not_awaited() + mock_interaction.response.send_message.assert_not_awaited() @pytest.mark.asyncio @@ -131,9 +79,9 @@ async def test_role_create_in_dm(mock_interaction: mock.Mock) -> None: """ Test creating a role in DMs fails gracefully """ - await do_role_create_failure( - mock_interaction, None, "dmrole", exception.MemebotUserError - ) + with pytest.raises(exception.MemebotUserError): + await role_create.callback(mock_interaction, "dmrole") + mock_interaction.response.send_message.assert_not_awaited() @pytest.mark.parametrize("conflict_name", ["conflictrole", "ConFlictROLE"]) @@ -145,35 +93,30 @@ async def test_role_create_conflict( Test creating a role fails when there already exists a role with the same name """ mock_guild_populated.roles[1].name = conflict_name.lower() - await do_role_create_failure( - mock_interaction, - mock_guild_populated, - conflict_name, - exception.MemebotUserError, - ) + with pytest.raises(exception.MemebotUserError): + mock_interaction.guild = mock_guild_populated + await role_create.callback(mock_interaction, conflict_name) mock_guild_populated.create_role.assert_not_called() + mock_interaction.response.send_message.assert_not_awaited() @pytest.mark.asyncio async def test_role_create_permission_error( - mock_interaction: mock.Mock, - mock_guild_populated: mock.Mock, + mock_interaction: mock.Mock, mock_guild_populated: mock.Mock ) -> None: """ Test attempting to create a role when Memebot doesn't have permission to do so """ with mock.patch("requests.Response") as MockResponse: role_name = "forbiddenrole" - # Force create_role to throw a dummy exception + mock_interaction.guild = mock_guild_populated mock_guild_populated.create_role = mock.AsyncMock( side_effect=discord.Forbidden(MockResponse(), None) ) - await do_role_create_failure( - mock_interaction, - mock_guild_populated, - role_name, - exception.MemebotInternalError, - ) + + with pytest.raises(exception.MemebotInternalError): + await role_create.callback(mock_interaction, role_name) + mock_interaction.response.send_message.assert_not_awaited() @pytest.mark.asyncio @@ -185,16 +128,14 @@ async def test_role_create_http_failure( """ with mock.patch("requests.Response") as MockResponse: role_name = "failurerole" - # Force create_role to throw a dummy exception + mock_interaction.guild = mock_guild_populated mock_guild_populated.create_role = mock.AsyncMock( side_effect=discord.HTTPException(MockResponse(), None) ) - await do_role_create_failure( - mock_interaction, - mock_guild_populated, - role_name, - exception.MemebotInternalError, - ) + + with pytest.raises(exception.MemebotInternalError): + await role_create.callback(mock_interaction, role_name) + mock_interaction.response.send_message.assert_not_awaited() @pytest.mark.asyncio @@ -202,14 +143,14 @@ async def test_role_delete_happy( mock_interaction: mock.Mock, mock_guild_populated: mock.Mock ) -> None: """ - Test deleting role from a guild which contains the role - and the role only has 0 members + Test deleting role from a guild which contains the role and the role only has 0 members """ target_role = mock_guild_populated.roles[0] target_role.members = [] - await do_role_test(mock_interaction, mock_guild_populated, role_delete, target_role) + mock_interaction.guild = mock_guild_populated + await role_delete.callback(mock_interaction, target_role) + mock_interaction.response.send_message.assert_awaited_once() - # Check that delete was only called on the desired role for role in mock_guild_populated.roles: if role != target_role: role.delete.assert_not_awaited() @@ -229,16 +170,12 @@ async def test_role_delete_too_many_members( mock_users = [MockUser() for _ in range(n_members)] target_role = mock_guild_populated.roles[0] target_role.members = mock_users + mock_interaction.guild = mock_guild_populated - await do_role_test_failure( - mock_interaction, - mock_guild_populated, - exception.MemebotUserError, - role_delete, - target_role, - ) - + with pytest.raises(exception.MemebotUserError): + await role_delete.callback(mock_interaction, target_role) target_role.delete.assert_not_awaited() + mock_interaction.response.send_message.assert_not_awaited() @pytest.mark.asyncio @@ -250,17 +187,14 @@ async def test_role_delete_forbidden( """ with mock.patch("requests.Response") as MockResponse: target_role = mock_guild_populated.roles[2] - # Force role.delete to throw a dummy exception + mock_interaction.guild = mock_guild_populated target_role.delete = mock.AsyncMock( side_effect=discord.Forbidden(MockResponse(), None) ) - await do_role_test_failure( - mock_interaction, - mock_guild_populated, - exception.MemebotInternalError, - role_delete, - target_role, - ) + + with pytest.raises(exception.MemebotInternalError): + await role_delete.callback(mock_interaction, target_role) + mock_interaction.response.send_message.assert_not_awaited() @pytest.mark.asyncio @@ -272,17 +206,14 @@ async def test_role_delete_http_failure( """ with mock.patch("requests.Response") as MockResponse: target_role = mock_guild_populated.roles[2] - # Force role.delete to throw a dummy exception + mock_interaction.guild = mock_guild_populated target_role.delete = mock.AsyncMock( side_effect=discord.HTTPException(MockResponse(), None) ) - await do_role_test_failure( - mock_interaction, - mock_guild_populated, - exception.MemebotInternalError, - role_delete, - target_role, - ) + + with pytest.raises(exception.MemebotInternalError): + await role_delete.callback(mock_interaction, target_role) + mock_interaction.response.send_message.assert_not_awaited() @pytest.mark.asyncio @@ -293,7 +224,9 @@ async def test_role_join_happy( Tests successfully joining an existing role for a user with no roles """ target_role = mock_guild_populated.roles[0] - await do_role_test(mock_interaction, mock_guild_populated, role_join, target_role) + mock_interaction.guild = mock_guild_populated + await role_join.callback(mock_interaction, target_role) + mock_interaction.response.send_message.assert_awaited_once() mock_interaction.user.add_roles.assert_awaited_once_with( target_role, reason=mock.ANY ) @@ -308,13 +241,10 @@ async def test_role_join_conflict( """ target_role = mock_guild_populated.roles[0] mock_interaction.user.roles = [target_role] - await do_role_test_failure( - mock_interaction, - mock_guild_populated, - exception.MemebotUserError, - role_join, - target_role, - ) + mock_interaction.guild = mock_guild_populated + + with pytest.raises(exception.MemebotUserError): + await role_join.callback(mock_interaction, target_role) mock_interaction.user.add_roles.assert_not_awaited() mock_interaction.response.send_message.assert_not_awaited() @@ -328,18 +258,15 @@ async def test_role_join_forbidden( """ with mock.patch("requests.Response") as MockResponse: target_role = mock_guild_populated.roles[2] - # Force role.delete to throw a dummy exception + mock_interaction.guild = mock_guild_populated mock_interaction.user.add_roles = mock.AsyncMock( side_effect=discord.Forbidden(MockResponse(), None) ) - await do_role_test_failure( - mock_interaction, - mock_guild_populated, - exception.MemebotInternalError, - role_join, - target_role, - ) - mock_interaction.user.add_roles.assert_awaited_once() + + with pytest.raises(exception.MemebotInternalError): + await role_join.callback(mock_interaction, target_role) + mock_interaction.user.add_roles.assert_awaited_once() + mock_interaction.response.send_message.assert_not_awaited() @pytest.mark.asyncio @@ -351,18 +278,15 @@ async def test_role_join_failure( """ with mock.patch("requests.Response") as MockResponse: target_role = mock_guild_populated.roles[2] - # Force role.delete to throw a dummy exception + mock_interaction.guild = mock_guild_populated mock_interaction.user.add_roles = mock.AsyncMock( side_effect=discord.HTTPException(MockResponse(), None) ) - await do_role_test_failure( - mock_interaction, - mock_guild_populated, - exception.MemebotInternalError, - role_join, - target_role, - ) - mock_interaction.user.add_roles.assert_awaited_once() + + with pytest.raises(exception.MemebotInternalError): + await role_join.callback(mock_interaction, target_role) + mock_interaction.user.add_roles.assert_awaited_once() + mock_interaction.response.send_message.assert_not_awaited() @pytest.mark.asyncio @@ -374,9 +298,9 @@ async def test_role_leave_with_membership( """ target_role = mock_guild_populated.roles[1] target_role.members = [mock_interaction.user] - - await do_role_test(mock_interaction, mock_guild_populated, role_leave, target_role) - + mock_interaction.guild = mock_guild_populated + await role_leave.callback(mock_interaction, target_role) + mock_interaction.response.send_message.assert_awaited_once() mock_interaction.user.remove_roles.assert_awaited_once_with( target_role, reason=mock.ANY ) @@ -391,15 +315,12 @@ async def test_role_leave_no_membership( """ target_role = mock_guild_populated.roles[1] target_role.members = [] + mock_interaction.guild = mock_guild_populated - await do_role_test_failure( - mock_interaction, - mock_guild_populated, - exception.MemebotUserError, - role_leave, - target_role, - ) + with pytest.raises(exception.MemebotUserError): + await role_leave.callback(mock_interaction, target_role) mock_interaction.user.remove_roles.assert_not_awaited() + mock_interaction.response.send_message.assert_not_awaited() @pytest.mark.asyncio @@ -412,18 +333,15 @@ async def test_role_leave_forbidden( with mock.patch("requests.Response") as MockResponse: target_role = mock_guild_populated.roles[2] target_role.members = [mock_interaction.user] - # Force role.delete to throw a dummy exception + mock_interaction.guild = mock_guild_populated mock_interaction.user.remove_roles = mock.AsyncMock( side_effect=discord.Forbidden(MockResponse(), None) ) - await do_role_test_failure( - mock_interaction, - mock_guild_populated, - exception.MemebotInternalError, - role_leave, - target_role, - ) - mock_interaction.user.remove_roles.assert_awaited_once() + + with pytest.raises(exception.MemebotInternalError): + await role_leave.callback(mock_interaction, target_role) + mock_interaction.user.remove_roles.assert_awaited_once() + mock_interaction.response.send_message.assert_not_awaited() @pytest.mark.asyncio @@ -436,18 +354,15 @@ async def test_role_leave_failure( with mock.patch("requests.Response") as MockResponse: target_role = mock_guild_populated.roles[2] target_role.members = [mock_interaction.user] - # Force role.delete to throw a dummy exception + mock_interaction.guild = mock_guild_populated mock_interaction.user.remove_roles = mock.AsyncMock( side_effect=discord.HTTPException(MockResponse(), None) ) - await do_role_test_failure( - mock_interaction, - mock_guild_populated, - exception.MemebotInternalError, - role_leave, - target_role, - ) - mock_interaction.user.remove_roles.assert_awaited_once() + + with pytest.raises(exception.MemebotInternalError): + await role_leave.callback(mock_interaction, target_role) + mock_interaction.user.remove_roles.assert_awaited_once() + mock_interaction.response.send_message.assert_not_awaited() @pytest.mark.asyncio @@ -460,7 +375,8 @@ async def test_role_list_all_roles( """ Test ability to list all roles """ - await do_role_test(mock_interaction, mock_guild_populated, role_list, None) + mock_interaction.guild = mock_guild_populated + await role_list.callback(mock_interaction, None) expected_output = f"Roles managed through `/role` command:\n- bar\n- baz\n- foo" mock_interaction.response.send_message.assert_awaited_once_with(expected_output) @@ -476,7 +392,8 @@ async def test_role_list_no_roles( Test that role list works when there are no manageable roles """ mock_guild.roles = discord.utils.SequenceProxy([mock_role_everyone, mock_role_bot]) - await do_role_test(mock_interaction, mock_guild, role_list, None) + mock_interaction.guild = mock_guild + await role_list.callback(mock_interaction, None) expected_output = f"Roles managed through `/role` command:" mock_interaction.response.send_message.assert_awaited_once_with(expected_output) @@ -492,7 +409,8 @@ async def test_role_list_user_own_roles( """ target = mock_interaction.user mock_role_bar.members = discord.utils.SequenceProxy([target]) - await do_role_test(mock_interaction, mock_guild_populated, role_list, target) + mock_interaction.guild = mock_guild_populated + await role_list.callback(mock_interaction, target) expected_output = ( f"Roles for user {target.name} managed through `/role` command:\n- bar" ) @@ -511,7 +429,8 @@ async def test_role_list_other_user_roles( with mock.patch("discord.Member", spec=True) as MockMember: target = MockMember() mock_role_bar.members = discord.utils.SequenceProxy([target]) - await do_role_test(mock_interaction, mock_guild_populated, role_list, target) + mock_interaction.guild = mock_guild_populated + await role_list.callback(mock_interaction, target) expected_output = ( f"Roles for user {target.name} managed through `/role` command:\n- bar" ) @@ -526,7 +445,8 @@ async def test_role_list_user_no_roles( Test role list when retrieving roles of a user who is not in any manageable roles """ target = mock_interaction.user - await do_role_test(mock_interaction, mock_guild_populated, role_list, target) + mock_interaction.guild = mock_guild_populated + await role_list.callback(mock_interaction, target) expected_output = f"Roles for user {target.name} managed through `/role` command:" mock_interaction.response.send_message.assert_awaited_once_with(expected_output) @@ -542,7 +462,8 @@ async def test_role_list_role( """ user = mock_interaction.user mock_role_bar.members = discord.utils.SequenceProxy([user]) - await do_role_test(mock_interaction, mock_guild_populated, role_list, mock_role_bar) + mock_interaction.guild = mock_guild_populated + await role_list.callback(mock_interaction, mock_role_bar) expected = f"Members of `@{mock_role_bar.name}`:\n- {user.nick} ({user.name})" mock_interaction.response.send_message.assert_awaited_once_with(expected) @@ -557,9 +478,8 @@ async def test_role_list_role_no_members( Test role list when retrieving members of a role with no members """ with pytest.raises(exception.MemebotUserError): - await do_role_test( - mock_interaction, mock_guild_populated, role_list, mock_role_bar - ) + mock_interaction.guild = mock_guild_populated + await role_list.callback(mock_interaction, mock_role_bar) mock_interaction.response.send_message.assert_not_awaited() @@ -573,22 +493,14 @@ async def test_role_list_role_not_managed( with mock.patch("discord.Role", spec=True) as MockRolePrivileged: mock_role_privileged = MockRolePrivileged() mock_role_privileged.members = discord.utils.SequenceProxy([]) + mock_interaction.guild = mock_guild_populated mock_guild_populated.roles = discord.utils.SequenceProxy( - [ - # Append a role to the end of the list, - # making it higher priority than the bot role - *mock_guild_populated.roles, - mock_role_privileged, - ] - ) - with pytest.raises(exception.MemebotUserError): - # TODO I believe this only throws because the role is empty. - # Memebot *does* check non-managed roles via role list. - # Should it? - await do_role_test( - mock_interaction, mock_guild_populated, role_list, mock_role_privileged + [*mock_guild_populated.roles, mock_role_privileged] ) - mock_interaction.response.send_message.assert_not_awaited() + + with pytest.raises(exception.MemebotUserError): + await role_list.callback(mock_interaction, mock_role_privileged) + mock_interaction.response.send_message.assert_not_awaited() @pytest.mark.asyncio @@ -602,7 +514,8 @@ async def test_role_list_member( Test listing roles of a particular user """ mock_role_bar.members = discord.utils.SequenceProxy([mock_member]) - await do_role_test(mock_interaction, mock_guild_populated, role_list, mock_member) + mock_interaction.guild = mock_guild_populated + await role_list.callback(mock_interaction, mock_member) mock_interaction.response.send_message.assert_awaited_once_with( f"Roles for user {mock_member.name} managed through `/role` command:" f"\n- {mock_role_bar.name}" @@ -618,7 +531,7 @@ async def test_role_list_member_no_roles( """ Test listing roles of a particular user which is not a member of any role """ - await do_role_test(mock_interaction, mock_guild_populated, role_list, mock_member) + role_list.callback(mock_interaction, mock_member) # TODO This does not respond with an error. Should it? # Should the message be different if not? mock_interaction.response.send_message.assert_awaited_once_with( From 8470738b67a54bb44d406d9dec5f6473ccc19158 Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Tue, 12 Nov 2024 18:02:50 -0500 Subject: [PATCH 26/26] Fix failing `/role` tests --- tests/commands/test_role.py | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/tests/commands/test_role.py b/tests/commands/test_role.py index 4abc3bb..1d994ac 100644 --- a/tests/commands/test_role.py +++ b/tests/commands/test_role.py @@ -483,26 +483,6 @@ async def test_role_list_role_no_members( mock_interaction.response.send_message.assert_not_awaited() -@pytest.mark.asyncio -async def test_role_list_role_not_managed( - mock_interaction: mock.Mock, mock_guild_populated: mock.Mock -) -> None: - """ - Test role list when retrieving members of a non-managed role - """ - with mock.patch("discord.Role", spec=True) as MockRolePrivileged: - mock_role_privileged = MockRolePrivileged() - mock_role_privileged.members = discord.utils.SequenceProxy([]) - mock_interaction.guild = mock_guild_populated - mock_guild_populated.roles = discord.utils.SequenceProxy( - [*mock_guild_populated.roles, mock_role_privileged] - ) - - with pytest.raises(exception.MemebotUserError): - await role_list.callback(mock_interaction, mock_role_privileged) - mock_interaction.response.send_message.assert_not_awaited() - - @pytest.mark.asyncio async def test_role_list_member( mock_interaction: mock.Mock, @@ -531,7 +511,7 @@ async def test_role_list_member_no_roles( """ Test listing roles of a particular user which is not a member of any role """ - role_list.callback(mock_interaction, mock_member) + await role_list.callback(mock_interaction, mock_member) # TODO This does not respond with an error. Should it? # Should the message be different if not? mock_interaction.response.send_message.assert_awaited_once_with(