From 74a25d5cf182bcc9386350d2c8fb4ef84fd99da6 Mon Sep 17 00:00:00 2001 From: Bast Date: Tue, 31 Aug 2021 23:16:54 -0700 Subject: [PATCH 01/83] Add basic event dispatcher --- modmail/utils/dispatcher.py | 159 +++++++++++++++++++++++++ tests/modmail/utils/test_dispatcher.py | 151 +++++++++++++++++++++++ 2 files changed, 310 insertions(+) create mode 100644 modmail/utils/dispatcher.py create mode 100644 tests/modmail/utils/test_dispatcher.py diff --git a/modmail/utils/dispatcher.py b/modmail/utils/dispatcher.py new file mode 100644 index 00000000..bde16121 --- /dev/null +++ b/modmail/utils/dispatcher.py @@ -0,0 +1,159 @@ +import asyncio +import bisect +import logging +from typing import Callable, Coroutine, Dict, List, Optional, Union + +from modmail import ModmailLogger + +logger: ModmailLogger = logging.getLogger("modmail.dispatcher") + +CoroutineFunction = Callable[..., Coroutine] + + +class Dispatcher: + """ + Dispatches events through an async event handler system. + + Supports blocking events and priority. + """ + + # These are separate because it makes using the bisect module easier. See _register_handler. + blocking_handlers: Dict[str, List[Optional[CoroutineFunction]]] + blocking_priorities: Dict[str, List[int]] + handlers: Dict[str, List[CoroutineFunction]] + + def __init__(self, *event_names: str): + self.handlers = {} + self.blocking_handlers = {} + self.blocking_priorities = {} + + self.register_event(*event_names) + + def register_event(self, *event_names: str) -> None: + """ + Registers an/some event type(s). + + If there's a problem/typo then registering handlers to unknown event types, or + calling handlers on an unknown event type will trigger a warning message. + """ + for event_name in event_names: + self.handlers[event_name] = [] + self.blocking_handlers[event_name] = [] + self.blocking_priorities[event_name] = [] + + def _register_handler( + self, + event_name: Optional[str], + priority: Optional[int], + func: CoroutineFunction, + ) -> None: + """ + Actually register the handler. + + This function contains registration code, to keep it separate from the decorator code. + """ + logger.debug("Registering %r to be called for event '%s', priority %s", func, event_name, priority) + + # This better fits under register(), but it'd be duped there. + # This extracts the event name from the function name, formatted on_event_name, if it's not provided + if not event_name: + if func.__name__.startswith("on_"): + event_name = func.__name__[3:] + else: + raise ValueError( + "You must pass an event name if the function name doesn't follow the on_eventname format." + ) + + if event_name not in self.handlers: + logger.warning( + "Warning: event handler %r registered for event name '%s' that was not registered.", + func, + event_name, + ) + self.handlers[event_name] = [] + self.blocking_handlers[event_name] = [] + self.blocking_priorities[event_name] = [] + + if priority is None: + self.handlers[event_name].append(func) + else: + index = bisect.bisect_left(self.blocking_priorities[event_name], priority) + self.blocking_priorities[event_name].insert(index, priority) + self.blocking_handlers[event_name].insert(index, func) + + def register( + self, + event_name: Optional[str] = None, + func: Optional[CoroutineFunction] = None, + priority: Optional[int] = None, + ) -> Union[CoroutineFunction, Callable]: + """ + Register an event handler to be called when this event is dispatched. + + This can be used both as a raw call (`register("thread_create", priority=3)`) + or as a decorator: + @dispatcher.register("thread_create") + def handle_new_threads(... + + If the event name is not provided, it is extracted from the method name similar to how dpy does it. + + Priority is optional. If provided, the handler will be called in order according to it's priority. + If the handler returns True, the event is considered "handled" and further dispatch is cancelled. + Lower numbers go first. Negative numbers are supported. + + If priority is not provided the event is dispatched asynchronously after all blocking handlers, + and the return result of the handler has no effect. + """ + if func: + self._register_handler(event_name, priority, func) + return func + + def register_decorator(func: CoroutineFunction) -> CoroutineFunction: + self._register_handler(event_name, priority, func) + return func + + return register_decorator + + def unregister(self, func: CoroutineFunction, *event_names: Optional[str]) -> None: + """ + Unregister a function from dispatch. + + If event_names is not provided, removes it from all handlers. + If you have accidentally added a handler twice to an event, this will only remove one instance. + """ + if not event_names: + event_names = self.handlers.keys() + + for event_name in event_names: + if func in self.handlers[event_name]: + self.handlers[event_name].remove(func) + + if func in self.blocking_handlers[event_name]: + # blocking handlers are two separate lists because it makes searching for items + # (and thus bisect) a hundred times easier. But that does mean we have to keep them in sync. + index = self.blocking_handlers[event_name].index(func) + del self.blocking_handlers[event_name][index] + del self.blocking_priorities[event_name][index] + + async def dispatch(self, event_name: str, *args, **kwargs) -> None: + """ + Trigger dispatch of an event, passing args directly to each handler. + + Beware passing mutable args--previous handlers, if misbehaving, can mutate them. + """ + if event_name not in self.blocking_handlers: + logger.exception( + "Unregistered event '%s' was dispatched to no handlers with data: %r %r", + event_name, + args, + kwargs, + ) + self.blocking_handlers[event_name] = [] + self.handlers[event_name] = [] + self.blocking_priorities[event_name] = [] + + for handler in self.blocking_handlers[event_name]: + if await handler(*args, **kwargs): + return + + await asyncio.gather(*(handler(*args, **kwargs) for handler in self.handlers[event_name])) diff --git a/tests/modmail/utils/test_dispatcher.py b/tests/modmail/utils/test_dispatcher.py new file mode 100644 index 00000000..bb9eaa28 --- /dev/null +++ b/tests/modmail/utils/test_dispatcher.py @@ -0,0 +1,151 @@ +import pytest + +from modmail.utils.dispatcher import Dispatcher + + +@pytest.fixture +def dispatcher() -> Dispatcher: + """Ensure we can create a dispatcher.""" + return Dispatcher("member_leave") + + +def call_counter(): + """Generates a function that returns the number of times it was called.""" + tocks = 0 + + def counter(): + nonlocal tocks + tocks += 1 + return tocks + + return counter + + +def make_mock_handler(): + """Generates an async function that returns the number of times it was called.""" + tocks = 0 + + async def mock_handler(*args): + nonlocal tocks + tocks += 1 + return tocks + + return mock_handler + + +@pytest.mark.dependency(name="register_thread_events") +def test_register_thread_create_event(dispatcher: Dispatcher): + """Ensure registering events functions at all.""" + dispatcher.register_event("thread_create", "thread_close", "thread_message") + + +@pytest.mark.dependency(depends_on=["register_thread_events"]) +def test_register_function_straight(dispatcher: Dispatcher): + """Test the regular form of handler registration.""" + handler = make_mock_handler() + + dispatcher.register("thread_create", handler) + + +@pytest.mark.dependency(depends_on=["register_thread_events"]) +@pytest.mark.asyncio +async def test_register_function_decorator(dispatcher: Dispatcher): + """Test the decorator form of handler registration.""" + calls = 0 + + @dispatcher.register("thread_create") + async def mock_handler(*args): + nonlocal calls + calls += 1 + + await dispatcher.dispatch("thread_create", False) + + assert calls == 1 + + +@pytest.mark.dependency(depends_on=["register_thread_events"]) +@pytest.mark.asyncio +async def test_register_function_deconame(dispatcher: Dispatcher): + """Test that basic dispatch works.""" + calls = 0 + + @dispatcher.register() + async def on_thread_create(*args): + nonlocal calls + calls += 1 + + await dispatcher.dispatch("thread_create", False) + + assert calls == 1 + + +@pytest.mark.dependency(depends_on=["register_thread_events"]) +@pytest.mark.asyncio +async def test_register_unregister(dispatcher: Dispatcher): + """Test that unregistering prevents a handler from being called.""" + calls = 0 + + @dispatcher.register() + async def on_thread_create(*args): + nonlocal calls + calls += 1 + + await dispatcher.dispatch("thread_create", False) + + assert calls == 1 + + dispatcher.unregister(on_thread_create) + + await dispatcher.dispatch("thread_create", True) + + assert calls == 1 + + +@pytest.mark.dependency(depends_on=["register_thread_events"]) +@pytest.mark.asyncio +async def test_priority_order(dispatcher: Dispatcher): + """Test priority ordering and blocking of further event dispatch works..""" + calls = 0 + + @dispatcher.register() + async def on_thread_create(*args): + nonlocal calls + calls += 1 + + priority_calls = 0 + + @dispatcher.register("thread_create", priority=3) + async def priority_handler(*args): + nonlocal priority_calls + priority_calls += 1 + return True + + await dispatcher.dispatch("thread_create", False) + + assert priority_calls == 1 + assert calls == 0 + + high_priority_calls = 0 + + @dispatcher.register("thread_create", priority=1) + async def high_priority_handler(*args): + nonlocal high_priority_calls + high_priority_calls += 1 + return False + + await dispatcher.dispatch("thread_create", False) + + assert priority_calls == 2 + assert calls == 0 + assert high_priority_calls == 1 + + +@pytest.mark.dependency(depends_on=["register_thread_events"]) +@pytest.mark.asyncio +async def test_bad_name_raises(dispatcher: Dispatcher): + """Test that attempting to register a function without a clear event name fails.""" + with pytest.raises(ValueError): + + @dispatcher.register() + async def bad_name(*args): + pass From b5b5eb85c0598af954c16061e444099f87333ad9 Mon Sep 17 00:00:00 2001 From: Bast Date: Tue, 31 Aug 2021 23:29:31 -0700 Subject: [PATCH 02/83] Log when attempting to unregister from a non-registered event type --- modmail/utils/dispatcher.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/modmail/utils/dispatcher.py b/modmail/utils/dispatcher.py index bde16121..3e54ec5c 100644 --- a/modmail/utils/dispatcher.py +++ b/modmail/utils/dispatcher.py @@ -125,6 +125,14 @@ def unregister(self, func: CoroutineFunction, *event_names: Optional[str]) -> No event_names = self.handlers.keys() for event_name in event_names: + if event_name not in self.handlers: + logger.exception( + "Attempted to unregister handler %r from event name %s, which wasn't registered.", + func, + event_name, + ) + continue + if func in self.handlers[event_name]: self.handlers[event_name].remove(func) From 40f76315af2ef84c097299fe6c105abdc7cb2d6a Mon Sep 17 00:00:00 2001 From: Bast Date: Tue, 31 Aug 2021 23:29:46 -0700 Subject: [PATCH 03/83] Remove hardcoded logger name from dispatcher --- modmail/utils/dispatcher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modmail/utils/dispatcher.py b/modmail/utils/dispatcher.py index 3e54ec5c..23815bb1 100644 --- a/modmail/utils/dispatcher.py +++ b/modmail/utils/dispatcher.py @@ -5,7 +5,7 @@ from modmail import ModmailLogger -logger: ModmailLogger = logging.getLogger("modmail.dispatcher") +logger: ModmailLogger = logging.getLogger(__name__) CoroutineFunction = Callable[..., Coroutine] From 6c1667843e541ff3fca13aa6efc54609a7daf288 Mon Sep 17 00:00:00 2001 From: Bast Date: Tue, 31 Aug 2021 23:31:57 -0700 Subject: [PATCH 04/83] Add more dispatcher tests for unregistration --- tests/modmail/utils/test_dispatcher.py | 69 ++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/tests/modmail/utils/test_dispatcher.py b/tests/modmail/utils/test_dispatcher.py index bb9eaa28..b0e3f60b 100644 --- a/tests/modmail/utils/test_dispatcher.py +++ b/tests/modmail/utils/test_dispatcher.py @@ -101,6 +101,34 @@ async def on_thread_create(*args): assert calls == 1 +@pytest.mark.dependency(depends_on=["register_thread_events"]) +@pytest.mark.asyncio +async def test_unregister_named(dispatcher: Dispatcher): + """Test that we can unregister from only one name.""" + calls = 0 + + @dispatcher.register() + async def on_thread_create(*args): + nonlocal calls + calls += 1 + + await dispatcher.dispatch("thread_create", False) + + assert calls == 1 + + dispatcher.unregister(on_thread_create, "thread_message") + + await dispatcher.dispatch("thread_create", True) + + assert calls == 2 + + dispatcher.unregister(on_thread_create, "thread_create") + + await dispatcher.dispatch("thread_create", True) + + assert calls == 2 + + @pytest.mark.dependency(depends_on=["register_thread_events"]) @pytest.mark.asyncio async def test_priority_order(dispatcher: Dispatcher): @@ -149,3 +177,44 @@ async def test_bad_name_raises(dispatcher: Dispatcher): @dispatcher.register() async def bad_name(*args): pass + + +@pytest.mark.asyncio +async def test_unregister_priority(dispatcher: Dispatcher): + """Test that priority events are successfully unregistered.""" + high_priority_calls = 0 + + @dispatcher.register("thread_create", priority=1) + async def high_priority_handler(*args): + nonlocal high_priority_calls + high_priority_calls += 1 + return False + + await dispatcher.dispatch("thread_create", False) + + assert high_priority_calls == 1 + + dispatcher.unregister(high_priority_handler) + + await dispatcher.dispatch("thread_create", False) + + assert high_priority_calls == 1 + + +@pytest.mark.asyncio +async def test_bad_eventname_register_dispatch(dispatcher: Dispatcher): + """Test that even unregistered events dispatch properly.""" + calls = 0 + + @dispatcher.register() + async def on_unnamed(*args): + nonlocal calls + calls += 1 + + await dispatcher.dispatch("unnamed", False) + + assert calls == 1 + + await dispatcher.dispatch("asdf") + + assert calls == 1 From a4ef24807ce65371fd86306d814b088b46a14a3c Mon Sep 17 00:00:00 2001 From: Bast Date: Tue, 31 Aug 2021 23:32:40 -0700 Subject: [PATCH 05/83] Move dispatcher out of utils --- modmail/{utils => }/dispatcher.py | 0 tests/modmail/{utils => }/test_dispatcher.py | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename modmail/{utils => }/dispatcher.py (100%) rename tests/modmail/{utils => }/test_dispatcher.py (99%) diff --git a/modmail/utils/dispatcher.py b/modmail/dispatcher.py similarity index 100% rename from modmail/utils/dispatcher.py rename to modmail/dispatcher.py diff --git a/tests/modmail/utils/test_dispatcher.py b/tests/modmail/test_dispatcher.py similarity index 99% rename from tests/modmail/utils/test_dispatcher.py rename to tests/modmail/test_dispatcher.py index b0e3f60b..05a4d505 100644 --- a/tests/modmail/utils/test_dispatcher.py +++ b/tests/modmail/test_dispatcher.py @@ -1,6 +1,6 @@ import pytest -from modmail.utils.dispatcher import Dispatcher +from modmail.dispatcher import Dispatcher @pytest.fixture From bfcbaccb9519f05daad372d82d5e4b8f162d6b99 Mon Sep 17 00:00:00 2001 From: Bast Date: Tue, 31 Aug 2021 23:43:13 -0700 Subject: [PATCH 06/83] Add changelog entry --- docs/changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog.md b/docs/changelog.md index 14225c04..d6978d00 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Running the bot after configuring the env vars is now as simple as `docker-compose up` - Automatic docker image creation: `ghcr.io/discord-modmail/modmail` (#19) - Dockerfile support for all supported hosting providers. (#58) +- Added Dispatcher system, although it is not hooked in yet. (#71) ### Changed From 791dea415a7d3fcd7c5464df26ff27c71f1b458f Mon Sep 17 00:00:00 2001 From: Bast Date: Wed, 1 Sep 2021 20:16:23 -0700 Subject: [PATCH 07/83] Add a general nonblocking coroutine decorator to general utilities --- modmail/utils/general.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 modmail/utils/general.py diff --git a/modmail/utils/general.py b/modmail/utils/general.py new file mode 100644 index 00000000..9cc48ad3 --- /dev/null +++ b/modmail/utils/general.py @@ -0,0 +1,24 @@ +import asyncio +from functools import wraps +from typing import Callable, Coroutine + +CoroutineFunction = Callable[..., Coroutine] + + +def nonblocking(func: CoroutineFunction) -> CoroutineFunction: + """Converts a coroutine into one that when awaited does not block.""" + + @wraps(func) + async def wrapper(*args, **kwargs): + """Start a coroutine without blocking and triggering exceptions properly.""" + task = asyncio.create_task(func(*args, **kwargs)) + + def ensure_exception(fut: asyncio.Future) -> None: + """Ensure an exception in a task is raised without hard awaiting.""" + if fut.done() and not fut.cancelled(): + return + fut.result() + + task.add_done_callback(ensure_exception) + + return wrapper From 7f40935bc2fef69c796390338ac10c529e39c718 Mon Sep 17 00:00:00 2001 From: Bast Date: Wed, 1 Sep 2021 20:16:45 -0700 Subject: [PATCH 08/83] Fix type signature being incorrectly too permissive --- modmail/dispatcher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index 23815bb1..4aabfb49 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -18,7 +18,7 @@ class Dispatcher: """ # These are separate because it makes using the bisect module easier. See _register_handler. - blocking_handlers: Dict[str, List[Optional[CoroutineFunction]]] + blocking_handlers: Dict[str, List[CoroutineFunction]] blocking_priorities: Dict[str, List[int]] handlers: Dict[str, List[CoroutineFunction]] From f3f528b7b53ad9d69b73220b6b1405818050f9af Mon Sep 17 00:00:00 2001 From: Bast Date: Wed, 1 Sep 2021 20:17:30 -0700 Subject: [PATCH 09/83] Rename Dispatcher.register_event and add a better explanation of it's purpose to its docstring --- modmail/dispatcher.py | 12 +++++++----- tests/modmail/test_dispatcher.py | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index 4aabfb49..62b32a84 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -27,14 +27,16 @@ def __init__(self, *event_names: str): self.blocking_handlers = {} self.blocking_priorities = {} - self.register_event(*event_names) + self.register_events(*event_names) - def register_event(self, *event_names: str) -> None: + def register_events(self, *event_names: str) -> None: """ - Registers an/some event type(s). + Registers the given arguments as event types. - If there's a problem/typo then registering handlers to unknown event types, or - calling handlers on an unknown event type will trigger a warning message. + This exists because if a user wants to dispatch or register a handler there is a or + significant possibility of typos. If we make event types manually registered, then we can + fire a warning message in cases that are likely to be typos and make development + significantly easier. """ for event_name in event_names: self.handlers[event_name] = [] diff --git a/tests/modmail/test_dispatcher.py b/tests/modmail/test_dispatcher.py index 05a4d505..43d157f1 100644 --- a/tests/modmail/test_dispatcher.py +++ b/tests/modmail/test_dispatcher.py @@ -36,7 +36,7 @@ async def mock_handler(*args): @pytest.mark.dependency(name="register_thread_events") def test_register_thread_create_event(dispatcher: Dispatcher): """Ensure registering events functions at all.""" - dispatcher.register_event("thread_create", "thread_close", "thread_message") + dispatcher.register_events("thread_create", "thread_close", "thread_message") @pytest.mark.dependency(depends_on=["register_thread_events"]) From ffc9d33473849dbabf54c2d4df635116ecafc87d Mon Sep 17 00:00:00 2001 From: Bast Date: Wed, 1 Sep 2021 20:17:50 -0700 Subject: [PATCH 10/83] Note the purpose for combining nonblocking with event dispatch --- modmail/dispatcher.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index 62b32a84..f27c37e0 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -105,6 +105,9 @@ def handle_new_threads(... If priority is not provided the event is dispatched asynchronously after all blocking handlers, and the return result of the handler has no effect. + + If you want priority and asynchronous dispatch, try using the `nonblocking` decorator from + `modmail.utils.general`. """ if func: self._register_handler(event_name, priority, func) From f4667703cf41e87791f8eeaa51e24ee381b952de Mon Sep 17 00:00:00 2001 From: Bast Date: Wed, 1 Sep 2021 20:18:23 -0700 Subject: [PATCH 11/83] Add function return type hints to dispatcher tests --- tests/modmail/test_dispatcher.py | 42 +++++++++++++++++--------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/tests/modmail/test_dispatcher.py b/tests/modmail/test_dispatcher.py index 43d157f1..6e912f98 100644 --- a/tests/modmail/test_dispatcher.py +++ b/tests/modmail/test_dispatcher.py @@ -1,3 +1,5 @@ +from typing import Callable + import pytest from modmail.dispatcher import Dispatcher @@ -9,11 +11,11 @@ def dispatcher() -> Dispatcher: return Dispatcher("member_leave") -def call_counter(): +def call_counter() -> Callable: """Generates a function that returns the number of times it was called.""" tocks = 0 - def counter(): + def counter() -> int: nonlocal tocks tocks += 1 return tocks @@ -21,11 +23,11 @@ def counter(): return counter -def make_mock_handler(): +def make_mock_handler() -> Callable: """Generates an async function that returns the number of times it was called.""" tocks = 0 - async def mock_handler(*args): + async def mock_handler(*args) -> int: nonlocal tocks tocks += 1 return tocks @@ -34,13 +36,13 @@ async def mock_handler(*args): @pytest.mark.dependency(name="register_thread_events") -def test_register_thread_create_event(dispatcher: Dispatcher): +def test_register_thread_create_event(dispatcher: Dispatcher) -> None: """Ensure registering events functions at all.""" dispatcher.register_events("thread_create", "thread_close", "thread_message") @pytest.mark.dependency(depends_on=["register_thread_events"]) -def test_register_function_straight(dispatcher: Dispatcher): +def test_register_function_straight(dispatcher: Dispatcher) -> None: """Test the regular form of handler registration.""" handler = make_mock_handler() @@ -49,7 +51,7 @@ def test_register_function_straight(dispatcher: Dispatcher): @pytest.mark.dependency(depends_on=["register_thread_events"]) @pytest.mark.asyncio -async def test_register_function_decorator(dispatcher: Dispatcher): +async def test_register_function_decorator(dispatcher: Dispatcher) -> None: """Test the decorator form of handler registration.""" calls = 0 @@ -65,7 +67,7 @@ async def mock_handler(*args): @pytest.mark.dependency(depends_on=["register_thread_events"]) @pytest.mark.asyncio -async def test_register_function_deconame(dispatcher: Dispatcher): +async def test_register_function_deconame(dispatcher: Dispatcher) -> None: """Test that basic dispatch works.""" calls = 0 @@ -81,7 +83,7 @@ async def on_thread_create(*args): @pytest.mark.dependency(depends_on=["register_thread_events"]) @pytest.mark.asyncio -async def test_register_unregister(dispatcher: Dispatcher): +async def test_register_unregister(dispatcher: Dispatcher) -> None: """Test that unregistering prevents a handler from being called.""" calls = 0 @@ -103,7 +105,7 @@ async def on_thread_create(*args): @pytest.mark.dependency(depends_on=["register_thread_events"]) @pytest.mark.asyncio -async def test_unregister_named(dispatcher: Dispatcher): +async def test_unregister_named(dispatcher: Dispatcher) -> None: """Test that we can unregister from only one name.""" calls = 0 @@ -131,19 +133,19 @@ async def on_thread_create(*args): @pytest.mark.dependency(depends_on=["register_thread_events"]) @pytest.mark.asyncio -async def test_priority_order(dispatcher: Dispatcher): +async def test_priority_order(dispatcher: Dispatcher) -> None: """Test priority ordering and blocking of further event dispatch works..""" calls = 0 @dispatcher.register() - async def on_thread_create(*args): + async def on_thread_create(*args) -> None: nonlocal calls calls += 1 priority_calls = 0 @dispatcher.register("thread_create", priority=3) - async def priority_handler(*args): + async def priority_handler(*args) -> bool: nonlocal priority_calls priority_calls += 1 return True @@ -156,7 +158,7 @@ async def priority_handler(*args): high_priority_calls = 0 @dispatcher.register("thread_create", priority=1) - async def high_priority_handler(*args): + async def high_priority_handler(*args) -> bool: nonlocal high_priority_calls high_priority_calls += 1 return False @@ -170,22 +172,22 @@ async def high_priority_handler(*args): @pytest.mark.dependency(depends_on=["register_thread_events"]) @pytest.mark.asyncio -async def test_bad_name_raises(dispatcher: Dispatcher): +async def test_bad_name_raises(dispatcher: Dispatcher) -> None: """Test that attempting to register a function without a clear event name fails.""" with pytest.raises(ValueError): @dispatcher.register() - async def bad_name(*args): + async def bad_name(*args) -> None: pass @pytest.mark.asyncio -async def test_unregister_priority(dispatcher: Dispatcher): +async def test_unregister_priority(dispatcher: Dispatcher) -> None: """Test that priority events are successfully unregistered.""" high_priority_calls = 0 @dispatcher.register("thread_create", priority=1) - async def high_priority_handler(*args): + async def high_priority_handler(*args) -> bool: nonlocal high_priority_calls high_priority_calls += 1 return False @@ -202,12 +204,12 @@ async def high_priority_handler(*args): @pytest.mark.asyncio -async def test_bad_eventname_register_dispatch(dispatcher: Dispatcher): +async def test_bad_eventname_register_dispatch(dispatcher: Dispatcher) -> None: """Test that even unregistered events dispatch properly.""" calls = 0 @dispatcher.register() - async def on_unnamed(*args): + async def on_unnamed(*args) -> None: nonlocal calls calls += 1 From 4940cc3a3c38d7faa24de453b5fcbbb04f601c98 Mon Sep 17 00:00:00 2001 From: Bast Date: Wed, 1 Sep 2021 20:29:16 -0700 Subject: [PATCH 12/83] Annotate that the nonblocking wrapper returns None --- modmail/utils/general.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modmail/utils/general.py b/modmail/utils/general.py index 9cc48ad3..a7f4ef02 100644 --- a/modmail/utils/general.py +++ b/modmail/utils/general.py @@ -9,7 +9,7 @@ def nonblocking(func: CoroutineFunction) -> CoroutineFunction: """Converts a coroutine into one that when awaited does not block.""" @wraps(func) - async def wrapper(*args, **kwargs): + async def wrapper(*args, **kwargs) -> None: """Start a coroutine without blocking and triggering exceptions properly.""" task = asyncio.create_task(func(*args, **kwargs)) From ac9f8119581169a511c506b7769b3ddc5b37aee8 Mon Sep 17 00:00:00 2001 From: Bast Date: Wed, 1 Sep 2021 20:29:38 -0700 Subject: [PATCH 13/83] Readjust how the nonblocking decorator works and add a free utility function to the mix --- modmail/utils/general.py | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/modmail/utils/general.py b/modmail/utils/general.py index a7f4ef02..377793fc 100644 --- a/modmail/utils/general.py +++ b/modmail/utils/general.py @@ -6,19 +6,32 @@ def nonblocking(func: CoroutineFunction) -> CoroutineFunction: - """Converts a coroutine into one that when awaited does not block.""" + """ + Converts a coroutine into one that when awaited does not block. + + Note that the returned function now returns None, rather than returning the task. + This is because it's intended for use with the priority dispatch, and tasks are "true" + and would break that feature. + """ @wraps(func) async def wrapper(*args, **kwargs) -> None: """Start a coroutine without blocking and triggering exceptions properly.""" - task = asyncio.create_task(func(*args, **kwargs)) + await_nonblocking(func(*args, **kwargs)) - def ensure_exception(fut: asyncio.Future) -> None: - """Ensure an exception in a task is raised without hard awaiting.""" - if fut.done() and not fut.cancelled(): - return - fut.result() + return wrapper - task.add_done_callback(ensure_exception) - return wrapper +def await_nonblocking(func: Coroutine) -> asyncio.Task: + """Call a coroutine from potentially sync code without blocking, and ensure errors are reported.""" + task = asyncio.create_task(func) + + def ensure_exception(fut: asyncio.Future) -> None: + """Ensure an exception in a task is raised without hard awaiting.""" + if fut.done() and not fut.cancelled(): + return + fut.result() + + task.add_done_callback(ensure_exception) + + return task From 2c56200e613aa95e41e346606b2f06a4831ccbbc Mon Sep 17 00:00:00 2001 From: Bast Date: Wed, 1 Sep 2021 20:40:12 -0700 Subject: [PATCH 14/83] Add tests for general utils (nonblocking async calls) --- tests/modmail/utils/test_general.py | 42 +++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 tests/modmail/utils/test_general.py diff --git a/tests/modmail/utils/test_general.py b/tests/modmail/utils/test_general.py new file mode 100644 index 00000000..068ef35e --- /dev/null +++ b/tests/modmail/utils/test_general.py @@ -0,0 +1,42 @@ +import asyncio + +import pytest + +from modmail.utils.general import await_nonblocking, nonblocking + + +@pytest.mark.asyncio +async def test_await_nonblocking() -> None: + """Ensure that we can execute coroutines asynchronously from a sync task.""" + a = 0 + + async def test(number: int) -> None: + nonlocal a + a = number + + task = await_nonblocking(test(1)) + assert a == 0 + await task + assert a == 1 + + +@pytest.mark.asyncio +async def test_nonblocking_decorator() -> None: + """ + Ensure the nonblocking decorator possesses nonblocking behavior. + + The sleeps are present to force asyncio to use a given resolution order for the + asynchronous tasks, so we can clearly tell if they were executed parallel or sync. + """ + a = 0 + + @nonblocking + async def test(number: int) -> None: + nonlocal a + await asyncio.sleep(0.05) + a = number + + await test(1) + assert a == 0 + await asyncio.sleep(0.1) + assert a == 1 From bbf15fe7d2f5184fcb1c0618f01542a29f77d52c Mon Sep 17 00:00:00 2001 From: Bast Date: Sat, 11 Sep 2021 22:54:39 -0700 Subject: [PATCH 15/83] Add support for class method registration in event dispatch --- modmail/bot.py | 3 +++ modmail/dispatcher.py | 36 ++++++++++++++++++++++++++++++++ modmail/utils/cogs.py | 6 +++++- tests/modmail/test_dispatcher.py | 27 ++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 1 deletion(-) diff --git a/modmail/bot.py b/modmail/bot.py index e62f1bda..1cb4f446 100644 --- a/modmail/bot.py +++ b/modmail/bot.py @@ -12,6 +12,7 @@ from discord.ext import commands from modmail.config import CONFIG +from modmail.dispatcher import Dispatcher from modmail.log import ModmailLogger from modmail.utils.extensions import EXTENSIONS, NO_UNLOAD, walk_extensions from modmail.utils.plugins import PLUGINS, walk_plugins @@ -35,11 +36,13 @@ class ModmailBot(commands.Bot): """ logger: ModmailLogger = logging.getLogger(__name__) + dispatcher: Dispatcher def __init__(self, **kwargs): self.config = CONFIG self.start_time: t.Optional[arrow.Arrow] = None # arrow.utcnow() self.http_session: t.Optional[ClientSession] = None + self.dispatcher = Dispatcher() status = discord.Status.online activity = Activity(type=discord.ActivityType.listening, name="users dming me!") diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index f27c37e0..abffa7cf 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -1,5 +1,7 @@ import asyncio import bisect +import dis +import inspect import logging from typing import Callable, Coroutine, Dict, List, Optional, Union @@ -43,6 +45,14 @@ def register_events(self, *event_names: str) -> None: self.blocking_handlers[event_name] = [] self.blocking_priorities[event_name] = [] + def activate(self, instance: object) -> None: + """Register all the handlers on a given class instance on instance initialization.""" + for attr in dir(instance): + value = getattr(instance, attr) + if hasattr(value, "_dispatchable") and value._dispatchable: + for (event_name, priority) in value._dispatchable: + self._register_handler(event_name, priority, value) + def _register_handler( self, event_name: Optional[str], @@ -66,6 +76,32 @@ def _register_handler( "You must pass an event name if the function name doesn't follow the on_eventname format." ) + if not hasattr(func, "_dispatchable"): + func._dispatchable = [] + frame = inspect.currentframe() + + # this backs up out until we're no longer in our code here + while frame.f_globals == globals(): + frame = frame.f_back + + # Determine if we're in a class + # This is _super cursed_ but also the only way I can figure out. PR's welcome. + # It may be a better idea to do this via detecting self in the parameters list. + in_class = False + for instruction in dis.get_instructions(frame.f_code): + if instruction.opname == "LOAD_CLASSDEREF": + in_class = True + break + + del frame + + if in_class: + func._dispatchable.append((event_name, priority)) + # We've been given an unbound method. We're registering on a class, so this will be re-called + # later during __init__. We store all the event names it should be registered under on + # _dispatchable for use at that time. + return + if event_name not in self.handlers: logger.warning( "Warning: event handler %r registered for event name '%s' that was not registered.", diff --git a/modmail/utils/cogs.py b/modmail/utils/cogs.py index 5d5a92b8..1a5832c8 100644 --- a/modmail/utils/cogs.py +++ b/modmail/utils/cogs.py @@ -3,6 +3,8 @@ from discord.ext import commands +import modmail + class BitwiseAutoEnum(IntEnum): """Enum class which generates binary value for each item.""" @@ -52,4 +54,6 @@ class ModmailCog(commands.Cog): are equally valid here. """ - pass + def __init__(self, bot: "modmail.bot.ModmailBot"): + self.dispatcher = bot.dispatcher + self.dispatcher.activate(self) diff --git a/tests/modmail/test_dispatcher.py b/tests/modmail/test_dispatcher.py index 6e912f98..7b6b4756 100644 --- a/tests/modmail/test_dispatcher.py +++ b/tests/modmail/test_dispatcher.py @@ -220,3 +220,30 @@ async def on_unnamed(*args) -> None: await dispatcher.dispatch("asdf") assert calls == 1 + + +@pytest.mark.asyncio +async def test_class_dispatch() -> None: + """Ensure the decorator works on class functions as well, and fires only for the instances.""" + dispatcher = Dispatcher("class_test") + + class A: + def __init__(self): + dispatcher.activate(self) + + async def fire(self) -> None: + await dispatcher.dispatch("class_test", 2) + + @dispatcher.register("class_test") + async def on_class_test(self, var1) -> None: + assert var1 == 2 + + assert dispatcher.handlers["class_test"] == [] + + a = A() + + assert dispatcher.handlers["class_test"] == [a.on_class_test] + + await a.on_class_test(2) + + await a.fire() From daed7d27d010fe034c8ac49aa02af976c691c896 Mon Sep 17 00:00:00 2001 From: Bast Date: Sat, 11 Sep 2021 23:04:23 -0700 Subject: [PATCH 16/83] Sync dispatcher to new isort config --- modmail/dispatcher.py | 1 + modmail/utils/general.py | 1 + 2 files changed, 2 insertions(+) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index abffa7cf..39a826c2 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -7,6 +7,7 @@ from modmail import ModmailLogger + logger: ModmailLogger = logging.getLogger(__name__) CoroutineFunction = Callable[..., Coroutine] diff --git a/modmail/utils/general.py b/modmail/utils/general.py index 377793fc..65e10f62 100644 --- a/modmail/utils/general.py +++ b/modmail/utils/general.py @@ -2,6 +2,7 @@ from functools import wraps from typing import Callable, Coroutine + CoroutineFunction = Callable[..., Coroutine] From b04eb2dbebbbbea663c717a2917e0a24c427a9e3 Mon Sep 17 00:00:00 2001 From: Bast Date: Sun, 12 Sep 2021 01:50:24 -0700 Subject: [PATCH 17/83] Fix docstring typo --- modmail/dispatcher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index 39a826c2..1750bab5 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -36,7 +36,7 @@ def register_events(self, *event_names: str) -> None: """ Registers the given arguments as event types. - This exists because if a user wants to dispatch or register a handler there is a or + This exists because if a user wants to dispatch or register a handler there is a significant possibility of typos. If we make event types manually registered, then we can fire a warning message in cases that are likely to be typos and make development significantly easier. From 95f37215690c833d4217c4626743e89c9d59c1a7 Mon Sep 17 00:00:00 2001 From: Bast Date: Fri, 24 Sep 2021 10:29:54 -0700 Subject: [PATCH 18/83] Change dispatcher class support to rely on `self` rather than extraclass variable access Also adjusted class test to be more applicative of what's expected to be in use in reality --- modmail/dispatcher.py | 25 ++++++++----------------- tests/modmail/test_dispatcher.py | 11 ++++++----- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index 1750bab5..b7f90a1a 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -1,6 +1,5 @@ import asyncio import bisect -import dis import inspect import logging from typing import Callable, Coroutine, Dict, List, Optional, Union @@ -79,22 +78,14 @@ def _register_handler( if not hasattr(func, "_dispatchable"): func._dispatchable = [] - frame = inspect.currentframe() - - # this backs up out until we're no longer in our code here - while frame.f_globals == globals(): - frame = frame.f_back - - # Determine if we're in a class - # This is _super cursed_ but also the only way I can figure out. PR's welcome. - # It may be a better idea to do this via detecting self in the parameters list. - in_class = False - for instruction in dis.get_instructions(frame.f_code): - if instruction.opname == "LOAD_CLASSDEREF": - in_class = True - break - - del frame + + # Check for `self` as first argument to tell if we're in a class + # There unfortunately appears to be no better way to do this + func_args = inspect.getfullargspec(func).args + if func_args and func_args[0] == "self": + in_class = True + else: + in_class = False if in_class: func._dispatchable.append((event_name, priority)) diff --git a/tests/modmail/test_dispatcher.py b/tests/modmail/test_dispatcher.py index 7b6b4756..322d51ad 100644 --- a/tests/modmail/test_dispatcher.py +++ b/tests/modmail/test_dispatcher.py @@ -225,24 +225,25 @@ async def on_unnamed(*args) -> None: @pytest.mark.asyncio async def test_class_dispatch() -> None: """Ensure the decorator works on class functions as well, and fires only for the instances.""" - dispatcher = Dispatcher("class_test") class A: + dispatcher = Dispatcher("class_test") + def __init__(self): - dispatcher.activate(self) + self.dispatcher.activate(self) async def fire(self) -> None: - await dispatcher.dispatch("class_test", 2) + await self.dispatcher.dispatch("class_test", 2) @dispatcher.register("class_test") async def on_class_test(self, var1) -> None: assert var1 == 2 - assert dispatcher.handlers["class_test"] == [] + assert A.dispatcher.handlers["class_test"] == [] a = A() - assert dispatcher.handlers["class_test"] == [a.on_class_test] + assert A.dispatcher.handlers["class_test"] == [a.on_class_test] await a.on_class_test(2) From 359deb86d0362f533562fc01e59087785d38f85a Mon Sep 17 00:00:00 2001 From: Bast Date: Wed, 29 Sep 2021 21:29:58 -0700 Subject: [PATCH 19/83] Fix dispatcher method support to support directly registering methods as well And fix error due to setting attributes on bound methods where they are not permitted --- modmail/dispatcher.py | 37 ++++++++++++++++++++++++-------- tests/modmail/test_dispatcher.py | 27 +++++++++++++++++++++++ 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index b7f90a1a..26c100f0 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -2,7 +2,7 @@ import bisect import inspect import logging -from typing import Callable, Coroutine, Dict, List, Optional, Union +from typing import Callable, Coroutine, Dict, List, Optional, Tuple, Union from modmail import ModmailLogger @@ -23,11 +23,13 @@ class Dispatcher: blocking_handlers: Dict[str, List[CoroutineFunction]] blocking_priorities: Dict[str, List[int]] handlers: Dict[str, List[CoroutineFunction]] + pending_handlers: Dict[Callable, List[Tuple[str, float]]] def __init__(self, *event_names: str): self.handlers = {} self.blocking_handlers = {} self.blocking_priorities = {} + self.pending_handlers = {} self.register_events(*event_names) @@ -46,12 +48,29 @@ def register_events(self, *event_names: str) -> None: self.blocking_priorities[event_name] = [] def activate(self, instance: object) -> None: - """Register all the handlers on a given class instance on instance initialization.""" + """ + Register all bound method handlers on a given class instance. + + Should be called during __init__. + """ for attr in dir(instance): value = getattr(instance, attr) - if hasattr(value, "_dispatchable") and value._dispatchable: - for (event_name, priority) in value._dispatchable: - self._register_handler(event_name, priority, value) + if not callable(value): + continue + + # Bound methods have __func__, which returns the actual function + # we use that to determine which method was actually registered. + if not hasattr(value, "__func__"): + continue + + value = value.__func__ + + if value not in self.pending_handlers: + continue + + for (event_name, priority) in self.pending_handlers[value]: + self._register_handler(event_name, priority, value) + self.pending_handlers[value].clear() def _register_handler( self, @@ -76,8 +95,8 @@ def _register_handler( "You must pass an event name if the function name doesn't follow the on_eventname format." ) - if not hasattr(func, "_dispatchable"): - func._dispatchable = [] + if func not in self.pending_handlers: + self.pending_handlers[func] = [] # Check for `self` as first argument to tell if we're in a class # There unfortunately appears to be no better way to do this @@ -88,10 +107,10 @@ def _register_handler( in_class = False if in_class: - func._dispatchable.append((event_name, priority)) # We've been given an unbound method. We're registering on a class, so this will be re-called # later during __init__. We store all the event names it should be registered under on - # _dispatchable for use at that time. + # in our pending_handlers for use at that time. + self.pending_handlers[func].append((event_name, priority)) return if event_name not in self.handlers: diff --git a/tests/modmail/test_dispatcher.py b/tests/modmail/test_dispatcher.py index 322d51ad..3e2df887 100644 --- a/tests/modmail/test_dispatcher.py +++ b/tests/modmail/test_dispatcher.py @@ -248,3 +248,30 @@ async def on_class_test(self, var1) -> None: await a.on_class_test(2) await a.fire() + + +@pytest.mark.asyncio +async def test_class_dispatch_latemethod() -> None: + """Ensure we can register methods after class definition.""" + + class A: + dispatcher = Dispatcher("class_test") + + def __init__(self): + self.dispatcher.register("class_test", self.on_class_test) + + async def fire(self) -> None: + await self.dispatcher.dispatch("class_test", 2) + + async def on_class_test(self, var1) -> None: + assert var1 == 2 + + assert A.dispatcher.handlers["class_test"] == [] + + a = A() + + assert A.dispatcher.handlers["class_test"] == [a.on_class_test] + + await a.on_class_test(2) + + await a.fire() From d229b02203cb04efea004fedfd544b764282024e Mon Sep 17 00:00:00 2001 From: Bast Date: Wed, 29 Sep 2021 21:36:04 -0700 Subject: [PATCH 20/83] More directly assert that dispatcher actually fires in method case --- tests/modmail/test_dispatcher.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/modmail/test_dispatcher.py b/tests/modmail/test_dispatcher.py index 3e2df887..8aa73e6d 100644 --- a/tests/modmail/test_dispatcher.py +++ b/tests/modmail/test_dispatcher.py @@ -253,6 +253,7 @@ async def on_class_test(self, var1) -> None: @pytest.mark.asyncio async def test_class_dispatch_latemethod() -> None: """Ensure we can register methods after class definition.""" + called = False class A: dispatcher = Dispatcher("class_test") @@ -264,6 +265,8 @@ async def fire(self) -> None: await self.dispatcher.dispatch("class_test", 2) async def on_class_test(self, var1) -> None: + nonlocal called + called = True assert var1 == 2 assert A.dispatcher.handlers["class_test"] == [] @@ -272,6 +275,6 @@ async def on_class_test(self, var1) -> None: assert A.dispatcher.handlers["class_test"] == [a.on_class_test] - await a.on_class_test(2) - await a.fire() + + assert called From 8c01ae8fc9b967334a44edd02f6705bc18a203da Mon Sep 17 00:00:00 2001 From: Bast Date: Wed, 29 Sep 2021 21:48:10 -0700 Subject: [PATCH 21/83] Do not clear dispatcher registers on duplicate event name registration --- modmail/dispatcher.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index 26c100f0..98c7dac4 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -43,6 +43,9 @@ def register_events(self, *event_names: str) -> None: significantly easier. """ for event_name in event_names: + if event_name in self.handlers: + # Do not clear registers if a name is already registered + continue self.handlers[event_name] = [] self.blocking_handlers[event_name] = [] self.blocking_priorities[event_name] = [] From 316508625655fcf739a51031205b067e20909db9 Mon Sep 17 00:00:00 2001 From: Bast Date: Wed, 29 Sep 2021 22:26:26 -0700 Subject: [PATCH 22/83] Fix potential infinite loop in dispatcher.activate() and handle already bound methods correctly --- modmail/dispatcher.py | 22 ++++++++++++++-------- tests/modmail/test_dispatcher.py | 1 + 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index 98c7dac4..15fae823 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -66,14 +66,14 @@ def activate(self, instance: object) -> None: if not hasattr(value, "__func__"): continue - value = value.__func__ + underlying_function = value.__func__ - if value not in self.pending_handlers: + if underlying_function not in self.pending_handlers: continue - for (event_name, priority) in self.pending_handlers[value]: + for (event_name, priority) in self.pending_handlers[underlying_function]: self._register_handler(event_name, priority, value) - self.pending_handlers[value].clear() + self.pending_handlers[underlying_function].clear() def _register_handler( self, @@ -98,18 +98,24 @@ def _register_handler( "You must pass an event name if the function name doesn't follow the on_eventname format." ) - if func not in self.pending_handlers: - self.pending_handlers[func] = [] - # Check for `self` as first argument to tell if we're in a class # There unfortunately appears to be no better way to do this func_args = inspect.getfullargspec(func).args if func_args and func_args[0] == "self": - in_class = True + if hasattr(func, "__self__") and func.__self__: + # This is an already bound method + in_class = False + else: + # This is an unbound class method + in_class = True else: + # This method doesn't have self, so it's probably not a class method + # And this is the best we can do in_class = False if in_class: + if func not in self.pending_handlers: + self.pending_handlers[func] = [] # We've been given an unbound method. We're registering on a class, so this will be re-called # later during __init__. We store all the event names it should be registered under on # in our pending_handlers for use at that time. diff --git a/tests/modmail/test_dispatcher.py b/tests/modmail/test_dispatcher.py index 8aa73e6d..432188e9 100644 --- a/tests/modmail/test_dispatcher.py +++ b/tests/modmail/test_dispatcher.py @@ -259,6 +259,7 @@ class A: dispatcher = Dispatcher("class_test") def __init__(self): + self.dispatcher.register_events("class_test") self.dispatcher.register("class_test", self.on_class_test) async def fire(self) -> None: From 629e1faba4deb286c440ac905571402f83d27301 Mon Sep 17 00:00:00 2001 From: Bast Date: Mon, 11 Oct 2021 15:27:02 -0700 Subject: [PATCH 23/83] nit: remove duplicate "warning" label from warning --- modmail/dispatcher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index 15fae823..e3bff695 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -124,7 +124,7 @@ def _register_handler( if event_name not in self.handlers: logger.warning( - "Warning: event handler %r registered for event name '%s' that was not registered.", + "event handler %r registered for event name '%s' that was not registered.", func, event_name, ) From b06c7fc8cc2695821440a95969b05ec63af60e0e Mon Sep 17 00:00:00 2001 From: Bast Date: Mon, 11 Oct 2021 23:02:03 -0700 Subject: [PATCH 24/83] Add utility function to help distinguish functions lingering from reloaded cogs --- modmail/utils/general.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/modmail/utils/general.py b/modmail/utils/general.py index 65e10f62..b2f8b5d4 100644 --- a/modmail/utils/general.py +++ b/modmail/utils/general.py @@ -36,3 +36,29 @@ def ensure_exception(fut: asyncio.Future) -> None: task.add_done_callback(ensure_exception) return task + + +def module_function_disidenticality(func1: Callable, func2: Callable) -> bool: + """ + Determine if two functions are probably from the same module loaded twice. + + Returns true if: + - same name + - same qualname + - same module path + - but not the same function object + + This happens when a module is reloaded and old references are kept around. + We unfortunately cannot compare the module object itself, as I don't know a + way to access it from a function. + + This can also happen if someone is generating functions in a loop and not + setting __name__ or __qualname__, but it's better practice to do that to + name those appropriately. + """ + return ( + func1.__qualname__ == func2.__qualname__ + and func1.__name__ == func2.__name__ + and func1.__module__ == func2.__module__ + and func1 != func2 + ) From fa054d512154d4904ffb51f2f537d0219346db38 Mon Sep 17 00:00:00 2001 From: Bast Date: Mon, 11 Oct 2021 23:19:31 -0700 Subject: [PATCH 25/83] Move handler removal to a dedicated function so it can be reused internally --- modmail/dispatcher.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index e3bff695..d201bf8c 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -195,14 +195,20 @@ def unregister(self, func: CoroutineFunction, *event_names: Optional[str]) -> No continue if func in self.handlers[event_name]: - self.handlers[event_name].remove(func) + self._remove_handler(func, event_name, False) if func in self.blocking_handlers[event_name]: - # blocking handlers are two separate lists because it makes searching for items - # (and thus bisect) a hundred times easier. But that does mean we have to keep them in sync. - index = self.blocking_handlers[event_name].index(func) - del self.blocking_handlers[event_name][index] - del self.blocking_priorities[event_name][index] + self._remove_handler(func, event_name, True) + + def _remove_handler(self, func: CoroutineFunction, event_name: str, blocking: bool = False) -> None: + if blocking: + # blocking handlers are two separate lists because it makes searching for items + # (and thus bisect) a hundred times easier. But that does mean we have to keep them in sync. + index = self.blocking_handlers[event_name].index(func) + del self.blocking_handlers[event_name][index] + del self.blocking_priorities[event_name][index] + else: + self.handlers[event_name].remove(func) async def dispatch(self, event_name: str, *args, **kwargs) -> None: """ From 427f587043ca02db9f693ae3db68d50eaed64fa9 Mon Sep 17 00:00:00 2001 From: Bast Date: Mon, 11 Oct 2021 23:20:01 -0700 Subject: [PATCH 26/83] Appropriately warn and handle when a cog is reloaded that is not unregistering it's handlers --- modmail/dispatcher.py | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index d201bf8c..2cc52888 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -5,12 +5,21 @@ from typing import Callable, Coroutine, Dict, List, Optional, Tuple, Union from modmail import ModmailLogger +from modmail.utils.general import module_function_disidenticality logger: ModmailLogger = logging.getLogger(__name__) CoroutineFunction = Callable[..., Coroutine] +HANDLER_DISIDENTICALITY_WARNING = ( + "Event handler %r registered for event name '%s' a second time," + " but it is _not the same function_. Have you forgotten to add deregistration" + " to cog_unload? By default I've deregistered the old handler to prevent duplication." + " If you actually intend to register two functions with the same name from the same module," + " assign their __name__ and __qualname__ attributes so they are easily distinguishable." +) + class Dispatcher: """ @@ -133,8 +142,40 @@ def _register_handler( self.blocking_priorities[event_name] = [] if priority is None: + if func in self.handlers[event_name]: + logger.error( + "Event handler was already registered as async: handler %s, event name %s." + " Second registration ignored." % (func, event_name) + ) + self._remove_handler(func, event_name, False) + + for handler in self.handlers[event_name]: + if handler.__qualname__ == func.__qualname__: + if module_function_disidenticality(handler, func): + logger.warning( + HANDLER_DISIDENTICALITY_WARNING, + handler, + event_name, + ) + self._remove_handler(handler, event_name, False) self.handlers[event_name].append(func) else: + for handler in self.blocking_handlers[event_name]: + if handler.__qualname__ == func.__qualname__: + if module_function_disidenticality(handler, func): + logger.warning(HANDLER_DISIDENTICALITY_WARNING, handler, event_name) + + self._remove_handler(handler, event_name, True) + + if handler == func: + logger.error( + "Event handler was already registered as blocking: handler %s, event name %s." + " Second registration ignored." % (func, event_name) + ) + + self._remove_handler(handler, event_name, True) + + self.handlers[event_name].append(func) index = bisect.bisect_left(self.blocking_priorities[event_name], priority) self.blocking_priorities[event_name].insert(index, priority) self.blocking_handlers[event_name].insert(index, func) From 9a1dd2e47ae93ad908da6de7ec95a0d5e1e18b93 Mon Sep 17 00:00:00 2001 From: Bast Date: Mon, 11 Oct 2021 23:55:48 -0700 Subject: [PATCH 27/83] Fix accidentally registering all blocking handlers as nonblocking too --- modmail/dispatcher.py | 1 - 1 file changed, 1 deletion(-) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index 2cc52888..784d284d 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -175,7 +175,6 @@ def _register_handler( self._remove_handler(handler, event_name, True) - self.handlers[event_name].append(func) index = bisect.bisect_left(self.blocking_priorities[event_name], priority) self.blocking_priorities[event_name].insert(index, priority) self.blocking_handlers[event_name].insert(index, func) From 45aad7d32b624abe11b9ca39039e02a0ffeadd25 Mon Sep 17 00:00:00 2001 From: Bast Date: Mon, 11 Oct 2021 23:56:13 -0700 Subject: [PATCH 28/83] Clean race condition out of tests --- tests/modmail/test_dispatcher.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/modmail/test_dispatcher.py b/tests/modmail/test_dispatcher.py index 432188e9..8325fc80 100644 --- a/tests/modmail/test_dispatcher.py +++ b/tests/modmail/test_dispatcher.py @@ -186,19 +186,19 @@ async def test_unregister_priority(dispatcher: Dispatcher) -> None: """Test that priority events are successfully unregistered.""" high_priority_calls = 0 - @dispatcher.register("thread_create", priority=1) - async def high_priority_handler(*args) -> bool: + @dispatcher.register("thread_create_two", priority=1) + async def high_priority_handler(*_args) -> bool: nonlocal high_priority_calls high_priority_calls += 1 return False - await dispatcher.dispatch("thread_create", False) + await dispatcher.dispatch("thread_create_two", False) assert high_priority_calls == 1 dispatcher.unregister(high_priority_handler) - await dispatcher.dispatch("thread_create", False) + await dispatcher.dispatch("thread_create_two", False) assert high_priority_calls == 1 From fe07177cfb9d6f0a6f35b6a554345c9bc221fb4b Mon Sep 17 00:00:00 2001 From: Bast Date: Mon, 11 Oct 2021 23:56:39 -0700 Subject: [PATCH 29/83] Change code layout for registration to be cleaner --- modmail/dispatcher.py | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index 784d284d..7b87cf4e 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -141,6 +141,7 @@ def _register_handler( self.blocking_handlers[event_name] = [] self.blocking_priorities[event_name] = [] + # Nonblocking (gathered) if priority is None: if func in self.handlers[event_name]: logger.error( @@ -158,26 +159,29 @@ def _register_handler( event_name, ) self._remove_handler(handler, event_name, False) + self.handlers[event_name].append(func) - else: - for handler in self.blocking_handlers[event_name]: - if handler.__qualname__ == func.__qualname__: - if module_function_disidenticality(handler, func): - logger.warning(HANDLER_DISIDENTICALITY_WARNING, handler, event_name) + return + + # Blocking (run in sequence) + for handler in self.blocking_handlers[event_name]: + if handler.__qualname__ == func.__qualname__: + if module_function_disidenticality(handler, func): + logger.warning(HANDLER_DISIDENTICALITY_WARNING, handler, event_name) - self._remove_handler(handler, event_name, True) + self._remove_handler(handler, event_name, True) - if handler == func: - logger.error( - "Event handler was already registered as blocking: handler %s, event name %s." - " Second registration ignored." % (func, event_name) - ) + if handler == func: + logger.error( + "Event handler was already registered as blocking: handler %s, event name %s." + " Second registration ignored." % (func, event_name) + ) - self._remove_handler(handler, event_name, True) + self._remove_handler(handler, event_name, True) - index = bisect.bisect_left(self.blocking_priorities[event_name], priority) - self.blocking_priorities[event_name].insert(index, priority) - self.blocking_handlers[event_name].insert(index, func) + index = bisect.bisect_left(self.blocking_priorities[event_name], priority) + self.blocking_priorities[event_name].insert(index, priority) + self.blocking_handlers[event_name].insert(index, func) def register( self, From 2a96b9e23c6eedc7920c7d609ecc5d06fa2c4014 Mon Sep 17 00:00:00 2001 From: Bast Date: Tue, 12 Oct 2021 00:46:31 -0700 Subject: [PATCH 30/83] Add test for duplicate handler prevention in the dispatcher For, eg, when a cog has been unloaded but not unregistered it's handlers --- tests/modmail/test_dispatcher.py | 34 ++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/modmail/test_dispatcher.py b/tests/modmail/test_dispatcher.py index 8325fc80..d9c7b4a3 100644 --- a/tests/modmail/test_dispatcher.py +++ b/tests/modmail/test_dispatcher.py @@ -279,3 +279,37 @@ async def on_class_test(self, var1) -> None: await a.fire() assert called + + +@pytest.mark.asyncio +async def test_class_dispatch_latemethod_deregister() -> None: + """Ensure lingering handlers are not kept beyond cog reload.""" + calls = 0 + + class A: + dispatcher = Dispatcher("class_test") + + def __init__(self): + self.dispatcher.register_events("class_test") + self.dispatcher.register("class_test", self.on_class_test) + + async def fire(self) -> None: + await self.dispatcher.dispatch("class_test") + + async def on_class_test(self) -> None: + nonlocal calls + calls += 1 + + a = A() + + await a.fire() + + assert calls == 1 + + del a + + a = A() + + await a.fire() + + assert calls == 2 From 8b0136dd932599a5c94019cf856e1c2af25b4bf3 Mon Sep 17 00:00:00 2001 From: Bast Date: Tue, 12 Oct 2021 00:47:50 -0700 Subject: [PATCH 31/83] Add dispatcher class method deactivation and add automated deactivation to ModmailCog --- modmail/dispatcher.py | 33 +++++++++++++++++++++++++++ modmail/utils/cogs.py | 4 ++++ tests/modmail/test_dispatcher.py | 38 ++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+) diff --git a/modmail/dispatcher.py b/modmail/dispatcher.py index 7b87cf4e..5de7d721 100644 --- a/modmail/dispatcher.py +++ b/modmail/dispatcher.py @@ -84,6 +84,39 @@ def activate(self, instance: object) -> None: self._register_handler(event_name, priority, value) self.pending_handlers[underlying_function].clear() + def deactivate(self, instance: object) -> None: + """ + Unregister all bound method handlers on a given class instance. + + Should be called during __del__. + """ + unregisterables = set() + for attr in dir(instance): + value = getattr(instance, attr) + if not callable(value): + continue + + # Bound methods have __func__, which returns the actual function + # we use that to determine which method was actually registered. + if not hasattr(value, "__func__"): + continue + + underlying_function = value.__func__ + + if underlying_function in self.pending_handlers: + # Was never registered + continue + + unregisterables.add(value) + + for event_name in self.handlers: + for unregisterable in unregisterables.intersection(self.handlers[event_name]): + self._remove_handler(unregisterable, event_name, False) + + for event_name in self.blocking_handlers: + for unregisterable in unregisterables.intersection(self.blocking_handlers[event_name]): + self._remove_handler(unregisterable, event_name, True) + def _register_handler( self, event_name: Optional[str], diff --git a/modmail/utils/cogs.py b/modmail/utils/cogs.py index 1a5832c8..b64be289 100644 --- a/modmail/utils/cogs.py +++ b/modmail/utils/cogs.py @@ -57,3 +57,7 @@ class ModmailCog(commands.Cog): def __init__(self, bot: "modmail.bot.ModmailBot"): self.dispatcher = bot.dispatcher self.dispatcher.activate(self) + + def cog_unload(self) -> None: + """Ensure dispatched class methods are unloaded when the cog is unloaded.""" + self.dispatcher.deactivate(self) diff --git a/tests/modmail/test_dispatcher.py b/tests/modmail/test_dispatcher.py index d9c7b4a3..1031c68c 100644 --- a/tests/modmail/test_dispatcher.py +++ b/tests/modmail/test_dispatcher.py @@ -313,3 +313,41 @@ async def on_class_test(self) -> None: await a.fire() assert calls == 2 + + +@pytest.mark.asyncio +async def test_class_dispatch_classmethod_deregister() -> None: + """Ensure deletion of a class instance unregisters it's class method handlers.""" + calls = 0 + + class A: + dispatcher = Dispatcher("class_test") + + def __init__(self): + self.dispatcher.register_events("class_test_deactivate") + self.dispatcher.activate(self) + + async def fire(self) -> None: + await self.dispatcher.dispatch("class_test_deactivate") + + @dispatcher.register() + async def on_class_test_deactivate(self) -> None: + nonlocal calls + calls += 1 + + def __del__(self): + self.dispatcher.deactivate(self) + + a = A() + + await a.fire() + + assert calls == 1 + + del a + + a = A() + + await a.fire() + + assert calls == 2 From 6b229f02e83998fcfee7b240243dfbcf98b59624 Mon Sep 17 00:00:00 2001 From: Bast Date: Tue, 12 Oct 2021 01:24:10 -0700 Subject: [PATCH 32/83] Make import more specific in ModmailCog --- modmail/utils/cogs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modmail/utils/cogs.py b/modmail/utils/cogs.py index b64be289..2b39e0b7 100644 --- a/modmail/utils/cogs.py +++ b/modmail/utils/cogs.py @@ -3,7 +3,7 @@ from discord.ext import commands -import modmail +import modmail.bot class BitwiseAutoEnum(IntEnum): From 4bdab4dde3454ce00aecbd14b403eebfe27de840 Mon Sep 17 00:00:00 2001 From: Bast Date: Tue, 12 Oct 2021 01:25:12 -0700 Subject: [PATCH 33/83] Improve changelog line --- docs/changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog.md b/docs/changelog.md index 54da19dc..044c2bfd 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -22,7 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Automatic docker image creation: `ghcr.io/discord-modmail/modmail` (#19) - Dockerfile support for all supported hosting providers. (#58) - Errors no longer happen silently and notify the user when they make a mistake. (#77) -- Added Dispatcher system, although it is not hooked in yet. (#71) +- Added Dispatcher system, although it is not hooked into important features like thread creation yet. (#71) ### Changed From 52bf9c6aeafcfb6d91ec3edaf1dbbf28821901a2 Mon Sep 17 00:00:00 2001 From: Bast Date: Tue, 12 Oct 2021 01:38:51 -0700 Subject: [PATCH 34/83] Move dispatcher changelog back to "changes" and not under a released section --- docs/changelog.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/changelog.md b/docs/changelog.md index 044c2bfd..a53544f5 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Embedified the meta commands so they have a nicer UI (#78) +### Added +- Added Dispatcher system, although it is not hooked into important features like thread creation yet. (#71) ## [0.2.0] - 2021-09-29 @@ -22,7 +24,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Automatic docker image creation: `ghcr.io/discord-modmail/modmail` (#19) - Dockerfile support for all supported hosting providers. (#58) - Errors no longer happen silently and notify the user when they make a mistake. (#77) -- Added Dispatcher system, although it is not hooked into important features like thread creation yet. (#71) ### Changed From 75c3fabae4cab29d1bba127a65a1c69b5f539927 Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Wed, 13 Oct 2021 10:04:14 +0530 Subject: [PATCH 35/83] Create dependabot.yml --- .github/dependabot.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..5e2009ae --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,11 @@ +version: 2 + +updates: +- package-ecosystem: pip + directory: "/" + schedule: + interval: daily + time: "07:00" + open-pull-requests-limit: 10 + commit-message: + prefix: "chore(deps): " From 042f5b720c904488c6921d6cdfc558c479786e8d Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 29 Aug 2021 23:24:48 -0400 Subject: [PATCH 36/83] tests: stop skipping passing tests Signed-off-by: onerandomusername --- tests/test_logs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_logs.py b/tests/test_logs.py index c97957fe..b781abd1 100644 --- a/tests/test_logs.py +++ b/tests/test_logs.py @@ -45,7 +45,6 @@ def test_notice_level(log): @pytest.mark.dependency(depends=["create_logger"]) -@pytest.mark.skip() def test_trace_level(log): """Test trace logging level prints a trace response.""" trace_test_phrase = "Getting in the weeds" From d64f03370a446b3445dceddd1a5ac5cbd7ae9cbd Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Mon, 30 Aug 2021 00:16:42 -0400 Subject: [PATCH 37/83] tests: test extension and plugin converters Signed-off-by: onerandomusername --- tests/modmail/extensions/__init__.py | 0 .../extensions/test_extension_manager.py | 29 +++++++++++++++++++ .../modmail/extensions/test_plugin_manager.py | 29 +++++++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 tests/modmail/extensions/__init__.py create mode 100644 tests/modmail/extensions/test_extension_manager.py create mode 100644 tests/modmail/extensions/test_plugin_manager.py diff --git a/tests/modmail/extensions/__init__.py b/tests/modmail/extensions/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/modmail/extensions/test_extension_manager.py b/tests/modmail/extensions/test_extension_manager.py new file mode 100644 index 00000000..c217e6d2 --- /dev/null +++ b/tests/modmail/extensions/test_extension_manager.py @@ -0,0 +1,29 @@ +from copy import copy + +import pytest + +from modmail.extensions.extension_manager import ExtensionConverter +from modmail.utils.extensions import EXTENSIONS, walk_extensions + + +# load EXTENSIONS +EXTENSIONS = copy(EXTENSIONS) +EXTENSIONS.update(walk_extensions()) + + +class TestExtensionConverter: + """Test the extension converter converts extensions properly.""" + + @pytest.fixture(scope="class", name="converter") + def converter(self) -> ExtensionConverter: + """Fixture method for a ExtensionConverter object.""" + return ExtensionConverter() + + @pytest.mark.asyncio + @pytest.mark.parametrize("extension", [e.rsplit(".", 1)[-1] for e in EXTENSIONS.keys()]) + async def test_conversion_success(self, extension: str, converter: ExtensionConverter) -> None: + """Test all extensions in the list are properly converted.""" + converter.source_list = EXTENSIONS + converted = await converter.convert(None, extension) + + assert converted.endswith(extension) diff --git a/tests/modmail/extensions/test_plugin_manager.py b/tests/modmail/extensions/test_plugin_manager.py new file mode 100644 index 00000000..4e10a41c --- /dev/null +++ b/tests/modmail/extensions/test_plugin_manager.py @@ -0,0 +1,29 @@ +from copy import copy + +import pytest + +from modmail.extensions.plugin_manager import PluginConverter +from modmail.utils.plugins import PLUGINS, walk_plugins + + +# load EXTENSIONS +PLUGINS = copy(PLUGINS) +PLUGINS.update(walk_plugins()) + + +class TestExtensionConverter: + """Test the extension converter converts extensions properly.""" + + @pytest.fixture(scope="class", name="converter") + def converter(self) -> PluginConverter: + """Fixture method for a ExtensionConverter object.""" + return PluginConverter() + + @pytest.mark.asyncio + @pytest.mark.parametrize("extension", [e.rsplit(".", 1)[-1] for e in PLUGINS.keys()]) + async def test_conversion_success(self, extension: str, converter: PluginConverter) -> None: + """Test all extensions in the list are properly converted.""" + converter.source_list = PLUGINS + converted = await converter.convert(None, extension) + + assert converted.endswith(extension) From 945475e794ca151aa108c9865d8c9f2f1e64410a Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Mon, 30 Aug 2021 00:17:29 -0400 Subject: [PATCH 38/83] nit: annotate as many variables in tests as possible Signed-off-by: onerandomusername --- tests/conftest.py | 2 +- tests/modmail/utils/test_embeds.py | 9 ++++----- tests/test_bot.py | 7 +++---- tests/test_logs.py | 6 +++--- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5cbb797c..759b9108 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,6 @@ import pytest -def pytest_report_header(config): +def pytest_report_header(config) -> str: """Pytest headers.""" return "package: modmail" diff --git a/tests/modmail/utils/test_embeds.py b/tests/modmail/utils/test_embeds.py index aeb256ed..9de5a069 100644 --- a/tests/modmail/utils/test_embeds.py +++ b/tests/modmail/utils/test_embeds.py @@ -1,12 +1,11 @@ import discord import pytest -from discord import Colour from modmail.utils.embeds import patch_embed @pytest.mark.dependency(name="patch_embed") -def test_patch_embed(): +def test_patch_embed() -> None: """Ensure that the function changes init only after the patch is called.""" from modmail.utils.embeds import __init__ as init from modmail.utils.embeds import original_init @@ -17,7 +16,7 @@ def test_patch_embed(): @pytest.mark.dependency(depends_on="patch_embed") -def test_create_embed(): +def test_create_embed() -> None: """Test creating an embed with patched parameters works properly.""" title = "Test title" description = "Test description" @@ -49,14 +48,14 @@ def test_create_embed(): @pytest.mark.dependency(depends_on="patch_embed") -def test_create_embed_with_extra_params(): +def test_create_embed_with_extra_params() -> None: """Test creating an embed with extra parameters errors properly.""" with pytest.raises(TypeError, match="ooga_booga"): discord.Embed("hello", ooga_booga=3) @pytest.mark.dependency(depends_on="patch_embed") -def test_create_embed_with_description_and_content(): +def test_create_embed_with_description_and_content() -> None: """ Create an embed while providing both description and content parameters. diff --git a/tests/test_bot.py b/tests/test_bot.py index cd220cac..22059ef5 100644 --- a/tests/test_bot.py +++ b/tests/test_bot.py @@ -4,7 +4,6 @@ - import module - create a bot object """ -import asyncio import pytest @@ -13,7 +12,7 @@ @pytest.mark.dependency(name="create_bot") @pytest.mark.asyncio -async def test_bot_creation(): +async def test_bot_creation() -> None: """Ensure we can make a ModmailBot instance.""" bot = ModmailBot() # cleanup @@ -33,7 +32,7 @@ def bot() -> ModmailBot: @pytest.mark.dependency(depends=["create_bot"]) @pytest.mark.asyncio -async def test_bot_close(bot): +async def test_bot_close(bot: ModmailBot) -> None: """Ensure bot closes without error.""" import contextlib import io @@ -46,6 +45,6 @@ async def test_bot_close(bot): @pytest.mark.dependency(depends=["create_bot"]) -def test_bot_main(): +def test_bot_main() -> None: """Import modmail.__main__.""" from modmail.__main__ import main diff --git a/tests/test_logs.py b/tests/test_logs.py index b781abd1..97fffaf1 100644 --- a/tests/test_logs.py +++ b/tests/test_logs.py @@ -13,7 +13,7 @@ @pytest.mark.dependency(name="create_logger") -def test_create_logging(): +def test_create_logging() -> None: """Modmail logging is importable and sets root logger correctly.""" log = logging.getLogger(__name__) assert isinstance(log, ModmailLogger) @@ -31,7 +31,7 @@ def log() -> ModmailLogger: @pytest.mark.dependency(depends=["create_logger"]) -def test_notice_level(log): +def test_notice_level(log: ModmailLogger) -> None: """Test notice logging level prints a notice response.""" notice_test_phrase = "Kinda important info" stdout = io.StringIO() @@ -45,7 +45,7 @@ def test_notice_level(log): @pytest.mark.dependency(depends=["create_logger"]) -def test_trace_level(log): +def test_trace_level(log: ModmailLogger) -> None: """Test trace logging level prints a trace response.""" trace_test_phrase = "Getting in the weeds" stdout = io.StringIO() From 5b11fb485ac0d9a3571ee9157913435fcf3a1e2f Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Mon, 30 Aug 2021 01:08:18 -0400 Subject: [PATCH 39/83] minor: don't use the global ext/plug lists Signed-off-by: onerandomusername --- tests/modmail/extensions/test_extension_manager.py | 11 +++++++---- tests/modmail/extensions/test_plugin_manager.py | 11 +++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/tests/modmail/extensions/test_extension_manager.py b/tests/modmail/extensions/test_extension_manager.py index c217e6d2..717a6b01 100644 --- a/tests/modmail/extensions/test_extension_manager.py +++ b/tests/modmail/extensions/test_extension_manager.py @@ -3,27 +3,30 @@ import pytest from modmail.extensions.extension_manager import ExtensionConverter -from modmail.utils.extensions import EXTENSIONS, walk_extensions +from modmail.utils.extensions import EXTENSIONS as GLOBAL_EXTENSIONS +from modmail.utils.extensions import walk_extensions # load EXTENSIONS -EXTENSIONS = copy(EXTENSIONS) +EXTENSIONS = copy(GLOBAL_EXTENSIONS) EXTENSIONS.update(walk_extensions()) class TestExtensionConverter: """Test the extension converter converts extensions properly.""" + all_extensions = {x: y for x, y in walk_extensions()} + @pytest.fixture(scope="class", name="converter") def converter(self) -> ExtensionConverter: """Fixture method for a ExtensionConverter object.""" return ExtensionConverter() @pytest.mark.asyncio - @pytest.mark.parametrize("extension", [e.rsplit(".", 1)[-1] for e in EXTENSIONS.keys()]) + @pytest.mark.parametrize("extension", [e.rsplit(".", 1)[-1] for e in all_extensions.keys()]) async def test_conversion_success(self, extension: str, converter: ExtensionConverter) -> None: """Test all extensions in the list are properly converted.""" - converter.source_list = EXTENSIONS + converter.source_list = self.all_extensions converted = await converter.convert(None, extension) assert converted.endswith(extension) diff --git a/tests/modmail/extensions/test_plugin_manager.py b/tests/modmail/extensions/test_plugin_manager.py index 4e10a41c..9a6bdd86 100644 --- a/tests/modmail/extensions/test_plugin_manager.py +++ b/tests/modmail/extensions/test_plugin_manager.py @@ -3,27 +3,30 @@ import pytest from modmail.extensions.plugin_manager import PluginConverter -from modmail.utils.plugins import PLUGINS, walk_plugins +from modmail.utils.plugins import PLUGINS as GLOBAL_PLUGINS +from modmail.utils.plugins import walk_plugins # load EXTENSIONS -PLUGINS = copy(PLUGINS) +PLUGINS = copy(GLOBAL_PLUGINS) PLUGINS.update(walk_plugins()) class TestExtensionConverter: """Test the extension converter converts extensions properly.""" + all_plugins = {x: y for x, y in walk_plugins()} + @pytest.fixture(scope="class", name="converter") def converter(self) -> PluginConverter: """Fixture method for a ExtensionConverter object.""" return PluginConverter() @pytest.mark.asyncio - @pytest.mark.parametrize("extension", [e.rsplit(".", 1)[-1] for e in PLUGINS.keys()]) + @pytest.mark.parametrize("extension", [e.rsplit(".", 1)[-1] for e in all_plugins.keys()]) async def test_conversion_success(self, extension: str, converter: PluginConverter) -> None: """Test all extensions in the list are properly converted.""" - converter.source_list = PLUGINS + converter.source_list = self.all_plugins converted = await converter.convert(None, extension) assert converted.endswith(extension) From a11bf2985b789dc27cab9a14d71df9387ad741cb Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 26 Sep 2021 00:30:24 -0400 Subject: [PATCH 40/83] fix: rename extensions to plugin in Plugins test Signed-off-by: onerandomusername --- tests/modmail/extensions/test_plugin_manager.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/modmail/extensions/test_plugin_manager.py b/tests/modmail/extensions/test_plugin_manager.py index 9a6bdd86..eddae16d 100644 --- a/tests/modmail/extensions/test_plugin_manager.py +++ b/tests/modmail/extensions/test_plugin_manager.py @@ -12,21 +12,21 @@ PLUGINS.update(walk_plugins()) -class TestExtensionConverter: +class TestPluginConverter: """Test the extension converter converts extensions properly.""" all_plugins = {x: y for x, y in walk_plugins()} @pytest.fixture(scope="class", name="converter") def converter(self) -> PluginConverter: - """Fixture method for a ExtensionConverter object.""" + """Fixture method for a PluginConverter object.""" return PluginConverter() @pytest.mark.asyncio - @pytest.mark.parametrize("extension", [e.rsplit(".", 1)[-1] for e in all_plugins.keys()]) - async def test_conversion_success(self, extension: str, converter: PluginConverter) -> None: - """Test all extensions in the list are properly converted.""" + @pytest.mark.parametrize("plugin", [e.rsplit(".", 1)[-1] for e in all_plugins.keys()]) + async def test_conversion_success(self, plugin: str, converter: PluginConverter) -> None: + """Test all plugins in the list are properly converted.""" converter.source_list = self.all_plugins - converted = await converter.convert(None, extension) + converted = await converter.convert(None, plugin) - assert converted.endswith(extension) + assert converted.endswith(plugin) From b4d8775edc3a97944b76e22cdd4af903846a0ca2 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Fri, 15 Oct 2021 23:04:41 -0400 Subject: [PATCH 41/83] tests(mocks): copy pydis/bot mock classes on top of pydis changes, adds discord.Thread mock object and fixes some docstrings. --- modmail/bot.py | 16 +- tests/mocks.py | 657 ++++++++++++++++++++++++++++++++++++++++++++ tests/test_mocks.py | 401 +++++++++++++++++++++++++++ 3 files changed, 1067 insertions(+), 7 deletions(-) create mode 100644 tests/mocks.py create mode 100644 tests/test_mocks.py diff --git a/modmail/bot.py b/modmail/bot.py index e62f1bda..93447437 100644 --- a/modmail/bot.py +++ b/modmail/bot.py @@ -49,14 +49,16 @@ def __init__(self, **kwargs): # allow only user mentions by default. # ! NOTE: This may change in the future to allow roles as well allowed_mentions = AllowedMentions(everyone=False, users=True, roles=False, replied_user=True) + # override passed kwargs if they are None + kwargs["case_insensitive"] = kwargs.get("case_insensitive", True) + # do not let the description be overridden. + kwargs["description"] = "Modmail bot by discord-modmail." + kwargs["status"] = kwargs.get("status", status) + kwargs["activity"] = kwargs.get("activity", activity) + kwargs["allowed_mentions"] = kwargs.get("allowed_mentions", allowed_mentions) + kwargs["command_prefix"] = kwargs.get("command_prefix", prefix) + kwargs["intents"] = kwargs.get("intents", REQUIRED_INTENTS) super().__init__( - case_insensitive=True, - description="Modmail bot by discord-modmail.", - status=status, - activity=activity, - allowed_mentions=allowed_mentions, - command_prefix=prefix, - intents=REQUIRED_INTENTS, **kwargs, ) diff --git a/tests/mocks.py b/tests/mocks.py new file mode 100644 index 00000000..3464d0eb --- /dev/null +++ b/tests/mocks.py @@ -0,0 +1,657 @@ +""" +Helper methods for testing. + +Slight modifications have been made to support our bot. + +Original Source: +https://github.com/python-discord/bot/blob/d183d03fa2939bebaac3da49646449fdd4d00e6c/tests/helpers.py# noqa: E501 + +MIT License + +Copyright (c) 2018 Python Discord + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. +""" +from __future__ import annotations + +import collections +import itertools +import logging +import unittest.mock +from asyncio import AbstractEventLoop +from typing import TYPE_CHECKING, Iterable, Optional + +import discord +import discord.mixins +from aiohttp import ClientSession +from discord.ext.commands import Context + +import modmail.bot + + +for logger in logging.Logger.manager.loggerDict.values(): + # Set all loggers to CRITICAL by default to prevent screen clutter during testing + + if not isinstance(logger, logging.Logger): + # There might be some logging.PlaceHolder objects in there + continue + + logger.setLevel(logging.CRITICAL) + + +class HashableMixin(discord.mixins.EqualityComparable): + """ + Mixin that provides similar hashing and equality functionality as discord.py's `Hashable` mixin. + + Given that most of our features need the created_at function to work, we are typically using + full fake discord ids, and so we still bitshift the id like Dpy does. + + However, given that the hash of 4>>22 and 5>>22 is the same, we check if the number is above 22. + If so, we hash it. This could result in some weird behavior with the hash of + 1<<22 + 1 equaling 1. + """ + + if TYPE_CHECKING: + id: int + + def __hash__(self): + if self.id < 1 << 22: + return self.id + else: + return self.id >> 22 + + +class ColourMixin: + """A mixin for Mocks that provides the aliasing of (accent_)color->(accent_)colour like discord.py.""" + + @property + def color(self) -> discord.Colour: + """Alias of colour.""" + return self.colour + + @color.setter + def color(self, color: discord.Colour) -> None: + self.colour = color + + @property + def accent_color(self) -> discord.Colour: + """Alias of accent_colour.""" + return self.accent_colour + + @accent_color.setter + def accent_color(self, color: discord.Colour) -> None: + self.accent_colour = color + + +class CustomMockMixin: + """ + Provides common functionality for our custom Mock types. + + The `_get_child_mock` method automatically returns an AsyncMock for coroutine methods of the mock + object. As discord.py also uses synchronous methods that nonetheless return coroutine objects, the + class attribute `additional_spec_asyncs` can be overwritten with an iterable containing additional + attribute names that should also mocked with an AsyncMock instead of a regular MagicMock/Mock. The + class method `spec_set` can be overwritten with the object that should be uses as the specification + for the mock. + + Mock/MagicMock subclasses that use this mixin only need to define `__init__` method if they need to + implement custom behavior. + """ + + child_mock_type = unittest.mock.MagicMock + discord_id = itertools.count(0) + spec_set = None + additional_spec_asyncs = None + + def __init__(self, **kwargs): + name = kwargs.pop( + "name", None + ) # `name` has special meaning for Mock classes, so we need to set it manually. + super().__init__(spec_set=self.spec_set, **kwargs) + + if self.additional_spec_asyncs: + self._spec_asyncs.extend(self.additional_spec_asyncs) + + if name: + self.name = name + + def _get_child_mock(self, **kw): + """ + Overwrite of the `_get_child_mock` method to stop the propagation of our custom mock classes. + + Mock objects automatically create children when you access an attribute or call a method on them. + By default, the class of these children is the type of the parent itself. + However, this would mean that the children created for our custom mock types would also be instances + of that custom mock type. This is not desirable, as attributes of, e.g., a `Bot` object are not + `Bot` objects themselves. The Python docs for `unittest.mock` hint that overwriting this method is the + best way to deal with that. + + This override will look for an attribute called `child_mock_type` and + use that as the type of the child mock. + """ + _new_name = kw.get("_new_name") + if _new_name in self.__dict__["_spec_asyncs"]: + return unittest.mock.AsyncMock(**kw) + + _type = type(self) + if issubclass(_type, unittest.mock.MagicMock) and _new_name in unittest.mock._async_method_magics: + # Any asynchronous magic becomes an AsyncMock + klass = unittest.mock.AsyncMock + else: + klass = self.child_mock_type + + if self._mock_sealed: + attribute = "." + kw["name"] if "name" in kw else "()" + mock_name = self._extract_mock_name() + attribute + raise AttributeError(mock_name) + + return klass(**kw) + + +# Create a guild instance to get a realistic Mock of `discord.Guild` +guild_data = { + "id": 1, + "name": "guild", + "region": "Europe", + "verification_level": 2, + "default_notications": 1, + "afk_timeout": 100, + "icon": "icon.png", + "banner": "banner.png", + "mfa_level": 1, + "splash": "splash.png", + "system_channel_id": 464033278631084042, + "description": "mocking is fun", + "max_presences": 10_000, + "max_members": 100_000, + "preferred_locale": "UTC", + "owner_id": 1, + "afk_channel_id": 464033278631084042, +} +guild_instance = discord.Guild(data=guild_data, state=unittest.mock.MagicMock()) + + +class MockGuild(CustomMockMixin, unittest.mock.Mock, HashableMixin): + """ + A `Mock` subclass to mock `discord.Guild` objects. + + A MockGuild instance will follow the specifications of a `discord.Guild` instance. This means + that if the code you're testing tries to access an attribute or method that normally does not + exist for a `discord.Guild` object this will raise an `AttributeError`. This is to make sure our + tests fail if the code we're testing uses a `discord.Guild` object in the wrong way. + + One restriction of that is that if the code tries to access an attribute that normally does not + exist for `discord.Guild` instance but was added dynamically, this will raise an exception with + the mocked object. To get around that, you can set the non-standard attribute explicitly for the + instance of `MockGuild`: + + >>> guild = MockGuild() + >>> guild.attribute_that_normally_does_not_exist = unittest.mock.MagicMock() + + In addition to attribute simulation, mocked guild object will pass an `isinstance` check against + `discord.Guild`: + + >>> guild = MockGuild() + >>> isinstance(guild, discord.Guild) + True + + For more info, see the `Mocking` section in `tests/README.md`. + """ + + spec_set = guild_instance + + def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None: + default_kwargs = {"id": next(self.discord_id), "members": []} + super().__init__(**collections.ChainMap(kwargs, default_kwargs)) + + self.roles = [MockRole(name="@everyone", position=1, id=0)] + if roles: + self.roles.extend(roles) + + +# Create a Role instance to get a realistic Mock of `discord.Role` +role_data = {"name": "role", "id": 1} +role_instance = discord.Role(guild=guild_instance, state=unittest.mock.MagicMock(), data=role_data) + + +class MockRole(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): + """ + A Mock subclass to mock `discord.Role` objects. + + Instances of this class will follow the specifications of `discord.Role` instances. For more + information, see the `MockGuild` docstring. + """ + + spec_set = role_instance + + def __init__(self, **kwargs) -> None: + default_kwargs = { + "id": next(self.discord_id), + "name": "role", + "position": 1, + "colour": discord.Colour(0xDEADBF), + "permissions": discord.Permissions(), + } + super().__init__(**collections.ChainMap(kwargs, default_kwargs)) + + if isinstance(self.colour, int): + self.colour = discord.Colour(self.colour) + + if isinstance(self.permissions, int): + self.permissions = discord.Permissions(self.permissions) + + if "mention" not in kwargs: + self.mention = f"&{self.name}" + + def __lt__(self, other): + """Simplified position-based comparisons similar to those of `discord.Role`.""" + return self.position < other.position + + def __ge__(self, other): + """Simplified position-based comparisons similar to those of `discord.Role`.""" + return self.position >= other.position + + +# Create a Member instance to get a realistic Mock of `discord.Member` +member_data = {"user": "lemon", "roles": [1]} +state_mock = unittest.mock.MagicMock() +member_instance = discord.Member(data=member_data, guild=guild_instance, state=state_mock) + + +class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): + """ + A Mock subclass to mock Member objects. + + Instances of this class will follow the specifications of `discord.Member` instances. For more + information, see the `MockGuild` docstring. + """ + + spec_set = member_instance + + def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None: + default_kwargs = {"name": "member", "id": next(self.discord_id), "bot": False, "pending": False} + super().__init__(**collections.ChainMap(kwargs, default_kwargs)) + + self.roles = [MockRole(name="@everyone", position=1, id=0)] + if roles: + self.roles.extend(roles) + self.top_role = max(self.roles) + + if "mention" not in kwargs: + self.mention = f"@{self.name}" + + +# Create a User instance to get a realistic Mock of `discord.User` +_user_data_mock = collections.defaultdict(unittest.mock.MagicMock, {"accent_color": 0}) +user_instance = discord.User( + data=unittest.mock.MagicMock(get=unittest.mock.Mock(side_effect=_user_data_mock.get)), + state=unittest.mock.MagicMock(), +) + + +class MockUser(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): + """ + A Mock subclass to mock User objects. + + Instances of this class will follow the specifications of `discord.User` instances. For more + information, see the `MockGuild` docstring. + """ + + spec_set = user_instance + + def __init__(self, **kwargs) -> None: + default_kwargs = {"name": "user", "id": next(self.discord_id), "bot": False} + super().__init__(**collections.ChainMap(kwargs, default_kwargs)) + + if "mention" not in kwargs: + self.mention = f"@{self.name}" + + +def _get_mock_loop() -> unittest.mock.Mock: + """Return a mocked asyncio.AbstractEventLoop.""" + loop = unittest.mock.create_autospec(spec=AbstractEventLoop, spec_set=True) + + # Since calling `create_task` on our MockBot does not actually schedule the coroutine object + # as a task in the asyncio loop, this `side_effect` calls `close()` on the coroutine object + # to prevent "has not been awaited"-warnings. + def mock_create_task(coroutine, **kwargs): + coroutine.close() + return unittest.mock.Mock() + + loop.create_task.side_effect = mock_create_task + + return loop + + +class MockBot(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Bot objects. + + Instances of this class will follow the specifications of `discord.ext.commands.Bot` instances. + For more information, see the `MockGuild` docstring. + """ + + spec_set = modmail.bot.ModmailBot( + command_prefix=unittest.mock.MagicMock(), + loop=_get_mock_loop(), + ) + additional_spec_asyncs = ("wait_for", "redis_ready") + + def __init__(self, **kwargs) -> None: + super().__init__(**kwargs) + + self.loop = _get_mock_loop() + self.http_session = unittest.mock.create_autospec(spec=ClientSession, spec_set=True) + + +# Create a TextChannel instance to get a realistic MagicMock of `discord.TextChannel` +channel_data = { + "id": 1, + "type": "TextChannel", + "name": "channel", + "parent_id": 1234567890, + "topic": "topic", + "position": 1, + "nsfw": False, + "last_message_id": 1, +} +state = unittest.mock.MagicMock() +guild = unittest.mock.MagicMock() +text_channel_instance = discord.TextChannel(state=state, guild=guild, data=channel_data) + +channel_data["type"] = "VoiceChannel" +voice_channel_instance = discord.VoiceChannel(state=state, guild=guild, data=channel_data) + + +class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): + """ + A MagicMock subclass to mock TextChannel objects. + + Instances of this class will follow the specifications of `discord.TextChannel` instances. For + more information, see the `MockGuild` docstring. + """ + + spec_set = text_channel_instance + + def __init__(self, **kwargs) -> None: + default_kwargs = {"id": next(self.discord_id), "name": "channel", "guild": MockGuild()} + super().__init__(**collections.ChainMap(kwargs, default_kwargs)) + + if "mention" not in kwargs: + self.mention = f"#{self.name}" + + +class MockVoiceChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): + """ + A MagicMock subclass to mock VoiceChannel objects. + + Instances of this class will follow the specifications of `discord.VoiceChannel` instances. For + more information, see the `MockGuild` docstring. + """ + + spec_set = voice_channel_instance + + def __init__(self, **kwargs) -> None: + default_kwargs = {"id": next(self.discord_id), "name": "channel", "guild": MockGuild()} + super().__init__(**collections.ChainMap(kwargs, default_kwargs)) + + if "mention" not in kwargs: + self.mention = f"#{self.name}" + + +# Create data for the DMChannel instance +state = unittest.mock.MagicMock() +me = unittest.mock.MagicMock() +dm_channel_data = {"id": 1, "recipients": [unittest.mock.MagicMock()]} +dm_channel_instance = discord.DMChannel(me=me, state=state, data=dm_channel_data) + + +class MockDMChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): + """ + A MagicMock subclass to mock DMChannel objects. + + Instances of this class will follow the specifications of `discord.DMChannel` instances. For + more information, see the `MockGuild` docstring. + """ + + spec_set = dm_channel_instance + + def __init__(self, **kwargs) -> None: + default_kwargs = {"id": next(self.discord_id), "recipient": MockUser(), "me": MockUser()} + super().__init__(**collections.ChainMap(kwargs, default_kwargs)) + + +# Create CategoryChannel instance to get a realistic MagicMock of `discord.CategoryChannel` +category_channel_data = { + "id": 1, + "type": discord.ChannelType.category, + "name": "category", + "position": 1, +} + +state = unittest.mock.MagicMock() +guild = unittest.mock.MagicMock() +category_channel_instance = discord.CategoryChannel(state=state, guild=guild, data=category_channel_data) + + +class MockCategoryChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): + """ + A MagicMock subclass to mock CategoryChannel objects. + + Instances of this class will follow the specifications of `discord.CategoryChannel` instances. For + more information, see the `MockGuild` docstring. + """ + + spec_set = category_channel_instance + + def __init__(self, **kwargs) -> None: + default_kwargs = {"id": next(self.discord_id)} + super().__init__(**collections.ChainMap(default_kwargs, kwargs)) + + +# Create a thread instance to get a realistic MagicMock of `discord.Thread` +thread_metadata = { + "archived": False, + "archiver_id": None, + "auto_archive_duration": 1440, + "archive_timestamp": "2021-10-17T20:35:48.058121+00:00", +} +thread_data = { + "id": "898070829617799179", + "parent_id": "898070393619902544", + "owner_id": "717983911824588862", + "name": "user-0005", + "type": discord.ChannelType.public_thread, + "last_message_id": None, + "message_count": 1, + "member_count": 2, + "thread_metadata": thread_metadata, +} + +state = unittest.mock.MagicMock() +guild = unittest.mock.MagicMock() +thread_instance = discord.Thread(state=state, guild=guild, data=thread_data) + + +class MockThread(CustomMockMixin, unittest.mock.Mock, HashableMixin): + """ + A MagicMock subclass to mock Thread objects. + + Instances of this class will follow the specifications of `discord.Thread` instances. For + more information, see the `MockGuild` docstring. + """ + + spec_set = thread_instance + + def __init__(self, **kwargs) -> None: + default_kwargs = {"id": next(self.discord_id)} + super().__init__(**collections.ChainMap(default_kwargs, kwargs)) + + +# Create a Message instance to get a realistic MagicMock of `discord.Message` +message_data = { + "id": 1, + "webhook_id": 898069816622067752, + "attachments": [], + "embeds": [], + "application": "Discord Modmail", + "activity": "mocking", + "channel": unittest.mock.MagicMock(), + "edited_timestamp": "2019-10-14T15:33:48+00:00", + "type": "message", + "pinned": False, + "mention_everyone": False, + "tts": None, + "content": "content", + "nonce": None, +} +state = unittest.mock.MagicMock() +channel = unittest.mock.MagicMock() +message_instance = discord.Message(state=state, channel=channel, data=message_data) + + +# Create a Context instance to get a realistic MagicMock of `discord.ext.commands.Context` +context_instance = Context(message=unittest.mock.MagicMock(), prefix="$", bot=MockBot(), view=None) +context_instance.invoked_from_error_handler = None + + +class MockContext(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Context objects. + + Instances of this class will follow the specifications of `discord.ext.commands.Context` + instances. For more information, see the `MockGuild` docstring. + """ + + spec_set = context_instance + + def __init__(self, **kwargs) -> None: + super().__init__(**kwargs) + self.me = kwargs.get("me", MockMember()) + self.bot = kwargs.get("bot", MockBot()) + self.guild = kwargs.get("guild", MockGuild()) + self.author = kwargs.get("author", MockMember()) + self.channel = kwargs.get("channel", MockTextChannel()) + self.message = kwargs.get("message", MockMessage()) + self.invoked_from_error_handler = kwargs.get("invoked_from_error_handler", False) + + +attachment_instance = discord.Attachment(data=unittest.mock.MagicMock(id=1), state=unittest.mock.MagicMock()) + + +class MockAttachment(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Attachment objects. + + Instances of this class will follow the specifications of `discord.Attachment` instances. For + more information, see the `MockGuild` docstring. + """ + + spec_set = attachment_instance + + +class MockMessage(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Message objects. + + Instances of this class will follow the specifications of `discord.Message` instances. For more + information, see the `MockGuild` docstring. + """ + + spec_set = message_instance + + def __init__(self, **kwargs) -> None: + default_kwargs = {"attachments": []} + super().__init__(**collections.ChainMap(kwargs, default_kwargs)) + self.author = kwargs.get("author", MockMember()) + self.channel = kwargs.get("channel", MockTextChannel()) + + +emoji_data = {"require_colons": True, "managed": True, "id": 1, "name": "hyperlemon"} +emoji_instance = discord.Emoji(guild=MockGuild(), state=unittest.mock.MagicMock(), data=emoji_data) + + +class MockEmoji(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Emoji objects. + + Instances of this class will follow the specifications of `discord.Emoji` instances. For more + information, see the `MockGuild` docstring. + """ + + spec_set = emoji_instance + + def __init__(self, **kwargs) -> None: + super().__init__(**kwargs) + self.guild = kwargs.get("guild", MockGuild()) + + +partial_emoji_instance = discord.PartialEmoji(animated=False, name="guido") + + +class MockPartialEmoji(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock PartialEmoji objects. + + Instances of this class will follow the specifications of `discord.PartialEmoji` instances. For + more information, see the `MockGuild` docstring. + """ + + spec_set = partial_emoji_instance + + +reaction_instance = discord.Reaction(message=MockMessage(), data={"me": True}, emoji=MockEmoji()) + + +class MockReaction(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Reaction objects. + + Instances of this class will follow the specifications of `discord.Reaction` instances. For + more information, see the `MockGuild` docstring. + """ + + spec_set = reaction_instance + + def __init__(self, **kwargs) -> None: + _users = kwargs.pop("users", []) + super().__init__(**kwargs) + self.emoji = kwargs.get("emoji", MockEmoji()) + self.message = kwargs.get("message", MockMessage()) + + user_iterator = unittest.mock.AsyncMock() + user_iterator.__aiter__.return_value = _users + self.users.return_value = user_iterator + + self.__str__.return_value = str(self.emoji) + + +webhook_instance = discord.Webhook(data=unittest.mock.MagicMock(), session=unittest.mock.MagicMock()) + + +class MockAsyncWebhook(CustomMockMixin, unittest.mock.MagicMock): + """ + A MagicMock subclass to mock Webhook objects using an AsyncWebhookAdapter. + + Instances of this class will follow the specifications of `discord.Webhook` instances. For + more information, see the `MockGuild` docstring. + """ + + spec_set = webhook_instance + additional_spec_asyncs = ("send", "edit", "delete", "execute") diff --git a/tests/test_mocks.py b/tests/test_mocks.py new file mode 100644 index 00000000..b50c6c98 --- /dev/null +++ b/tests/test_mocks.py @@ -0,0 +1,401 @@ +""" +Meta test file for tests/mocks.py. + +Original Source: +https://github.com/python-discord/bot/blob/d183d03fa2939bebaac3da49646449fdd4d00e6c/tests/test_helpers.py # noqa: E501 + +MIT License + +Copyright (c) 2018 Python Discord + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. +""" + +import asyncio +import unittest +import unittest.mock + +import discord + +from tests import mocks as test_mocks + + +class DiscordMocksTests(unittest.TestCase): + """Tests for our specialized discord.py mocks.""" + + def test_mock_role_default_initialization(self): + """Test if the default initialization of MockRole results in the correct object.""" + role = test_mocks.MockRole() + + # The `spec` argument makes sure `isistance` checks with `discord.Role` pass + self.assertIsInstance(role, discord.Role) + + self.assertEqual(role.name, "role") + self.assertEqual(role.position, 1) + self.assertEqual(role.mention, "&role") + + def test_mock_role_alternative_arguments(self): + """Test if MockRole initializes with the arguments provided.""" + role = test_mocks.MockRole( + name="Admins", + id=90210, + position=10, + ) + + self.assertEqual(role.name, "Admins") + self.assertEqual(role.id, 90210) + self.assertEqual(role.position, 10) + self.assertEqual(role.mention, "&Admins") + + def test_mock_role_accepts_dynamic_arguments(self): + """Test if MockRole accepts and sets abitrary keyword arguments.""" + role = test_mocks.MockRole( + guild="Dino Man", + hoist=True, + ) + + self.assertEqual(role.guild, "Dino Man") + self.assertTrue(role.hoist) + + def test_mock_role_uses_position_for_less_than_greater_than(self): + """Test if `<` and `>` comparisons for MockRole are based on its position attribute.""" + role_one = test_mocks.MockRole(position=1) + role_two = test_mocks.MockRole(position=2) + role_three = test_mocks.MockRole(position=3) + + self.assertLess(role_one, role_two) + self.assertLess(role_one, role_three) + self.assertLess(role_two, role_three) + self.assertGreater(role_three, role_two) + self.assertGreater(role_three, role_one) + self.assertGreater(role_two, role_one) + + def test_mock_member_default_initialization(self): + """Test if the default initialization of Mockmember results in the correct object.""" + member = test_mocks.MockMember() + + # The `spec` argument makes sure `isistance` checks with `discord.Member` pass + self.assertIsInstance(member, discord.Member) + + self.assertEqual(member.name, "member") + self.assertListEqual(member.roles, [test_mocks.MockRole(name="@everyone", position=1, id=0)]) + self.assertEqual(member.mention, "@member") + + def test_mock_member_alternative_arguments(self): + """Test if MockMember initializes with the arguments provided.""" + core_developer = test_mocks.MockRole(name="Core Developer", position=2) + member = test_mocks.MockMember(name="Mark", id=12345, roles=[core_developer]) + + self.assertEqual(member.name, "Mark") + self.assertEqual(member.id, 12345) + self.assertListEqual( + member.roles, [test_mocks.MockRole(name="@everyone", position=1, id=0), core_developer] + ) + self.assertEqual(member.mention, "@Mark") + + def test_mock_member_accepts_dynamic_arguments(self): + """Test if MockMember accepts and sets abitrary keyword arguments.""" + member = test_mocks.MockMember( + nick="Dino Man", + colour=discord.Colour.default(), + ) + + self.assertEqual(member.nick, "Dino Man") + self.assertEqual(member.colour, discord.Colour.default()) + + def test_mock_guild_default_initialization(self): + """Test if the default initialization of Mockguild results in the correct object.""" + guild = test_mocks.MockGuild() + + # The `spec` argument makes sure `isistance` checks with `discord.Guild` pass + self.assertIsInstance(guild, discord.Guild) + + self.assertListEqual(guild.roles, [test_mocks.MockRole(name="@everyone", position=1, id=0)]) + self.assertListEqual(guild.members, []) + + def test_mock_guild_alternative_arguments(self): + """Test if MockGuild initializes with the arguments provided.""" + core_developer = test_mocks.MockRole(name="Core Developer", position=2) + guild = test_mocks.MockGuild( + roles=[core_developer], + members=[test_mocks.MockMember(id=54321)], + ) + + self.assertListEqual( + guild.roles, [test_mocks.MockRole(name="@everyone", position=1, id=0), core_developer] + ) + self.assertListEqual(guild.members, [test_mocks.MockMember(id=54321)]) + + def test_mock_guild_accepts_dynamic_arguments(self): + """Test if MockGuild accepts and sets abitrary keyword arguments.""" + guild = test_mocks.MockGuild( + emojis=(":hyperjoseph:", ":pensive_ela:"), + premium_subscription_count=15, + ) + + self.assertTupleEqual(guild.emojis, (":hyperjoseph:", ":pensive_ela:")) + self.assertEqual(guild.premium_subscription_count, 15) + + def test_mock_bot_default_initialization(self): + """Tests if MockBot initializes with the correct values.""" + bot = test_mocks.MockBot() + + # The `spec` argument makes sure `isistance` checks with `discord.ext.commands.Bot` pass + self.assertIsInstance(bot, discord.ext.commands.Bot) + + def test_mock_context_default_initialization(self): + """Tests if MockContext initializes with the correct values.""" + context = test_mocks.MockContext() + + # The `spec` argument makes sure `isistance` checks with `discord.ext.commands.Context` pass + self.assertIsInstance(context, discord.ext.commands.Context) + + self.assertIsInstance(context.bot, test_mocks.MockBot) + self.assertIsInstance(context.guild, test_mocks.MockGuild) + self.assertIsInstance(context.author, test_mocks.MockMember) + + def test_mocks_allows_access_to_attributes_part_of_spec(self): + """Accessing attributes that are valid for the objects they mock should succeed.""" + mocks = ( + (test_mocks.MockGuild(), "name"), + (test_mocks.MockRole(), "hoist"), + (test_mocks.MockMember(), "display_name"), + (test_mocks.MockBot(), "user"), + (test_mocks.MockContext(), "invoked_with"), + (test_mocks.MockTextChannel(), "last_message"), + (test_mocks.MockMessage(), "mention_everyone"), + ) + + for mock, valid_attribute in mocks: + with self.subTest(mock=mock): + try: + getattr(mock, valid_attribute) + except AttributeError: + msg = f"accessing valid attribute `{valid_attribute}` raised an AttributeError" + self.fail(msg) + + @unittest.mock.patch(f"{__name__}.DiscordMocksTests.subTest") + @unittest.mock.patch(f"{__name__}.getattr") + def test_mock_allows_access_to_attributes_test(self, mock_getattr, mock_subtest): + """The valid attribute test should raise an AssertionError after an AttributeError.""" + mock_getattr.side_effect = AttributeError + + msg = "accessing valid attribute `name` raised an AttributeError" + with self.assertRaises(AssertionError, msg=msg): + self.test_mocks_allows_access_to_attributes_part_of_spec() + + def test_mocks_rejects_access_to_attributes_not_part_of_spec(self): + """Accessing attributes that are invalid for the objects they mock should fail.""" + mocks = ( + test_mocks.MockGuild(), + test_mocks.MockRole(), + test_mocks.MockMember(), + test_mocks.MockBot(), + test_mocks.MockContext(), + test_mocks.MockTextChannel(), + test_mocks.MockMessage(), + ) + + for mock in mocks: + with self.subTest(mock=mock): + with self.assertRaises(AttributeError): + mock.the_cake_is_a_lie + + def test_mocks_use_mention_when_provided_as_kwarg(self): + """The mock should use the passed `mention` instead of the default one if present.""" + test_cases = ( + (test_mocks.MockRole, "role mention"), + (test_mocks.MockMember, "member mention"), + (test_mocks.MockTextChannel, "channel mention"), + ) + + for mock_type, mention in test_cases: + with self.subTest(mock_type=mock_type, mention=mention): + mock = mock_type(mention=mention) + self.assertEqual(mock.mention, mention) + + def test_create_test_on_mock_bot_closes_passed_coroutine(self): + """`bot.loop.create_task` should close the passed coroutine object to prevent warnings.""" + + async def dementati(): + """Dummy coroutine for testing purposes.""" + + coroutine_object = dementati() + + bot = test_mocks.MockBot() + bot.loop.create_task(coroutine_object) + with self.assertRaises(RuntimeError, msg="cannot reuse already awaited coroutine"): + asyncio.run(coroutine_object) + + def test_user_mock_uses_explicitly_passed_mention_attribute(self): + """Ensure MockUser uses an explictly passed value for user.mention.""" + user = test_mocks.MockUser(mention="hello") + self.assertEqual(user.mention, "hello") + + +class MockObjectTests(unittest.TestCase): + """Tests the mock objects and mixins we've defined.""" + + @classmethod + def setUpClass(cls): + """Called by unittest before running the test methods.""" + cls.hashable_mocks = (test_mocks.MockRole, test_mocks.MockMember, test_mocks.MockGuild) + + def test_colour_mixin(self): + """Test if the ColourMixin adds aliasing of color -> colour for child classes.""" + + class MockHemlock(unittest.mock.MagicMock, test_mocks.ColourMixin): + pass + + hemlock = MockHemlock() + hemlock.color = 1 + self.assertEqual(hemlock.colour, 1) + self.assertEqual(hemlock.colour, hemlock.color) + + def test_hashable_mixin_hash_returns_id(self): + """Test the HashableMixing uses the id attribute for hashing.""" + + class MockScragly(unittest.mock.Mock, test_mocks.HashableMixin): + pass + + scragly = MockScragly() + scragly.id = 10 + self.assertEqual(hash(scragly), scragly.id) + + def test_hashable_mixin_hash_returns_id_bitshift(self): + """Test the HashableMixing uses the id attribute for hashing when above 1<<22.""" + + class MockScragly(unittest.mock.Mock, test_mocks.HashableMixin): + pass + + scragly = MockScragly() + scragly.id = 10 << 22 + self.assertEqual(hash(scragly), scragly.id >> 22) + + def test_hashable_mixin_uses_id_for_equality_comparison(self): + """Test the HashableMixing uses the id attribute for equal comparison.""" + + class MockScragly(test_mocks.HashableMixin): + pass + + scragly = MockScragly() + scragly.id = 10 + eevee = MockScragly() + eevee.id = 10 + python = MockScragly() + python.id = 20 + + self.assertTrue(scragly == eevee) + self.assertFalse(scragly == python) + + def test_hashable_mixin_uses_id_for_nonequality_comparison(self): + """Test if the HashableMixing uses the id attribute for non-equal comparison.""" + + class MockScragly(test_mocks.HashableMixin): + pass + + scragly = MockScragly() + scragly.id = 10 + eevee = MockScragly() + eevee.id = 10 + python = MockScragly() + python.id = 20 + + self.assertTrue(scragly != python) + self.assertFalse(scragly != eevee) + + def test_mock_class_with_hashable_mixin_uses_id_for_hashing(self): + """Test if the MagicMock subclasses that implement the HashableMixin use id for hash.""" + for mock in self.hashable_mocks: + with self.subTest(mock_class=mock): + instance = test_mocks.MockRole(id=100) + self.assertEqual(hash(instance), instance.id) + + def test_mock_class_with_hashable_mixin_uses_id_for_equality(self): + """Test if MagicMocks that implement the HashableMixin use id for equality comparisons.""" + for mock_class in self.hashable_mocks: + with self.subTest(mock_class=mock_class): + instance_one = mock_class() + instance_two = mock_class() + instance_three = mock_class() + + instance_one.id = 10 + instance_two.id = 10 + instance_three.id = 20 + + self.assertTrue(instance_one == instance_two) + self.assertFalse(instance_one == instance_three) + + def test_mock_class_with_hashable_mixin_uses_id_for_nonequality(self): + """Test if MagicMocks that implement HashableMixin use id for nonequality comparisons.""" + for mock_class in self.hashable_mocks: + with self.subTest(mock_class=mock_class): + instance_one = mock_class() + instance_two = mock_class() + instance_three = mock_class() + + instance_one.id = 10 + instance_two.id = 10 + instance_three.id = 20 + + self.assertFalse(instance_one != instance_two) + self.assertTrue(instance_one != instance_three) + + def test_custom_mock_mixin_accepts_mock_seal(self): + """The `CustomMockMixin` should support `unittest.mock.seal`.""" + + class MyMock(test_mocks.CustomMockMixin, unittest.mock.MagicMock): + + child_mock_type = unittest.mock.MagicMock + pass + + mock = MyMock() + unittest.mock.seal(mock) + with self.assertRaises(AttributeError, msg="MyMock.shirayuki"): + mock.shirayuki = "hello!" + + def test_spec_propagation_of_mock_subclasses(self): + """Test if the `spec` does not propagate to attributes of the mock object.""" + test_values = ( + (test_mocks.MockGuild, "region"), + (test_mocks.MockRole, "mentionable"), + (test_mocks.MockMember, "display_name"), + (test_mocks.MockBot, "owner_id"), + (test_mocks.MockContext, "command_failed"), + (test_mocks.MockMessage, "mention_everyone"), + (test_mocks.MockEmoji, "managed"), + (test_mocks.MockPartialEmoji, "url"), + (test_mocks.MockReaction, "me"), + ) + + for mock_type, valid_attribute in test_values: + with self.subTest(mock_type=mock_type, attribute=valid_attribute): + mock = mock_type() + self.assertTrue(isinstance(mock, mock_type)) + attribute = getattr(mock, valid_attribute) + self.assertTrue(isinstance(attribute, mock_type.child_mock_type)) + + def test_custom_mock_mixin_mocks_async_magic_methods_with_async_mock(self): + """The CustomMockMixin should mock async magic methods with an AsyncMock.""" + + class MyMock(test_mocks.CustomMockMixin, unittest.mock.MagicMock): + pass + + mock = MyMock() + self.assertIsInstance(mock.__aenter__, unittest.mock.AsyncMock) From 59d7aaebdaba8eb60ae4b9dffe51510e9baa0da4 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 17 Oct 2021 18:40:24 -0400 Subject: [PATCH 42/83] chore: use less "from x " imports --- tests/mocks.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/mocks.py b/tests/mocks.py index 3464d0eb..469dab4e 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -30,16 +30,16 @@ """ from __future__ import annotations +import asyncio import collections import itertools import logging import unittest.mock -from asyncio import AbstractEventLoop from typing import TYPE_CHECKING, Iterable, Optional +import aiohttp import discord import discord.mixins -from aiohttp import ClientSession from discord.ext.commands import Context import modmail.bot @@ -325,7 +325,7 @@ def __init__(self, **kwargs) -> None: def _get_mock_loop() -> unittest.mock.Mock: """Return a mocked asyncio.AbstractEventLoop.""" - loop = unittest.mock.create_autospec(spec=AbstractEventLoop, spec_set=True) + loop = unittest.mock.create_autospec(spec=asyncio.AbstractEventLoop, spec_set=True) # Since calling `create_task` on our MockBot does not actually schedule the coroutine object # as a task in the asyncio loop, this `side_effect` calls `close()` on the coroutine object @@ -357,7 +357,7 @@ def __init__(self, **kwargs) -> None: super().__init__(**kwargs) self.loop = _get_mock_loop() - self.http_session = unittest.mock.create_autospec(spec=ClientSession, spec_set=True) + self.http_session = unittest.mock.create_autospec(spec=aiohttp.ClientSession, spec_set=True) # Create a TextChannel instance to get a realistic MagicMock of `discord.TextChannel` From d7566e4ba5537b3d46facb06a455feca3b51511b Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Mon, 18 Oct 2021 00:19:21 -0400 Subject: [PATCH 43/83] chore: don't create a real bot instance as a fixture, create a mocked bot --- tests/test_bot.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/test_bot.py b/tests/test_bot.py index 22059ef5..1fe77535 100644 --- a/tests/test_bot.py +++ b/tests/test_bot.py @@ -8,6 +8,7 @@ import pytest from modmail.bot import ModmailBot +from tests import mocks @pytest.mark.dependency(name="create_bot") @@ -26,7 +27,7 @@ def bot() -> ModmailBot: ModmailBot instance. """ - bot: ModmailBot = ModmailBot() + bot: ModmailBot = mocks.MockBot() return bot @@ -42,9 +43,3 @@ async def test_bot_close(bot: ModmailBot) -> None: await bot.close() resp = stdout.getvalue() assert resp == "" - - -@pytest.mark.dependency(depends=["create_bot"]) -def test_bot_main() -> None: - """Import modmail.__main__.""" - from modmail.__main__ import main From cfe7dd04fb54c1594dc92da248817d5eee934874 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Mon, 18 Oct 2021 00:20:22 -0400 Subject: [PATCH 44/83] minor: fix small regex issue with error_handler --- modmail/extensions/utils/error_handler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py index 02b059f2..cedd8f1b 100644 --- a/modmail/extensions/utils/error_handler.py +++ b/modmail/extensions/utils/error_handler.py @@ -18,7 +18,7 @@ ERROR_COLOUR = discord.Colour.red() -ERROR_TITLE_REGEX = re.compile(r"(?<=[a-zA-Z])([A-Z])(?=[a-z])") +ERROR_TITLE_REGEX = re.compile(r"((?<=[a-z])[A-Z]|(?<=[a-zA-Z])[A-Z](?=[a-z]))") ANY_DEV_MODE = BOT_MODE & (BotModes.DEVELOP.value + BotModes.PLUGIN_DEV.value) From e66092ed637b3e381f2a269cf9b1d95c34d9ce3f Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Wed, 20 Oct 2021 16:13:15 -0400 Subject: [PATCH 45/83] deps: update testing dependencies --- poetry.lock | 106 ++++++++++++++++++++----------------------------- pyproject.toml | 4 +- 2 files changed, 45 insertions(+), 65 deletions(-) diff --git a/poetry.lock b/poetry.lock index 6bf14cad..351e76cf 100644 --- a/poetry.lock +++ b/poetry.lock @@ -238,17 +238,17 @@ cron = ["capturer (>=2.4)"] [[package]] name = "coverage" -version = "5.5" +version = "6.0.2" description = "Code coverage measurement for Python" category = "dev" optional = false -python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, <4" +python-versions = ">=3.6" [package.dependencies] -toml = {version = "*", optional = true, markers = "extra == \"toml\""} +tomli = {version = "*", optional = true, markers = "extra == \"toml\""} [package.extras] -toml = ["toml"] +toml = ["tomli"] [[package]] name = "discord.py" @@ -907,16 +907,15 @@ testing = ["coverage", "hypothesis (>=5.7.1)"] [[package]] name = "pytest-cov" -version = "2.12.1" +version = "3.0.0" description = "Pytest plugin for measuring coverage." category = "dev" optional = false -python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*" +python-versions = ">=3.6" [package.dependencies] -coverage = ">=5.2.1" +coverage = {version = ">=5.2.1", extras = ["toml"]} pytest = ">=4.6" -toml = "*" [package.extras] testing = ["fields", "hunter", "process-tests", "six", "pytest-xdist", "virtualenv"] @@ -1207,7 +1206,7 @@ testing = ["pytest (>=4.6)", "pytest-checkdocs (>=2.4)", "pytest-flake8", "pytes [metadata] lock-version = "1.1" python-versions = "^3.8" -content-hash = "13d426f2143ed7a3fb374ff734603e7dd6e16bf259834232f9fb6c056f11f2bb" +content-hash = "5549883a46cb4aa2d993a219229ca89255dce3f6b10fbc5b86cc86c1f15893f8" [metadata.files] aiodns = [ @@ -1427,58 +1426,39 @@ coloredlogs = [ {file = "coloredlogs-15.0.1.tar.gz", hash = "sha256:7c991aa71a4577af2f82600d8f8f3a89f936baeaf9b50a9c197da014e5bf16b0"}, ] coverage = [ - {file = "coverage-5.5-cp27-cp27m-macosx_10_9_x86_64.whl", hash = "sha256:b6d534e4b2ab35c9f93f46229363e17f63c53ad01330df9f2d6bd1187e5eaacf"}, - {file = "coverage-5.5-cp27-cp27m-manylinux1_i686.whl", hash = "sha256:b7895207b4c843c76a25ab8c1e866261bcfe27bfaa20c192de5190121770672b"}, - {file = "coverage-5.5-cp27-cp27m-manylinux1_x86_64.whl", hash = "sha256:c2723d347ab06e7ddad1a58b2a821218239249a9e4365eaff6649d31180c1669"}, - {file = "coverage-5.5-cp27-cp27m-manylinux2010_i686.whl", hash = "sha256:900fbf7759501bc7807fd6638c947d7a831fc9fdf742dc10f02956ff7220fa90"}, - {file = "coverage-5.5-cp27-cp27m-manylinux2010_x86_64.whl", hash = "sha256:004d1880bed2d97151facef49f08e255a20ceb6f9432df75f4eef018fdd5a78c"}, - {file = "coverage-5.5-cp27-cp27m-win32.whl", hash = "sha256:06191eb60f8d8a5bc046f3799f8a07a2d7aefb9504b0209aff0b47298333302a"}, - {file = "coverage-5.5-cp27-cp27m-win_amd64.whl", hash = "sha256:7501140f755b725495941b43347ba8a2777407fc7f250d4f5a7d2a1050ba8e82"}, - {file = "coverage-5.5-cp27-cp27mu-manylinux1_i686.whl", hash = "sha256:372da284cfd642d8e08ef606917846fa2ee350f64994bebfbd3afb0040436905"}, - {file = "coverage-5.5-cp27-cp27mu-manylinux1_x86_64.whl", hash = "sha256:8963a499849a1fc54b35b1c9f162f4108017b2e6db2c46c1bed93a72262ed083"}, - {file = "coverage-5.5-cp27-cp27mu-manylinux2010_i686.whl", hash = "sha256:869a64f53488f40fa5b5b9dcb9e9b2962a66a87dab37790f3fcfb5144b996ef5"}, - {file = "coverage-5.5-cp27-cp27mu-manylinux2010_x86_64.whl", hash = "sha256:4a7697d8cb0f27399b0e393c0b90f0f1e40c82023ea4d45d22bce7032a5d7b81"}, - {file = "coverage-5.5-cp310-cp310-macosx_10_14_x86_64.whl", hash = "sha256:8d0a0725ad7c1a0bcd8d1b437e191107d457e2ec1084b9f190630a4fb1af78e6"}, - {file = "coverage-5.5-cp310-cp310-manylinux1_x86_64.whl", hash = "sha256:51cb9476a3987c8967ebab3f0fe144819781fca264f57f89760037a2ea191cb0"}, - {file = "coverage-5.5-cp310-cp310-win_amd64.whl", hash = "sha256:c0891a6a97b09c1f3e073a890514d5012eb256845c451bd48f7968ef939bf4ae"}, - {file = "coverage-5.5-cp35-cp35m-macosx_10_9_x86_64.whl", hash = "sha256:3487286bc29a5aa4b93a072e9592f22254291ce96a9fbc5251f566b6b7343cdb"}, - {file = "coverage-5.5-cp35-cp35m-manylinux1_i686.whl", hash = "sha256:deee1077aae10d8fa88cb02c845cfba9b62c55e1183f52f6ae6a2df6a2187160"}, - {file = "coverage-5.5-cp35-cp35m-manylinux1_x86_64.whl", hash = "sha256:f11642dddbb0253cc8853254301b51390ba0081750a8ac03f20ea8103f0c56b6"}, - {file = "coverage-5.5-cp35-cp35m-manylinux2010_i686.whl", hash = "sha256:6c90e11318f0d3c436a42409f2749ee1a115cd8b067d7f14c148f1ce5574d701"}, - {file = "coverage-5.5-cp35-cp35m-manylinux2010_x86_64.whl", hash = "sha256:30c77c1dc9f253283e34c27935fded5015f7d1abe83bc7821680ac444eaf7793"}, - {file = "coverage-5.5-cp35-cp35m-win32.whl", hash = "sha256:9a1ef3b66e38ef8618ce5fdc7bea3d9f45f3624e2a66295eea5e57966c85909e"}, - {file = "coverage-5.5-cp35-cp35m-win_amd64.whl", hash = "sha256:972c85d205b51e30e59525694670de6a8a89691186012535f9d7dbaa230e42c3"}, - {file = "coverage-5.5-cp36-cp36m-macosx_10_9_x86_64.whl", hash = "sha256:af0e781009aaf59e25c5a678122391cb0f345ac0ec272c7961dc5455e1c40066"}, - {file = "coverage-5.5-cp36-cp36m-manylinux1_i686.whl", hash = "sha256:74d881fc777ebb11c63736622b60cb9e4aee5cace591ce274fb69e582a12a61a"}, - {file = "coverage-5.5-cp36-cp36m-manylinux1_x86_64.whl", hash = "sha256:92b017ce34b68a7d67bd6d117e6d443a9bf63a2ecf8567bb3d8c6c7bc5014465"}, - {file = "coverage-5.5-cp36-cp36m-manylinux2010_i686.whl", hash = "sha256:d636598c8305e1f90b439dbf4f66437de4a5e3c31fdf47ad29542478c8508bbb"}, - {file = "coverage-5.5-cp36-cp36m-manylinux2010_x86_64.whl", hash = "sha256:41179b8a845742d1eb60449bdb2992196e211341818565abded11cfa90efb821"}, - {file = "coverage-5.5-cp36-cp36m-win32.whl", hash = "sha256:040af6c32813fa3eae5305d53f18875bedd079960822ef8ec067a66dd8afcd45"}, - {file = "coverage-5.5-cp36-cp36m-win_amd64.whl", hash = "sha256:5fec2d43a2cc6965edc0bb9e83e1e4b557f76f843a77a2496cbe719583ce8184"}, - {file = "coverage-5.5-cp37-cp37m-macosx_10_9_x86_64.whl", hash = "sha256:18ba8bbede96a2c3dde7b868de9dcbd55670690af0988713f0603f037848418a"}, - {file = "coverage-5.5-cp37-cp37m-manylinux1_i686.whl", hash = "sha256:2910f4d36a6a9b4214bb7038d537f015346f413a975d57ca6b43bf23d6563b53"}, - {file = "coverage-5.5-cp37-cp37m-manylinux1_x86_64.whl", hash = "sha256:f0b278ce10936db1a37e6954e15a3730bea96a0997c26d7fee88e6c396c2086d"}, - {file = "coverage-5.5-cp37-cp37m-manylinux2010_i686.whl", hash = "sha256:796c9c3c79747146ebd278dbe1e5c5c05dd6b10cc3bcb8389dfdf844f3ead638"}, - {file = "coverage-5.5-cp37-cp37m-manylinux2010_x86_64.whl", hash = "sha256:53194af30d5bad77fcba80e23a1441c71abfb3e01192034f8246e0d8f99528f3"}, - {file = "coverage-5.5-cp37-cp37m-win32.whl", hash = "sha256:184a47bbe0aa6400ed2d41d8e9ed868b8205046518c52464fde713ea06e3a74a"}, - {file = "coverage-5.5-cp37-cp37m-win_amd64.whl", hash = "sha256:2949cad1c5208b8298d5686d5a85b66aae46d73eec2c3e08c817dd3513e5848a"}, - {file = "coverage-5.5-cp38-cp38-macosx_10_9_x86_64.whl", hash = "sha256:217658ec7187497e3f3ebd901afdca1af062b42cfe3e0dafea4cced3983739f6"}, - {file = "coverage-5.5-cp38-cp38-manylinux1_i686.whl", hash = "sha256:1aa846f56c3d49205c952d8318e76ccc2ae23303351d9270ab220004c580cfe2"}, - {file = "coverage-5.5-cp38-cp38-manylinux1_x86_64.whl", hash = "sha256:24d4a7de75446be83244eabbff746d66b9240ae020ced65d060815fac3423759"}, - {file = "coverage-5.5-cp38-cp38-manylinux2010_i686.whl", hash = "sha256:d1f8bf7b90ba55699b3a5e44930e93ff0189aa27186e96071fac7dd0d06a1873"}, - {file = "coverage-5.5-cp38-cp38-manylinux2010_x86_64.whl", hash = "sha256:970284a88b99673ccb2e4e334cfb38a10aab7cd44f7457564d11898a74b62d0a"}, - {file = "coverage-5.5-cp38-cp38-win32.whl", hash = "sha256:01d84219b5cdbfc8122223b39a954820929497a1cb1422824bb86b07b74594b6"}, - {file = "coverage-5.5-cp38-cp38-win_amd64.whl", hash = "sha256:2e0d881ad471768bf6e6c2bf905d183543f10098e3b3640fc029509530091502"}, - {file = "coverage-5.5-cp39-cp39-macosx_10_9_x86_64.whl", hash = "sha256:d1f9ce122f83b2305592c11d64f181b87153fc2c2bbd3bb4a3dde8303cfb1a6b"}, - {file = "coverage-5.5-cp39-cp39-manylinux1_i686.whl", hash = "sha256:13c4ee887eca0f4c5a247b75398d4114c37882658300e153113dafb1d76de529"}, - {file = "coverage-5.5-cp39-cp39-manylinux1_x86_64.whl", hash = "sha256:52596d3d0e8bdf3af43db3e9ba8dcdaac724ba7b5ca3f6358529d56f7a166f8b"}, - {file = "coverage-5.5-cp39-cp39-manylinux2010_i686.whl", hash = "sha256:2cafbbb3af0733db200c9b5f798d18953b1a304d3f86a938367de1567f4b5bff"}, - {file = "coverage-5.5-cp39-cp39-manylinux2010_x86_64.whl", hash = "sha256:44d654437b8ddd9eee7d1eaee28b7219bec228520ff809af170488fd2fed3e2b"}, - {file = "coverage-5.5-cp39-cp39-win32.whl", hash = "sha256:d314ed732c25d29775e84a960c3c60808b682c08d86602ec2c3008e1202e3bb6"}, - {file = "coverage-5.5-cp39-cp39-win_amd64.whl", hash = "sha256:13034c4409db851670bc9acd836243aeee299949bd5673e11844befcb0149f03"}, - {file = "coverage-5.5-pp36-none-any.whl", hash = "sha256:f030f8873312a16414c0d8e1a1ddff2d3235655a2174e3648b4fa66b3f2f1079"}, - {file = "coverage-5.5-pp37-none-any.whl", hash = "sha256:2a3859cb82dcbda1cfd3e6f71c27081d18aa251d20a17d87d26d4cd216fb0af4"}, - {file = "coverage-5.5.tar.gz", hash = "sha256:ebe78fe9a0e874362175b02371bdfbee64d8edc42a044253ddf4ee7d3c15212c"}, + {file = "coverage-6.0.2-cp310-cp310-macosx_10_9_x86_64.whl", hash = "sha256:1549e1d08ce38259de2bc3e9a0d5f3642ff4a8f500ffc1b2df73fd621a6cdfc0"}, + {file = "coverage-6.0.2-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:bcae10fccb27ca2a5f456bf64d84110a5a74144be3136a5e598f9d9fb48c0caa"}, + {file = "coverage-6.0.2-cp310-cp310-manylinux_2_5_i686.manylinux1_i686.manylinux_2_12_i686.manylinux2010_i686.whl", hash = "sha256:53a294dc53cfb39c74758edaa6305193fb4258a30b1f6af24b360a6c8bd0ffa7"}, + {file = "coverage-6.0.2-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl", hash = "sha256:8251b37be1f2cd9c0e5ccd9ae0380909c24d2a5ed2162a41fcdbafaf59a85ebd"}, + {file = "coverage-6.0.2-cp310-cp310-win32.whl", hash = "sha256:db42baa892cba723326284490283a68d4de516bfb5aaba369b4e3b2787a778b7"}, + {file = "coverage-6.0.2-cp310-cp310-win_amd64.whl", hash = "sha256:bbffde2a68398682623d9dd8c0ca3f46fda074709b26fcf08ae7a4c431a6ab2d"}, + {file = "coverage-6.0.2-cp36-cp36m-macosx_10_9_x86_64.whl", hash = "sha256:60e51a3dd55540bec686d7fff61b05048ca31e804c1f32cbb44533e6372d9cc3"}, + {file = "coverage-6.0.2-cp36-cp36m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:6a6a9409223a27d5ef3cca57dd7cd4dfcb64aadf2fad5c3b787830ac9223e01a"}, + {file = "coverage-6.0.2-cp36-cp36m-manylinux_2_5_i686.manylinux1_i686.manylinux_2_12_i686.manylinux2010_i686.whl", hash = "sha256:4b34ae4f51bbfa5f96b758b55a163d502be3dcb24f505d0227858c2b3f94f5b9"}, + {file = "coverage-6.0.2-cp36-cp36m-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl", hash = "sha256:3bbda1b550e70fa6ac40533d3f23acd4f4e9cb4e6e77251ce77fdf41b3309fb2"}, + {file = "coverage-6.0.2-cp36-cp36m-win32.whl", hash = "sha256:4e28d2a195c533b58fc94a12826f4431726d8eb029ac21d874345f943530c122"}, + {file = "coverage-6.0.2-cp36-cp36m-win_amd64.whl", hash = "sha256:a82d79586a0a4f5fd1cf153e647464ced402938fbccb3ffc358c7babd4da1dd9"}, + {file = "coverage-6.0.2-cp37-cp37m-macosx_10_9_x86_64.whl", hash = "sha256:3be1206dc09fb6298de3fce70593e27436862331a85daee36270b6d0e1c251c4"}, + {file = "coverage-6.0.2-cp37-cp37m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:c9cd3828bbe1a40070c11fe16a51df733fd2f0cb0d745fb83b7b5c1f05967df7"}, + {file = "coverage-6.0.2-cp37-cp37m-manylinux_2_5_i686.manylinux1_i686.manylinux_2_12_i686.manylinux2010_i686.whl", hash = "sha256:d036dc1ed8e1388e995833c62325df3f996675779541f682677efc6af71e96cc"}, + {file = "coverage-6.0.2-cp37-cp37m-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl", hash = "sha256:04560539c19ec26995ecfb3d9307ff154fbb9a172cb57e3b3cfc4ced673103d1"}, + {file = "coverage-6.0.2-cp37-cp37m-win32.whl", hash = "sha256:e4fb7ced4d9dec77d6cf533acfbf8e1415fe799430366affb18d69ee8a3c6330"}, + {file = "coverage-6.0.2-cp37-cp37m-win_amd64.whl", hash = "sha256:77b1da5767ed2f44611bc9bc019bc93c03fa495728ec389759b6e9e5039ac6b1"}, + {file = "coverage-6.0.2-cp38-cp38-macosx_10_9_x86_64.whl", hash = "sha256:61b598cbdbaae22d9e34e3f675997194342f866bb1d781da5d0be54783dce1ff"}, + {file = "coverage-6.0.2-cp38-cp38-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:36e9040a43d2017f2787b28d365a4bb33fcd792c7ff46a047a04094dc0e2a30d"}, + {file = "coverage-6.0.2-cp38-cp38-manylinux_2_5_i686.manylinux1_i686.manylinux_2_12_i686.manylinux2010_i686.whl", hash = "sha256:9f1627e162e3864a596486774876415a7410021f4b67fd2d9efdf93ade681afc"}, + {file = "coverage-6.0.2-cp38-cp38-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl", hash = "sha256:e7a0b42db2a47ecb488cde14e0f6c7679a2c5a9f44814393b162ff6397fcdfbb"}, + {file = "coverage-6.0.2-cp38-cp38-win32.whl", hash = "sha256:a1b73c7c4d2a42b9d37dd43199c5711d91424ff3c6c22681bc132db4a4afec6f"}, + {file = "coverage-6.0.2-cp38-cp38-win_amd64.whl", hash = "sha256:1db67c497688fd4ba85b373b37cc52c50d437fd7267520ecd77bddbd89ea22c9"}, + {file = "coverage-6.0.2-cp39-cp39-macosx_10_9_x86_64.whl", hash = "sha256:f2f184bf38e74f152eed7f87e345b51f3ab0b703842f447c22efe35e59942c24"}, + {file = "coverage-6.0.2-cp39-cp39-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:cd1cf1deb3d5544bd942356364a2fdc8959bad2b6cf6eb17f47d301ea34ae822"}, + {file = "coverage-6.0.2-cp39-cp39-manylinux_2_5_i686.manylinux1_i686.manylinux_2_12_i686.manylinux2010_i686.whl", hash = "sha256:ad9b8c1206ae41d46ec7380b78ba735ebb77758a650643e841dd3894966c31d0"}, + {file = "coverage-6.0.2-cp39-cp39-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl", hash = "sha256:381d773d896cc7f8ba4ff3b92dee4ed740fb88dfe33b6e42efc5e8ab6dfa1cfe"}, + {file = "coverage-6.0.2-cp39-cp39-win32.whl", hash = "sha256:424c44f65e8be58b54e2b0bd1515e434b940679624b1b72726147cfc6a9fc7ce"}, + {file = "coverage-6.0.2-cp39-cp39-win_amd64.whl", hash = "sha256:abbff240f77347d17306d3201e14431519bf64495648ca5a49571f988f88dee9"}, + {file = "coverage-6.0.2-pp36-none-any.whl", hash = "sha256:7092eab374346121805fb637572483270324407bf150c30a3b161fc0c4ca5164"}, + {file = "coverage-6.0.2-pp37-none-any.whl", hash = "sha256:30922626ce6f7a5a30bdba984ad21021529d3d05a68b4f71ea3b16bda35b8895"}, + {file = "coverage-6.0.2.tar.gz", hash = "sha256:6807947a09510dc31fa86f43595bf3a14017cd60bf633cc746d52141bfa6b149"}, ] "discord.py" = [] distlib = [ @@ -1868,8 +1848,8 @@ pytest-asyncio = [ {file = "pytest_asyncio-0.15.1-py3-none-any.whl", hash = "sha256:3042bcdf1c5d978f6b74d96a151c4cfb9dcece65006198389ccd7e6c60eb1eea"}, ] pytest-cov = [ - {file = "pytest-cov-2.12.1.tar.gz", hash = "sha256:261ceeb8c227b726249b376b8526b600f38667ee314f910353fa318caa01f4d7"}, - {file = "pytest_cov-2.12.1-py2.py3-none-any.whl", hash = "sha256:261bb9e47e65bd099c89c3edf92972865210c36813f80ede5277dceb77a4a62a"}, + {file = "pytest-cov-3.0.0.tar.gz", hash = "sha256:e7f0f5b1617d2210a2cabc266dfe2f4c75a8d32fb89eafb7ad9d06f6d076d470"}, + {file = "pytest_cov-3.0.0-py3-none-any.whl", hash = "sha256:578d5d15ac4a25e5f961c938b85a05b09fdaae9deef3bb6de9a6e766622ca7a6"}, ] pytest-dependency = [ {file = "pytest-dependency-0.5.1.tar.gz", hash = "sha256:c2a892906192663f85030a6ab91304e508e546cddfe557d692d61ec57a1d946b"}, diff --git a/pyproject.toml b/pyproject.toml index 2a7a2de7..d32d6502 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,10 +45,10 @@ isort = "^5.9.2" pep8-naming = "~=0.11" # testing codecov = "^2.1.11" -coverage = { extras = ["toml"], version = "^5.5" } +coverage = { extras = ["toml"], version = "^6.0.2" } pytest = "^6.2.4" pytest-asyncio = "^0.15.1" -pytest-cov = "^2.12.1" +pytest-cov = "^3.0.0" pytest-dependency = "^0.5.1" pytest-sugar = "^0.9.4" pytest-xdist = { version = "^2.3.0", extras = ["psutil"] } From ce1ef378aad249899cd1c80e01956a166a2bce49 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Wed, 20 Oct 2021 18:22:58 -0400 Subject: [PATCH 46/83] tests: rewrite test_mocks to use pytest instead of unittest --- tests/test_mocks.py | 317 +++++++++++++++++++++----------------------- 1 file changed, 152 insertions(+), 165 deletions(-) diff --git a/tests/test_mocks.py b/tests/test_mocks.py index b50c6c98..8212bf9f 100644 --- a/tests/test_mocks.py +++ b/tests/test_mocks.py @@ -4,6 +4,9 @@ Original Source: https://github.com/python-discord/bot/blob/d183d03fa2939bebaac3da49646449fdd4d00e6c/tests/test_helpers.py # noqa: E501 +NOTE: THIS FILE WAS REWRITTEN TO USE PYTEST + + MIT License Copyright (c) 2018 Python Discord @@ -28,27 +31,28 @@ """ import asyncio -import unittest import unittest.mock import discord +import discord.ext.commands +import pytest from tests import mocks as test_mocks -class DiscordMocksTests(unittest.TestCase): +class TestDiscordMocks: """Tests for our specialized discord.py mocks.""" def test_mock_role_default_initialization(self): """Test if the default initialization of MockRole results in the correct object.""" role = test_mocks.MockRole() - # The `spec` argument makes sure `isistance` checks with `discord.Role` pass - self.assertIsInstance(role, discord.Role) + # The `spec` argument makes sure `isinstance` checks with `discord.Role` pass + assert isinstance(role, discord.Role) - self.assertEqual(role.name, "role") - self.assertEqual(role.position, 1) - self.assertEqual(role.mention, "&role") + assert role.name == "role" + assert role.position == 1 + assert role.mention == "&role" def test_mock_role_alternative_arguments(self): """Test if MockRole initializes with the arguments provided.""" @@ -58,10 +62,10 @@ def test_mock_role_alternative_arguments(self): position=10, ) - self.assertEqual(role.name, "Admins") - self.assertEqual(role.id, 90210) - self.assertEqual(role.position, 10) - self.assertEqual(role.mention, "&Admins") + assert role.name == "Admins" + assert role.id == 90210 + assert role.position == 10 + assert role.mention == "&Admins" def test_mock_role_accepts_dynamic_arguments(self): """Test if MockRole accepts and sets abitrary keyword arguments.""" @@ -70,8 +74,8 @@ def test_mock_role_accepts_dynamic_arguments(self): hoist=True, ) - self.assertEqual(role.guild, "Dino Man") - self.assertTrue(role.hoist) + assert role.guild == "Dino Man" + assert role.hoist def test_mock_role_uses_position_for_less_than_greater_than(self): """Test if `<` and `>` comparisons for MockRole are based on its position attribute.""" @@ -79,35 +83,33 @@ def test_mock_role_uses_position_for_less_than_greater_than(self): role_two = test_mocks.MockRole(position=2) role_three = test_mocks.MockRole(position=3) - self.assertLess(role_one, role_two) - self.assertLess(role_one, role_three) - self.assertLess(role_two, role_three) - self.assertGreater(role_three, role_two) - self.assertGreater(role_three, role_one) - self.assertGreater(role_two, role_one) + assert role_one < role_two + assert role_one < role_three + assert role_two < role_three + assert role_three > role_two + assert role_three > role_one + assert role_two > role_one def test_mock_member_default_initialization(self): """Test if the default initialization of Mockmember results in the correct object.""" member = test_mocks.MockMember() - # The `spec` argument makes sure `isistance` checks with `discord.Member` pass - self.assertIsInstance(member, discord.Member) + # The `spec` argument makes sure `isinstance` checks with `discord.Member` pass + assert isinstance(member, discord.Member) - self.assertEqual(member.name, "member") - self.assertListEqual(member.roles, [test_mocks.MockRole(name="@everyone", position=1, id=0)]) - self.assertEqual(member.mention, "@member") + assert member.name == "member" + assert member.roles == [test_mocks.MockRole(name="@everyone", position=1, id=0)] + assert member.mention == "@member" def test_mock_member_alternative_arguments(self): """Test if MockMember initializes with the arguments provided.""" core_developer = test_mocks.MockRole(name="Core Developer", position=2) member = test_mocks.MockMember(name="Mark", id=12345, roles=[core_developer]) - self.assertEqual(member.name, "Mark") - self.assertEqual(member.id, 12345) - self.assertListEqual( - member.roles, [test_mocks.MockRole(name="@everyone", position=1, id=0), core_developer] - ) - self.assertEqual(member.mention, "@Mark") + assert member.name == "Mark" + assert member.id == 12345 + assert member.roles == [test_mocks.MockRole(name="@everyone", position=1, id=0), core_developer] + assert member.mention == "@Mark" def test_mock_member_accepts_dynamic_arguments(self): """Test if MockMember accepts and sets abitrary keyword arguments.""" @@ -116,18 +118,18 @@ def test_mock_member_accepts_dynamic_arguments(self): colour=discord.Colour.default(), ) - self.assertEqual(member.nick, "Dino Man") - self.assertEqual(member.colour, discord.Colour.default()) + assert member.nick == "Dino Man" + assert member.colour == discord.Colour.default() def test_mock_guild_default_initialization(self): """Test if the default initialization of Mockguild results in the correct object.""" guild = test_mocks.MockGuild() # The `spec` argument makes sure `isistance` checks with `discord.Guild` pass - self.assertIsInstance(guild, discord.Guild) + assert isinstance(guild, discord.Guild) - self.assertListEqual(guild.roles, [test_mocks.MockRole(name="@everyone", position=1, id=0)]) - self.assertListEqual(guild.members, []) + assert guild.roles == [test_mocks.MockRole(name="@everyone", position=1, id=0)] + assert guild.members == [] def test_mock_guild_alternative_arguments(self): """Test if MockGuild initializes with the arguments provided.""" @@ -137,10 +139,8 @@ def test_mock_guild_alternative_arguments(self): members=[test_mocks.MockMember(id=54321)], ) - self.assertListEqual( - guild.roles, [test_mocks.MockRole(name="@everyone", position=1, id=0), core_developer] - ) - self.assertListEqual(guild.members, [test_mocks.MockMember(id=54321)]) + assert guild.roles == [test_mocks.MockRole(name="@everyone", position=1, id=0), core_developer] + assert guild.members == [test_mocks.MockMember(id=54321)] def test_mock_guild_accepts_dynamic_arguments(self): """Test if MockGuild accepts and sets abitrary keyword arguments.""" @@ -149,113 +149,101 @@ def test_mock_guild_accepts_dynamic_arguments(self): premium_subscription_count=15, ) - self.assertTupleEqual(guild.emojis, (":hyperjoseph:", ":pensive_ela:")) - self.assertEqual(guild.premium_subscription_count, 15) + assert guild.emojis == (":hyperjoseph:", ":pensive_ela:") + assert guild.premium_subscription_count == 15 def test_mock_bot_default_initialization(self): """Tests if MockBot initializes with the correct values.""" bot = test_mocks.MockBot() - # The `spec` argument makes sure `isistance` checks with `discord.ext.commands.Bot` pass - self.assertIsInstance(bot, discord.ext.commands.Bot) + # The `spec` argument makes sure `isinstance` checks with `discord.ext.commands.Bot` pass + assert isinstance(bot, discord.ext.commands.Bot) def test_mock_context_default_initialization(self): """Tests if MockContext initializes with the correct values.""" context = test_mocks.MockContext() - # The `spec` argument makes sure `isistance` checks with `discord.ext.commands.Context` pass - self.assertIsInstance(context, discord.ext.commands.Context) - - self.assertIsInstance(context.bot, test_mocks.MockBot) - self.assertIsInstance(context.guild, test_mocks.MockGuild) - self.assertIsInstance(context.author, test_mocks.MockMember) - - def test_mocks_allows_access_to_attributes_part_of_spec(self): + # The `spec` argument makes sure `isinstance` checks with `discord.ext.commands.Context` pass + assert isinstance(context, discord.ext.commands.Context) + + assert isinstance(context.bot, test_mocks.MockBot) + assert isinstance(context.guild, test_mocks.MockGuild) + assert isinstance(context.author, test_mocks.MockMember) + assert isinstance(context.message, test_mocks.MockMessage) + + @pytest.mark.parametrize( + ["mock", "valid_attribute"], + [ + [test_mocks.MockGuild(), "name"], + [test_mocks.MockRole(), "hoist"], + [test_mocks.MockMember(), "display_name"], + [test_mocks.MockBot(), "user"], + [test_mocks.MockContext(), "invoked_with"], + [test_mocks.MockTextChannel(), "last_message"], + [test_mocks.MockMessage(), "mention_everyone"], + ], + ) + def test_mocks_allows_access_to_attributes_part_of_spec(self, mock, valid_attribute: str): """Accessing attributes that are valid for the objects they mock should succeed.""" - mocks = ( - (test_mocks.MockGuild(), "name"), - (test_mocks.MockRole(), "hoist"), - (test_mocks.MockMember(), "display_name"), - (test_mocks.MockBot(), "user"), - (test_mocks.MockContext(), "invoked_with"), - (test_mocks.MockTextChannel(), "last_message"), - (test_mocks.MockMessage(), "mention_everyone"), - ) - - for mock, valid_attribute in mocks: - with self.subTest(mock=mock): - try: - getattr(mock, valid_attribute) - except AttributeError: - msg = f"accessing valid attribute `{valid_attribute}` raised an AttributeError" - self.fail(msg) - - @unittest.mock.patch(f"{__name__}.DiscordMocksTests.subTest") - @unittest.mock.patch(f"{__name__}.getattr") - def test_mock_allows_access_to_attributes_test(self, mock_getattr, mock_subtest): - """The valid attribute test should raise an AssertionError after an AttributeError.""" - mock_getattr.side_effect = AttributeError - - msg = "accessing valid attribute `name` raised an AttributeError" - with self.assertRaises(AssertionError, msg=msg): - self.test_mocks_allows_access_to_attributes_part_of_spec() - - def test_mocks_rejects_access_to_attributes_not_part_of_spec(self): + try: + getattr(mock, valid_attribute) + except AttributeError: + msg = f"accessing valid attribute `{valid_attribute}` raised an AttributeError" + pytest.fail(msg) + + @pytest.mark.parametrize( + ["mock"], + [ + [test_mocks.MockGuild()], + [test_mocks.MockRole()], + [test_mocks.MockMember()], + [test_mocks.MockBot()], + [test_mocks.MockContext()], + [test_mocks.MockTextChannel()], + [test_mocks.MockMessage()], + ], + ) + def test_mocks_rejects_access_to_attributes_not_part_of_spec(self, mock): """Accessing attributes that are invalid for the objects they mock should fail.""" - mocks = ( - test_mocks.MockGuild(), - test_mocks.MockRole(), - test_mocks.MockMember(), - test_mocks.MockBot(), - test_mocks.MockContext(), - test_mocks.MockTextChannel(), - test_mocks.MockMessage(), - ) - - for mock in mocks: - with self.subTest(mock=mock): - with self.assertRaises(AttributeError): - mock.the_cake_is_a_lie - - def test_mocks_use_mention_when_provided_as_kwarg(self): + with pytest.raises(AttributeError): + mock.the_cake_is_a_lie + + @pytest.mark.parametrize( + ["mock_type", "provided_mention"], + [ + [test_mocks.MockRole, "role mention"], + [test_mocks.MockMember, "member mention"], + [test_mocks.MockTextChannel, "channel mention"], + [test_mocks.MockUser, "user mention"], + ], + ) + def test_mocks_use_mention_when_provided_as_kwarg(self, mock_type, provided_mention: str): """The mock should use the passed `mention` instead of the default one if present.""" - test_cases = ( - (test_mocks.MockRole, "role mention"), - (test_mocks.MockMember, "member mention"), - (test_mocks.MockTextChannel, "channel mention"), - ) - - for mock_type, mention in test_cases: - with self.subTest(mock_type=mock_type, mention=mention): - mock = mock_type(mention=mention) - self.assertEqual(mock.mention, mention) + mock = mock_type(mention=provided_mention) + assert mock.mention == provided_mention def test_create_test_on_mock_bot_closes_passed_coroutine(self): """`bot.loop.create_task` should close the passed coroutine object to prevent warnings.""" async def dementati(): """Dummy coroutine for testing purposes.""" + pass coroutine_object = dementati() bot = test_mocks.MockBot() bot.loop.create_task(coroutine_object) - with self.assertRaises(RuntimeError, msg="cannot reuse already awaited coroutine"): + with pytest.raises(RuntimeError) as error: asyncio.run(coroutine_object) + assert error.match("cannot reuse already awaited coroutine") - def test_user_mock_uses_explicitly_passed_mention_attribute(self): - """Ensure MockUser uses an explictly passed value for user.mention.""" - user = test_mocks.MockUser(mention="hello") - self.assertEqual(user.mention, "hello") +hashable_mocks = (test_mocks.MockRole, test_mocks.MockMember, test_mocks.MockGuild) +print([[x] for x in hashable_mocks]) -class MockObjectTests(unittest.TestCase): - """Tests the mock objects and mixins we've defined.""" - @classmethod - def setUpClass(cls): - """Called by unittest before running the test methods.""" - cls.hashable_mocks = (test_mocks.MockRole, test_mocks.MockMember, test_mocks.MockGuild) +class TestMockObjects: + """Tests the mock objects and mixins we've defined.""" def test_colour_mixin(self): """Test if the ColourMixin adds aliasing of color -> colour for child classes.""" @@ -265,8 +253,8 @@ class MockHemlock(unittest.mock.MagicMock, test_mocks.ColourMixin): hemlock = MockHemlock() hemlock.color = 1 - self.assertEqual(hemlock.colour, 1) - self.assertEqual(hemlock.colour, hemlock.color) + assert hemlock.colour == 1 + assert hemlock.colour == hemlock.color def test_hashable_mixin_hash_returns_id(self): """Test the HashableMixing uses the id attribute for hashing.""" @@ -276,7 +264,7 @@ class MockScragly(unittest.mock.Mock, test_mocks.HashableMixin): scragly = MockScragly() scragly.id = 10 - self.assertEqual(hash(scragly), scragly.id) + assert hash(scragly) == scragly.id def test_hashable_mixin_hash_returns_id_bitshift(self): """Test the HashableMixing uses the id attribute for hashing when above 1<<22.""" @@ -286,7 +274,7 @@ class MockScragly(unittest.mock.Mock, test_mocks.HashableMixin): scragly = MockScragly() scragly.id = 10 << 22 - self.assertEqual(hash(scragly), scragly.id >> 22) + assert hash(scragly) == scragly.id >> 22 def test_hashable_mixin_uses_id_for_equality_comparison(self): """Test the HashableMixing uses the id attribute for equal comparison.""" @@ -301,8 +289,8 @@ class MockScragly(test_mocks.HashableMixin): python = MockScragly() python.id = 20 - self.assertTrue(scragly == eevee) - self.assertFalse(scragly == python) + assert scragly == eevee + assert (scragly == python) is False def test_hashable_mixin_uses_id_for_nonequality_comparison(self): """Test if the HashableMixing uses the id attribute for non-equal comparison.""" @@ -317,45 +305,42 @@ class MockScragly(test_mocks.HashableMixin): python = MockScragly() python.id = 20 - self.assertTrue(scragly != python) - self.assertFalse(scragly != eevee) + assert scragly != python + assert (scragly != eevee) is False - def test_mock_class_with_hashable_mixin_uses_id_for_hashing(self): + @pytest.mark.parametrize(["mock_cls"], [[x] for x in hashable_mocks]) + def test_mock_class_with_hashable_mixin_uses_id_for_hashing(self, mock_cls): """Test if the MagicMock subclasses that implement the HashableMixin use id for hash.""" - for mock in self.hashable_mocks: - with self.subTest(mock_class=mock): - instance = test_mocks.MockRole(id=100) - self.assertEqual(hash(instance), instance.id) + instance = mock_cls(id=100) + assert hash(instance) == instance.id - def test_mock_class_with_hashable_mixin_uses_id_for_equality(self): + @pytest.mark.parametrize(["mock_class"], [[x] for x in hashable_mocks]) + def test_mock_class_with_hashable_mixin_uses_id_for_equality(self, mock_class): """Test if MagicMocks that implement the HashableMixin use id for equality comparisons.""" - for mock_class in self.hashable_mocks: - with self.subTest(mock_class=mock_class): - instance_one = mock_class() - instance_two = mock_class() - instance_three = mock_class() + instance_one = mock_class() + instance_two = mock_class() + instance_three = mock_class() - instance_one.id = 10 - instance_two.id = 10 - instance_three.id = 20 + instance_one.id = 10 + instance_two.id = 10 + instance_three.id = 20 - self.assertTrue(instance_one == instance_two) - self.assertFalse(instance_one == instance_three) + assert instance_one == instance_two + assert (instance_one == instance_three) is False - def test_mock_class_with_hashable_mixin_uses_id_for_nonequality(self): + @pytest.mark.parametrize(["mock_class"], [[x] for x in hashable_mocks]) + def test_mock_class_with_hashable_mixin_uses_id_for_nonequality(self, mock_class): """Test if MagicMocks that implement HashableMixin use id for nonequality comparisons.""" - for mock_class in self.hashable_mocks: - with self.subTest(mock_class=mock_class): - instance_one = mock_class() - instance_two = mock_class() - instance_three = mock_class() + instance_one = mock_class() + instance_two = mock_class() + instance_three = mock_class() - instance_one.id = 10 - instance_two.id = 10 - instance_three.id = 20 + instance_one.id = 10 + instance_two.id = 10 + instance_three.id = 20 - self.assertFalse(instance_one != instance_two) - self.assertTrue(instance_one != instance_three) + assert instance_one != instance_three + assert (instance_one != instance_two) is False def test_custom_mock_mixin_accepts_mock_seal(self): """The `CustomMockMixin` should support `unittest.mock.seal`.""" @@ -367,12 +352,14 @@ class MyMock(test_mocks.CustomMockMixin, unittest.mock.MagicMock): mock = MyMock() unittest.mock.seal(mock) - with self.assertRaises(AttributeError, msg="MyMock.shirayuki"): + with pytest.raises(AttributeError) as error: mock.shirayuki = "hello!" - def test_spec_propagation_of_mock_subclasses(self): - """Test if the `spec` does not propagate to attributes of the mock object.""" - test_values = ( + assert error.match("shirayuki") + + @pytest.mark.parametrize( + ["mock_type", "valid_attribute"], + [ (test_mocks.MockGuild, "region"), (test_mocks.MockRole, "mentionable"), (test_mocks.MockMember, "display_name"), @@ -382,14 +369,14 @@ def test_spec_propagation_of_mock_subclasses(self): (test_mocks.MockEmoji, "managed"), (test_mocks.MockPartialEmoji, "url"), (test_mocks.MockReaction, "me"), - ) - - for mock_type, valid_attribute in test_values: - with self.subTest(mock_type=mock_type, attribute=valid_attribute): - mock = mock_type() - self.assertTrue(isinstance(mock, mock_type)) - attribute = getattr(mock, valid_attribute) - self.assertTrue(isinstance(attribute, mock_type.child_mock_type)) + ], + ) + def test_spec_propagation_of_mock_subclasses(self, mock_type, valid_attribute: str): + """Test if the `spec` does not propagate to attributes of the mock object.""" + mock = mock_type() + assert isinstance(mock, mock_type) + attribute = getattr(mock, valid_attribute) + assert isinstance(attribute, mock_type.child_mock_type) def test_custom_mock_mixin_mocks_async_magic_methods_with_async_mock(self): """The CustomMockMixin should mock async magic methods with an AsyncMock.""" @@ -398,4 +385,4 @@ class MyMock(test_mocks.CustomMockMixin, unittest.mock.MagicMock): pass mock = MyMock() - self.assertIsInstance(mock.__aenter__, unittest.mock.AsyncMock) + assert isinstance(mock.__aenter__, unittest.mock.AsyncMock) From 9c390b3d78ad807ed66adc80e903975bfc608a49 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Wed, 20 Oct 2021 18:31:14 -0400 Subject: [PATCH 47/83] fix: make mock context objects more accurate previously, channel, message, and guild were all mocked independently. however, multiple aspects of the context method and the message channel should be the same as the message's channel additionally, the attributes of MockContext have been typed, to make writing tests easier. --- tests/mocks.py | 17 +++++++++++------ tests/test_mocks.py | 4 ++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/mocks.py b/tests/mocks.py index 469dab4e..e2cee005 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -34,6 +34,7 @@ import collections import itertools import logging +import typing import unittest.mock from typing import TYPE_CHECKING, Iterable, Optional @@ -543,12 +544,16 @@ class MockContext(CustomMockMixin, unittest.mock.MagicMock): def __init__(self, **kwargs) -> None: super().__init__(**kwargs) - self.me = kwargs.get("me", MockMember()) - self.bot = kwargs.get("bot", MockBot()) - self.guild = kwargs.get("guild", MockGuild()) - self.author = kwargs.get("author", MockMember()) - self.channel = kwargs.get("channel", MockTextChannel()) - self.message = kwargs.get("message", MockMessage()) + self.me: typing.Union[MockMember, MockUser] = kwargs.get("me", MockMember()) + self.bot: MockBot = kwargs.get("bot", MockBot()) + self.guild: typing.Optional[MockGuild] = kwargs.get("guild", MockGuild()) + self.author: typing.Union[MockMember, MockUser] = kwargs.get("author", MockMember()) + self.channel: typing.Union[MockTextChannel, MockThread, MockDMChannel] = kwargs.get( + "channel", MockTextChannel(guild=self.guild) + ) + self.message: MockMessage = kwargs.get( + "message", MockMessage(author=self.author, channel=self.channel) + ) self.invoked_from_error_handler = kwargs.get("invoked_from_error_handler", False) diff --git a/tests/test_mocks.py b/tests/test_mocks.py index 8212bf9f..4c4da4d0 100644 --- a/tests/test_mocks.py +++ b/tests/test_mocks.py @@ -171,6 +171,10 @@ def test_mock_context_default_initialization(self): assert isinstance(context.author, test_mocks.MockMember) assert isinstance(context.message, test_mocks.MockMessage) + # ensure that the mocks are the same attributes, like discord.py + assert context.message.channel is context.channel + assert context.channel.guild is context.guild + @pytest.mark.parametrize( ["mock", "valid_attribute"], [ From 4c5fab002f9ae1e795f504f15f8fef557a0089b8 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Wed, 20 Oct 2021 21:51:11 -0400 Subject: [PATCH 48/83] tests(mocks): add MockClientUser --- tests/mocks.py | 42 ++++++++++++++++++++++++++++++++++++++---- tests/test_mocks.py | 12 ++++++++++-- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/tests/mocks.py b/tests/mocks.py index e2cee005..1ef9c167 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -63,12 +63,12 @@ class HashableMixin(discord.mixins.EqualityComparable): Given that most of our features need the created_at function to work, we are typically using full fake discord ids, and so we still bitshift the id like Dpy does. - However, given that the hash of 4>>22 and 5>>22 is the same, we check if the number is above 22. + However, given that the hash of 4>>22 and 5>>22 is the same, we check if the number is above 1<<22. If so, we hash it. This could result in some weird behavior with the hash of 1<<22 + 1 equaling 1. """ - if TYPE_CHECKING: + if TYPE_CHECKING: # pragma: nocover id: int def __hash__(self): @@ -324,6 +324,32 @@ def __init__(self, **kwargs) -> None: self.mention = f"@{self.name}" +# Create a User instance to get a realistic Mock of `discord.ClientUser` +_user_data_mock = collections.defaultdict(unittest.mock.MagicMock, {"accent_color": 0}) +clientuser_instance = discord.ClientUser( + data=unittest.mock.MagicMock(get=unittest.mock.Mock(side_effect=_user_data_mock.get)), + state=unittest.mock.MagicMock(), +) + + +class MockClientUser(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): + """ + A Mock subclass to mock ClientUser objects. + + Instances of this class will follow the specifications of `discord.ClientUser` instances. For more + information, see the `MockGuild` docstring. + """ + + spec_set = clientuser_instance + + def __init__(self, **kwargs) -> None: + default_kwargs = {"name": "user", "id": next(self.discord_id), "bot": True} + super().__init__(**collections.ChainMap(kwargs, default_kwargs)) + + if "mention" not in kwargs: + self.mention = f"@{self.name}" + + def _get_mock_loop() -> unittest.mock.Mock: """Return a mocked asyncio.AbstractEventLoop.""" loop = unittest.mock.create_autospec(spec=asyncio.AbstractEventLoop, spec_set=True) @@ -356,6 +382,7 @@ class MockBot(CustomMockMixin, unittest.mock.MagicMock): def __init__(self, **kwargs) -> None: super().__init__(**kwargs) + self.user = MockClientUser() self.loop = _get_mock_loop() self.http_session = unittest.mock.create_autospec(spec=aiohttp.ClientSession, spec_set=True) @@ -544,9 +571,10 @@ class MockContext(CustomMockMixin, unittest.mock.MagicMock): def __init__(self, **kwargs) -> None: super().__init__(**kwargs) - self.me: typing.Union[MockMember, MockUser] = kwargs.get("me", MockMember()) self.bot: MockBot = kwargs.get("bot", MockBot()) - self.guild: typing.Optional[MockGuild] = kwargs.get("guild", MockGuild()) + self.guild: typing.Optional[MockGuild] = kwargs.get( + "guild", MockGuild(me=MockMember(id=self.bot.user.id, bot=True)) + ) self.author: typing.Union[MockMember, MockUser] = kwargs.get("author", MockMember()) self.channel: typing.Union[MockTextChannel, MockThread, MockDMChannel] = kwargs.get( "channel", MockTextChannel(guild=self.guild) @@ -556,6 +584,12 @@ def __init__(self, **kwargs) -> None: ) self.invoked_from_error_handler = kwargs.get("invoked_from_error_handler", False) + @property + def me(self) -> typing.Union[MockMember, MockClientUser]: + """Similar to MockGuild.me except will return the class MockClientUser if guild is None.""" + # bot.user will never be None at this point. + return self.guild.me if self.guild is not None else self.bot.user + attachment_instance = discord.Attachment(data=unittest.mock.MagicMock(id=1), state=unittest.mock.MagicMock()) diff --git a/tests/test_mocks.py b/tests/test_mocks.py index 4c4da4d0..768215e8 100644 --- a/tests/test_mocks.py +++ b/tests/test_mocks.py @@ -175,6 +175,10 @@ def test_mock_context_default_initialization(self): assert context.message.channel is context.channel assert context.channel.guild is context.guild + # ensure the me instance is of the right type and shtuff. + assert isinstance(context.me, test_mocks.MockMember) + assert context.me is context.guild.me + @pytest.mark.parametrize( ["mock", "valid_attribute"], [ @@ -191,7 +195,7 @@ def test_mocks_allows_access_to_attributes_part_of_spec(self, mock, valid_attrib """Accessing attributes that are valid for the objects they mock should succeed.""" try: getattr(mock, valid_attribute) - except AttributeError: + except AttributeError: # pragma: nocover msg = f"accessing valid attribute `{valid_attribute}` raised an AttributeError" pytest.fail(msg) @@ -229,7 +233,7 @@ def test_mocks_use_mention_when_provided_as_kwarg(self, mock_type, provided_ment def test_create_test_on_mock_bot_closes_passed_coroutine(self): """`bot.loop.create_task` should close the passed coroutine object to prevent warnings.""" - async def dementati(): + async def dementati(): # pragma: nocover """Dummy coroutine for testing purposes.""" pass @@ -260,6 +264,10 @@ class MockHemlock(unittest.mock.MagicMock, test_mocks.ColourMixin): assert hemlock.colour == 1 assert hemlock.colour == hemlock.color + hemlock.accent_color = 123 + assert hemlock.accent_colour == 123 + assert hemlock.accent_colour == hemlock.accent_color + def test_hashable_mixin_hash_returns_id(self): """Test the HashableMixing uses the id attribute for hashing.""" From 2c7de885ee1f247a74eb9fcedba21dcba67f67ae Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Wed, 20 Oct 2021 21:53:08 -0400 Subject: [PATCH 49/83] tests: start scanning tests coverage --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index d32d6502..ab59efdc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,7 +63,7 @@ build-backend = "poetry.core.masonry.api" [tool.coverage.run] branch = true -source_pkgs = ["modmail"] +source_pkgs = ['modmail', 'tests'] omit = ["modmail/plugins/**.*"] [tool.pytest.ini_options] From 27fff2d2bab5593830fa7a9dff74ec77eafab6da Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sat, 23 Oct 2021 01:53:31 -0400 Subject: [PATCH 50/83] fix: restructure tests to have modmail tests in modmail folder --- tests/{ => modmail}/test_bot.py | 0 tests/{ => modmail}/test_logs.py | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename tests/{ => modmail}/test_bot.py (100%) rename tests/{ => modmail}/test_logs.py (100%) diff --git a/tests/test_bot.py b/tests/modmail/test_bot.py similarity index 100% rename from tests/test_bot.py rename to tests/modmail/test_bot.py diff --git a/tests/test_logs.py b/tests/modmail/test_logs.py similarity index 100% rename from tests/test_logs.py rename to tests/modmail/test_logs.py From 77ec504daf9eabb884cb68f691fef8ea2b422f5d Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 24 Oct 2021 01:26:05 -0400 Subject: [PATCH 51/83] fix: remove logger changes from tests --- tests/mocks.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/mocks.py b/tests/mocks.py index 1ef9c167..59bdac23 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -46,16 +46,6 @@ import modmail.bot -for logger in logging.Logger.manager.loggerDict.values(): - # Set all loggers to CRITICAL by default to prevent screen clutter during testing - - if not isinstance(logger, logging.Logger): - # There might be some logging.PlaceHolder objects in there - continue - - logger.setLevel(logging.CRITICAL) - - class HashableMixin(discord.mixins.EqualityComparable): """ Mixin that provides similar hashing and equality functionality as discord.py's `Hashable` mixin. From 2cf6df5259aeb84aad05ea8c8f140e9b6c93195a Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 24 Oct 2021 01:43:59 -0400 Subject: [PATCH 52/83] chore: generate realistic snowflakes --- tests/mocks.py | 65 ++++++++++++++++++++++++++++----------------- tests/test_mocks.py | 17 +++--------- 2 files changed, 44 insertions(+), 38 deletions(-) diff --git a/tests/mocks.py b/tests/mocks.py index 59bdac23..ab466f37 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -39,33 +39,43 @@ from typing import TYPE_CHECKING, Iterable, Optional import aiohttp +import arrow import discord import discord.mixins from discord.ext.commands import Context +from discord.utils import time_snowflake import modmail.bot +_snowflake_count = itertools.count(1) + + +def generate_realistic_id() -> int: + """Generate realistic id, based from the current time.""" + return discord.utils.time_snowflake(arrow.utcnow()) + next(_snowflake_count) + + +class GenerateID: + """Class to be able to use next() to generate new ids.""" + + def __next__(self) -> int: + return generate_realistic_id() + + class HashableMixin(discord.mixins.EqualityComparable): """ Mixin that provides similar hashing and equality functionality as discord.py's `Hashable` mixin. Given that most of our features need the created_at function to work, we are typically using full fake discord ids, and so we still bitshift the id like Dpy does. - - However, given that the hash of 4>>22 and 5>>22 is the same, we check if the number is above 1<<22. - If so, we hash it. This could result in some weird behavior with the hash of - 1<<22 + 1 equaling 1. """ if TYPE_CHECKING: # pragma: nocover id: int def __hash__(self): - if self.id < 1 << 22: - return self.id - else: - return self.id >> 22 + return self.id >> 22 class ColourMixin: @@ -106,7 +116,7 @@ class method `spec_set` can be overwritten with the object that should be uses a """ child_mock_type = unittest.mock.MagicMock - discord_id = itertools.count(0) + discord_id = GenerateID() spec_set = None additional_spec_asyncs = None @@ -157,7 +167,7 @@ def _get_child_mock(self, **kw): # Create a guild instance to get a realistic Mock of `discord.Guild` guild_data = { - "id": 1, + "id": generate_realistic_id(), "name": "guild", "region": "Europe", "verification_level": 2, @@ -167,13 +177,13 @@ def _get_child_mock(self, **kw): "banner": "banner.png", "mfa_level": 1, "splash": "splash.png", - "system_channel_id": 464033278631084042, + "system_channel_id": generate_realistic_id(), "description": "mocking is fun", "max_presences": 10_000, "max_members": 100_000, "preferred_locale": "UTC", "owner_id": 1, - "afk_channel_id": 464033278631084042, + "afk_channel_id": generate_realistic_id(), } guild_instance = discord.Guild(data=guild_data, state=unittest.mock.MagicMock()) @@ -217,7 +227,7 @@ def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None # Create a Role instance to get a realistic Mock of `discord.Role` -role_data = {"name": "role", "id": 1} +role_data = {"name": "role", "id": generate_realistic_id()} role_instance = discord.Role(guild=guild_instance, state=unittest.mock.MagicMock(), data=role_data) @@ -380,14 +390,14 @@ def __init__(self, **kwargs) -> None: # Create a TextChannel instance to get a realistic MagicMock of `discord.TextChannel` channel_data = { - "id": 1, + "id": generate_realistic_id(), "type": "TextChannel", "name": "channel", - "parent_id": 1234567890, + "parent_id": generate_realistic_id(), "topic": "topic", "position": 1, "nsfw": False, - "last_message_id": 1, + "last_message_id": generate_realistic_id(), } state = unittest.mock.MagicMock() guild = unittest.mock.MagicMock() @@ -436,7 +446,10 @@ def __init__(self, **kwargs) -> None: # Create data for the DMChannel instance state = unittest.mock.MagicMock() me = unittest.mock.MagicMock() -dm_channel_data = {"id": 1, "recipients": [unittest.mock.MagicMock()]} +dm_channel_data = { + "id": generate_realistic_id(), + "recipients": [unittest.mock.MagicMock()], +} dm_channel_instance = discord.DMChannel(me=me, state=state, data=dm_channel_data) @@ -457,7 +470,7 @@ def __init__(self, **kwargs) -> None: # Create CategoryChannel instance to get a realistic MagicMock of `discord.CategoryChannel` category_channel_data = { - "id": 1, + "id": generate_realistic_id(), "type": discord.ChannelType.category, "name": "category", "position": 1, @@ -491,9 +504,9 @@ def __init__(self, **kwargs) -> None: "archive_timestamp": "2021-10-17T20:35:48.058121+00:00", } thread_data = { - "id": "898070829617799179", - "parent_id": "898070393619902544", - "owner_id": "717983911824588862", + "id": generate_realistic_id(), + "parent_id": generate_realistic_id(), + "owner_id": generate_realistic_id(), "name": "user-0005", "type": discord.ChannelType.public_thread, "last_message_id": None, @@ -524,8 +537,8 @@ def __init__(self, **kwargs) -> None: # Create a Message instance to get a realistic MagicMock of `discord.Message` message_data = { - "id": 1, - "webhook_id": 898069816622067752, + "id": generate_realistic_id(), + "webhook_id": generate_realistic_id(), "attachments": [], "embeds": [], "application": "Discord Modmail", @@ -581,7 +594,9 @@ def me(self) -> typing.Union[MockMember, MockClientUser]: return self.guild.me if self.guild is not None else self.bot.user -attachment_instance = discord.Attachment(data=unittest.mock.MagicMock(id=1), state=unittest.mock.MagicMock()) +attachment_instance = discord.Attachment( + data=unittest.mock.MagicMock(id=generate_realistic_id()), state=unittest.mock.MagicMock() +) class MockAttachment(CustomMockMixin, unittest.mock.MagicMock): @@ -612,7 +627,7 @@ def __init__(self, **kwargs) -> None: self.channel = kwargs.get("channel", MockTextChannel()) -emoji_data = {"require_colons": True, "managed": True, "id": 1, "name": "hyperlemon"} +emoji_data = {"require_colons": True, "managed": True, "id": generate_realistic_id(), "name": "hyperlemon"} emoji_instance = discord.Emoji(guild=MockGuild(), state=unittest.mock.MagicMock(), data=emoji_data) diff --git a/tests/test_mocks.py b/tests/test_mocks.py index 768215e8..99ef9223 100644 --- a/tests/test_mocks.py +++ b/tests/test_mocks.py @@ -33,6 +33,7 @@ import asyncio import unittest.mock +import arrow import discord import discord.ext.commands import pytest @@ -271,16 +272,6 @@ class MockHemlock(unittest.mock.MagicMock, test_mocks.ColourMixin): def test_hashable_mixin_hash_returns_id(self): """Test the HashableMixing uses the id attribute for hashing.""" - class MockScragly(unittest.mock.Mock, test_mocks.HashableMixin): - pass - - scragly = MockScragly() - scragly.id = 10 - assert hash(scragly) == scragly.id - - def test_hashable_mixin_hash_returns_id_bitshift(self): - """Test the HashableMixing uses the id attribute for hashing when above 1<<22.""" - class MockScragly(unittest.mock.Mock, test_mocks.HashableMixin): pass @@ -322,9 +313,9 @@ class MockScragly(test_mocks.HashableMixin): @pytest.mark.parametrize(["mock_cls"], [[x] for x in hashable_mocks]) def test_mock_class_with_hashable_mixin_uses_id_for_hashing(self, mock_cls): - """Test if the MagicMock subclasses that implement the HashableMixin use id for hash.""" - instance = mock_cls(id=100) - assert hash(instance) == instance.id + """Test if the MagicMock subclasses that implement the HashableMixin use id bitshift for hash.""" + instance = mock_cls(id=100 << 22) + assert hash(instance) == instance.id >> 22 @pytest.mark.parametrize(["mock_class"], [[x] for x in hashable_mocks]) def test_mock_class_with_hashable_mixin_uses_id_for_equality(self, mock_class): From cb9f6b914dac85c379085660209c63baabf42034 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 24 Oct 2021 01:52:04 -0400 Subject: [PATCH 53/83] chore: don't use the same variable name --- tests/mocks.py | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/tests/mocks.py b/tests/mocks.py index ab466f37..30d89ecc 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -271,8 +271,7 @@ def __ge__(self, other): # Create a Member instance to get a realistic Mock of `discord.Member` member_data = {"user": "lemon", "roles": [1]} -state_mock = unittest.mock.MagicMock() -member_instance = discord.Member(data=member_data, guild=guild_instance, state=state_mock) +member_instance = discord.Member(data=member_data, guild=guild_instance, state=unittest.mock.MagicMock()) class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): @@ -399,12 +398,14 @@ def __init__(self, **kwargs) -> None: "nsfw": False, "last_message_id": generate_realistic_id(), } -state = unittest.mock.MagicMock() -guild = unittest.mock.MagicMock() -text_channel_instance = discord.TextChannel(state=state, guild=guild, data=channel_data) +text_channel_instance = discord.TextChannel( + state=unittest.mock.MagicMock(), guild=unittest.mock.MagicMock(), data=channel_data +) channel_data["type"] = "VoiceChannel" -voice_channel_instance = discord.VoiceChannel(state=state, guild=guild, data=channel_data) +voice_channel_instance = discord.VoiceChannel( + state=unittest.mock.MagicMock(), guild=unittest.mock.MagicMock(), data=channel_data +) class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): @@ -444,13 +445,13 @@ def __init__(self, **kwargs) -> None: # Create data for the DMChannel instance -state = unittest.mock.MagicMock() -me = unittest.mock.MagicMock() dm_channel_data = { "id": generate_realistic_id(), "recipients": [unittest.mock.MagicMock()], } -dm_channel_instance = discord.DMChannel(me=me, state=state, data=dm_channel_data) +dm_channel_instance = discord.DMChannel( + me=unittest.mock.MagicMock(), state=unittest.mock.MagicMock(), data=dm_channel_data +) class MockDMChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): @@ -476,9 +477,9 @@ def __init__(self, **kwargs) -> None: "position": 1, } -state = unittest.mock.MagicMock() -guild = unittest.mock.MagicMock() -category_channel_instance = discord.CategoryChannel(state=state, guild=guild, data=category_channel_data) +category_channel_instance = discord.CategoryChannel( + state=unittest.mock.MagicMock(), guild=unittest.mock.MagicMock(), data=category_channel_data +) class MockCategoryChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): @@ -515,9 +516,9 @@ def __init__(self, **kwargs) -> None: "thread_metadata": thread_metadata, } -state = unittest.mock.MagicMock() -guild = unittest.mock.MagicMock() -thread_instance = discord.Thread(state=state, guild=guild, data=thread_data) +thread_instance = discord.Thread( + state=unittest.mock.MagicMock(), guild=unittest.mock.MagicMock(), data=thread_data +) class MockThread(CustomMockMixin, unittest.mock.Mock, HashableMixin): @@ -552,9 +553,9 @@ def __init__(self, **kwargs) -> None: "content": "content", "nonce": None, } -state = unittest.mock.MagicMock() -channel = unittest.mock.MagicMock() -message_instance = discord.Message(state=state, channel=channel, data=message_data) +message_instance = discord.Message( + state=unittest.mock.MagicMock(), channel=unittest.mock.MagicMock(), data=message_data +) # Create a Context instance to get a realistic MagicMock of `discord.ext.commands.Context` From f6802fefbeec75765eae694750d72d17ce5e219e Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 24 Oct 2021 01:53:33 -0400 Subject: [PATCH 54/83] chore: use SCREAMING_SNAKE_CASE --- tests/test_mocks.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/test_mocks.py b/tests/test_mocks.py index 99ef9223..ca760112 100644 --- a/tests/test_mocks.py +++ b/tests/test_mocks.py @@ -247,8 +247,7 @@ async def dementati(): # pragma: nocover assert error.match("cannot reuse already awaited coroutine") -hashable_mocks = (test_mocks.MockRole, test_mocks.MockMember, test_mocks.MockGuild) -print([[x] for x in hashable_mocks]) +HASHABLE_MOCKS = (test_mocks.MockRole, test_mocks.MockMember, test_mocks.MockGuild) class TestMockObjects: @@ -311,13 +310,13 @@ class MockScragly(test_mocks.HashableMixin): assert scragly != python assert (scragly != eevee) is False - @pytest.mark.parametrize(["mock_cls"], [[x] for x in hashable_mocks]) + @pytest.mark.parametrize(["mock_cls"], [[x] for x in HASHABLE_MOCKS]) def test_mock_class_with_hashable_mixin_uses_id_for_hashing(self, mock_cls): """Test if the MagicMock subclasses that implement the HashableMixin use id bitshift for hash.""" instance = mock_cls(id=100 << 22) assert hash(instance) == instance.id >> 22 - @pytest.mark.parametrize(["mock_class"], [[x] for x in hashable_mocks]) + @pytest.mark.parametrize(["mock_class"], [[x] for x in HASHABLE_MOCKS]) def test_mock_class_with_hashable_mixin_uses_id_for_equality(self, mock_class): """Test if MagicMocks that implement the HashableMixin use id for equality comparisons.""" instance_one = mock_class() @@ -331,7 +330,7 @@ def test_mock_class_with_hashable_mixin_uses_id_for_equality(self, mock_class): assert instance_one == instance_two assert (instance_one == instance_three) is False - @pytest.mark.parametrize(["mock_class"], [[x] for x in hashable_mocks]) + @pytest.mark.parametrize(["mock_class"], [[x] for x in HASHABLE_MOCKS]) def test_mock_class_with_hashable_mixin_uses_id_for_nonequality(self, mock_class): """Test if MagicMocks that implement HashableMixin use id for nonequality comparisons.""" instance_one = mock_class() From 3f1e72548cf69c52cd1a809be81846d67e99333e Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 24 Oct 2021 20:30:23 -0400 Subject: [PATCH 55/83] chore: remove unnecessary imports, fix typo, add missing commas --- tests/mocks.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/mocks.py b/tests/mocks.py index 30d89ecc..74b1cf5f 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -33,7 +33,6 @@ import asyncio import collections import itertools -import logging import typing import unittest.mock from typing import TYPE_CHECKING, Iterable, Optional @@ -43,7 +42,6 @@ import discord import discord.mixins from discord.ext.commands import Context -from discord.utils import time_snowflake import modmail.bot @@ -150,8 +148,7 @@ def _get_child_mock(self, **kw): if _new_name in self.__dict__["_spec_asyncs"]: return unittest.mock.AsyncMock(**kw) - _type = type(self) - if issubclass(_type, unittest.mock.MagicMock) and _new_name in unittest.mock._async_method_magics: + if isinstance(self, unittest.mock.MagicMock) and _new_name in unittest.mock._async_method_magics: # Any asynchronous magic becomes an AsyncMock klass = unittest.mock.AsyncMock else: @@ -171,7 +168,7 @@ def _get_child_mock(self, **kw): "name": "guild", "region": "Europe", "verification_level": 2, - "default_notications": 1, + "default_notifications": 1, "afk_timeout": 100, "icon": "icon.png", "banner": "banner.png", @@ -212,7 +209,6 @@ class MockGuild(CustomMockMixin, unittest.mock.Mock, HashableMixin): >>> isinstance(guild, discord.Guild) True - For more info, see the `Mocking` section in `tests/README.md`. """ spec_set = guild_instance @@ -227,7 +223,10 @@ def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None # Create a Role instance to get a realistic Mock of `discord.Role` -role_data = {"name": "role", "id": generate_realistic_id()} +role_data = { + "name": "role", + "id": generate_realistic_id(), +} role_instance = discord.Role(guild=guild_instance, state=unittest.mock.MagicMock(), data=role_data) @@ -270,7 +269,10 @@ def __ge__(self, other): # Create a Member instance to get a realistic Mock of `discord.Member` -member_data = {"user": "lemon", "roles": [1]} +member_data = { + "user": "lemon", + "roles": [1], +} member_instance = discord.Member(data=member_data, guild=guild_instance, state=unittest.mock.MagicMock()) From 22b63d011e334fba61b47ecf3c60379ce933dbd3 Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Mon, 25 Oct 2021 05:57:32 +0530 Subject: [PATCH 56/83] Add workflow to regenerate requirements after dependabots PR --- .../update_requirements_dependabot.yml | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 .github/workflows/update_requirements_dependabot.yml diff --git a/.github/workflows/update_requirements_dependabot.yml b/.github/workflows/update_requirements_dependabot.yml new file mode 100644 index 00000000..b44eeb7b --- /dev/null +++ b/.github/workflows/update_requirements_dependabot.yml @@ -0,0 +1,40 @@ +name: Update `requirements.txt` + +on: + pull_request: + +jobs: + regenerate-requirements: + runs-on: ubuntu-latest + if: github.event.issue.user.login == 'dependabot[bot]' + steps: + # Checks out the repository in the current folder. + - name: Checkout repository + uses: actions/checkout@v2 + + # Set up the right version of Python + - uses: actions/checkout@v2 + - name: Set up Python + uses: actions/setup-python@v2 + with: + python-version: '3.9' + + # Install the required dependencies for using the feature, we aren't using cache here + # as the number is relatively small and having cache would be complete redundancy. + - name: Install dependencies using pip + run: | + pip install tomli + + # Run the script for regenerating requirements.txt so that the dependancies + # updated by dependabot are reflected in `requirements.txt` + - name: Regenerate `requirements.txt` if opened PR is made by dependabot + run: | + python -m scripts.export_requirements + + - name: Commit Changes + run: | + git config user.name 'github-actions[bot]' + git config user.email 'github-actions[bot]@users.noreply.github.com' + git add . + git commit --amend --no-edit + git push --force origin ${GITHUB_REF##*/} From 7b35353a2f4600b0f99d0c8f69b198974e55d90a Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Mon, 25 Oct 2021 01:50:12 -0400 Subject: [PATCH 57/83] tests: mock methods of mocked objects to return mocked counterparts --- tests/mocks.py | 195 ++++++++++++++++++++++++++++++++++++++++++-- tests/test_mocks.py | 47 +++++++++++ 2 files changed, 237 insertions(+), 5 deletions(-) diff --git a/tests/mocks.py b/tests/mocks.py index 74b1cf5f..accfa6d3 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -3,6 +3,9 @@ Slight modifications have been made to support our bot. +Additional modifications have been made to mocked class method side_effects +in order to return the proper mock type, if it exists. + Original Source: https://github.com/python-discord/bot/blob/d183d03fa2939bebaac3da49646449fdd4d00e6c/tests/helpers.py# noqa: E501 @@ -40,14 +43,39 @@ import aiohttp import arrow import discord +import discord.ext.commands import discord.mixins -from discord.ext.commands import Context import modmail.bot _snowflake_count = itertools.count(1) +__all__ = [ + "generate_realistic_id", + "HashableMixin", + "CustomMockMixin", + "ColourMixin", + "MockAsyncWebhook", + "MockAttachment", + "MockBot", + "MockCategoryChannel", + "MockContext", + "MockClientUser", + "MockDMChannel", + "MockEmoji", + "MockGuild", + "MockMember", + "MockMessage", + "MockPartialEmoji", + "MockReaction", + "MockRole", + "MockTextChannel", + "MockThread", + "MockUser", + "MockVoiceChannel", +] + def generate_realistic_id() -> int: """Generate realistic id, based from the current time.""" @@ -98,6 +126,98 @@ def accent_color(self, color: discord.Colour) -> None: self.accent_colour = color +def generate_mock_attachment(*args, **kwargs): + return MockAttachment() + + +def generate_mock_bot(*args, **kwargs): + return MockBot() + + +def generate_mock_category_channel(*args, **kwargs): + return MockCategoryChannel() + + +def generate_mock_context(*args, **kwargs): + return MockContext() + + +def generate_mock_client_user(*args, **kwargs): + return MockClientUser() + + +def generate_mock_dm_channel(*args, **kwargs): + return MockDMChannel() + + +def generate_mock_emoji(*args, **kwargs): + return MockEmoji() + + +def generate_mock_guild(*args, **kwargs): + return MockGuild() + + +def generate_mock_member(*args, **kwargs): + return MockMember() + + +def generate_mock_message(content=unittest.mock.DEFAULT, *args, **kwargs): + return MockMessage(content=content) + + +def generate_mock_partial_emoji(*args, **kwargs): + return MockPartialEmoji() + + +def generate_mock_reaction(*args, **kwargs): + return MockReaction() + + +def generate_mock_role(*args, **kwargs): + return MockRole() + + +def generate_mock_text_channel(*args, **kwargs): + return MockTextChannel() + + +def generate_mock_thread(*args, **kwargs): + return MockThread() + + +def generate_mock_user(*args, **kwargs): + return MockUser() + + +def generate_mock_voice_channel(*args, **kwargs): + return MockVoiceChannel() + + +# all of the classes here can be created from a mock object +# the key is the object, and the method is a factory method for creating a new instance +# some of the factories can factory take their input and pass it to the mock object. +COPYABLE_MOCKS = { + discord.Attachment: generate_mock_attachment, + discord.ext.commands.Bot: generate_mock_bot, + discord.CategoryChannel: generate_mock_category_channel, + discord.ext.commands.Context: generate_mock_context, + discord.ClientUser: generate_mock_client_user, + discord.DMChannel: generate_mock_dm_channel, + discord.Emoji: generate_mock_emoji, + discord.Guild: generate_mock_guild, + discord.Member: generate_mock_member, + discord.Message: generate_mock_message, + discord.PartialEmoji: generate_mock_partial_emoji, + discord.Reaction: generate_mock_reaction, + discord.Role: generate_mock_role, + discord.TextChannel: generate_mock_text_channel, + discord.Thread: generate_mock_thread, + discord.User: generate_mock_user, + discord.VoiceChannel: generate_mock_voice_channel, +} + + class CustomMockMixin: """ Provides common functionality for our custom Mock types. @@ -130,6 +250,66 @@ def __init__(self, **kwargs): if name: self.name = name + # make all of the methods return the proper mock type + # configure mock + mock_config = {} + for attr in dir(self.spec_set): + if attr.startswith("__") and attr.endswith("__"): + # ignore all magic attributes + continue + try: + attr = getattr(self.spec_set, attr) + except AttributeError: + continue + # we only want functions, so we can properly mock their return + if not callable(attr): + continue + + if isinstance(attr, (unittest.mock.Mock, unittest.mock.AsyncMock)): + # skip all mocks + continue + + try: + hints = typing.get_type_hints(attr) + except NameError: + hints = attr.__annotations__ + + # fix not typed methods + # this list can be added to as methods are discovered + if attr.__name__ == "send": + hints["return"] = discord.Message + elif self.__class__ == discord.Message and attr.__name__ == "edit": + # set up message editing to return the same object + mock_config[f"{attr.__name__}.return_value"] = self + continue + + if hints.get("return") is None: + continue + + klass = hints["return"] + + if isinstance(klass, str): + klass_name = klass + elif hasattr(klass, "__name__"): + klass_name = klass.__name__ + else: + continue + + method = None + for cls in COPYABLE_MOCKS: + if klass_name == cls.__name__: + method = COPYABLE_MOCKS[cls] + break + + if not method: + continue + + # print(self.__class__, attr.__name__, cls) + + mock_config[f"{attr.__name__}.side_effect"] = method + + self.configure_mock(**mock_config) + def _get_child_mock(self, **kw): """ Overwrite of the `_get_child_mock` method to stop the propagation of our custom mock classes. @@ -318,8 +498,10 @@ class MockUser(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): spec_set = user_instance def __init__(self, **kwargs) -> None: - default_kwargs = {"name": "user", "id": next(self.discord_id), "bot": False} - super().__init__(**collections.ChainMap(kwargs, default_kwargs)) + kwargs["name"] = kwargs.get("name", "user") + kwargs["id"] = kwargs.get("id", next(self.discord_id)) + kwargs["bot"] = kwargs.get("bot", False) + super().__init__(**kwargs) if "mention" not in kwargs: self.mention = f"@{self.name}" @@ -561,11 +743,13 @@ def __init__(self, **kwargs) -> None: # Create a Context instance to get a realistic MagicMock of `discord.ext.commands.Context` -context_instance = Context(message=unittest.mock.MagicMock(), prefix="$", bot=MockBot(), view=None) +context_instance = discord.ext.commands.Context( + message=unittest.mock.MagicMock(), prefix="$", bot=MockBot(), view=None +) context_instance.invoked_from_error_handler = None -class MockContext(CustomMockMixin, unittest.mock.MagicMock): +class MockContext(CustomMockMixin, unittest.mock.NonCallableMagicMock): """ A MagicMock subclass to mock Context objects. @@ -625,6 +809,7 @@ class MockMessage(CustomMockMixin, unittest.mock.MagicMock): def __init__(self, **kwargs) -> None: default_kwargs = {"attachments": []} + kwargs["id"] = kwargs.get("id", generate_realistic_id()) super().__init__(**collections.ChainMap(kwargs, default_kwargs)) self.author = kwargs.get("author", MockMember()) self.channel = kwargs.get("channel", MockTextChannel()) diff --git a/tests/test_mocks.py b/tests/test_mocks.py index ca760112..60621f18 100644 --- a/tests/test_mocks.py +++ b/tests/test_mocks.py @@ -388,3 +388,50 @@ class MyMock(test_mocks.CustomMockMixin, unittest.mock.MagicMock): mock = MyMock() assert isinstance(mock.__aenter__, unittest.mock.AsyncMock) + + +class TestReturnTypes: + """ + Our mocks are designed to automatically return the correct objects based on certain methods. + + Eg, ctx.send should return a message object. + """ + + @pytest.mark.asyncio + async def test_message_edit_returns_self(self): + """Message editing edits the message in place. We should be returning the message.""" + msg = test_mocks.MockMessage() + + new_msg = await msg.edit() + + assert isinstance(new_msg, discord.Message) + + assert msg is new_msg + + @pytest.mark.asyncio + async def test_channel_send_returns_message(self): + """Ensure that channel objects return mocked messages when sending messages.""" + channel = test_mocks.MockTextChannel() + + msg = await channel.send("hi") + + print(type(msg)) + assert isinstance(msg, discord.Message) + + @pytest.mark.asyncio + async def test_message_thread_create_returns_thread(self): + """Thread create methods should return a MockThread.""" + msg = test_mocks.MockMessage() + + thread = await msg.create_thread() + + assert isinstance(thread, discord.Thread) + + @pytest.mark.asyncio + async def test_channel_thread_create_returns_thread(self): + """Thread create methods should return a MockThread.""" + channel = test_mocks.MockTextChannel() + + thread = await channel.create_thread() + + assert isinstance(thread, discord.Thread) From 49297e3c84923923aaa305868ec65733d03ae559 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Tue, 26 Oct 2021 00:31:04 -0400 Subject: [PATCH 58/83] fix: make mocks not callable if their counterpart isn't --- tests/mocks.py | 39 ++++++++++++++++++++++----------------- tests/test_mocks.py | 11 +++++++++++ 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/tests/mocks.py b/tests/mocks.py index accfa6d3..3490118d 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -126,6 +126,10 @@ def accent_color(self, color: discord.Colour) -> None: self.accent_colour = color +def generate_mock_webhook(*args, **kwargs): + return MockAsyncWebhook() + + def generate_mock_attachment(*args, **kwargs): return MockAttachment() @@ -215,6 +219,7 @@ def generate_mock_voice_channel(*args, **kwargs): discord.Thread: generate_mock_thread, discord.User: generate_mock_user, discord.VoiceChannel: generate_mock_voice_channel, + discord.Webhook: generate_mock_webhook, } @@ -365,7 +370,7 @@ def _get_child_mock(self, **kw): guild_instance = discord.Guild(data=guild_data, state=unittest.mock.MagicMock()) -class MockGuild(CustomMockMixin, unittest.mock.Mock, HashableMixin): +class MockGuild(CustomMockMixin, unittest.mock.NonCallableMock, HashableMixin): """ A `Mock` subclass to mock `discord.Guild` objects. @@ -410,7 +415,7 @@ def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None role_instance = discord.Role(guild=guild_instance, state=unittest.mock.MagicMock(), data=role_data) -class MockRole(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): +class MockRole(CustomMockMixin, unittest.mock.NonCallableMock, ColourMixin, HashableMixin): """ A Mock subclass to mock `discord.Role` objects. @@ -456,7 +461,7 @@ def __ge__(self, other): member_instance = discord.Member(data=member_data, guild=guild_instance, state=unittest.mock.MagicMock()) -class MockMember(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): +class MockMember(CustomMockMixin, unittest.mock.NonCallableMock, ColourMixin, HashableMixin): """ A Mock subclass to mock Member objects. @@ -487,7 +492,7 @@ def __init__(self, roles: Optional[Iterable[MockRole]] = None, **kwargs) -> None ) -class MockUser(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): +class MockUser(CustomMockMixin, unittest.mock.NonCallableMock, ColourMixin, HashableMixin): """ A Mock subclass to mock User objects. @@ -515,7 +520,7 @@ def __init__(self, **kwargs) -> None: ) -class MockClientUser(CustomMockMixin, unittest.mock.Mock, ColourMixin, HashableMixin): +class MockClientUser(CustomMockMixin, unittest.mock.NonCallableMock, ColourMixin, HashableMixin): """ A Mock subclass to mock ClientUser objects. @@ -549,7 +554,7 @@ def mock_create_task(coroutine, **kwargs): return loop -class MockBot(CustomMockMixin, unittest.mock.MagicMock): +class MockBot(CustomMockMixin, unittest.mock.NonCallableMock): """ A MagicMock subclass to mock Bot objects. @@ -592,7 +597,7 @@ def __init__(self, **kwargs) -> None: ) -class MockTextChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): +class MockTextChannel(CustomMockMixin, unittest.mock.NonCallableMock, HashableMixin): """ A MagicMock subclass to mock TextChannel objects. @@ -610,7 +615,7 @@ def __init__(self, **kwargs) -> None: self.mention = f"#{self.name}" -class MockVoiceChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): +class MockVoiceChannel(CustomMockMixin, unittest.mock.NonCallableMock, HashableMixin): """ A MagicMock subclass to mock VoiceChannel objects. @@ -638,7 +643,7 @@ def __init__(self, **kwargs) -> None: ) -class MockDMChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): +class MockDMChannel(CustomMockMixin, unittest.mock.NonCallableMock, HashableMixin): """ A MagicMock subclass to mock DMChannel objects. @@ -666,7 +671,7 @@ def __init__(self, **kwargs) -> None: ) -class MockCategoryChannel(CustomMockMixin, unittest.mock.Mock, HashableMixin): +class MockCategoryChannel(CustomMockMixin, unittest.mock.NonCallableMock, HashableMixin): """ A MagicMock subclass to mock CategoryChannel objects. @@ -705,7 +710,7 @@ def __init__(self, **kwargs) -> None: ) -class MockThread(CustomMockMixin, unittest.mock.Mock, HashableMixin): +class MockThread(CustomMockMixin, unittest.mock.NonCallableMock, HashableMixin): """ A MagicMock subclass to mock Thread objects. @@ -786,7 +791,7 @@ def me(self) -> typing.Union[MockMember, MockClientUser]: ) -class MockAttachment(CustomMockMixin, unittest.mock.MagicMock): +class MockAttachment(CustomMockMixin, unittest.mock.NonCallableMagicMock): """ A MagicMock subclass to mock Attachment objects. @@ -797,7 +802,7 @@ class MockAttachment(CustomMockMixin, unittest.mock.MagicMock): spec_set = attachment_instance -class MockMessage(CustomMockMixin, unittest.mock.MagicMock): +class MockMessage(CustomMockMixin, unittest.mock.NonCallableMagicMock): """ A MagicMock subclass to mock Message objects. @@ -819,7 +824,7 @@ def __init__(self, **kwargs) -> None: emoji_instance = discord.Emoji(guild=MockGuild(), state=unittest.mock.MagicMock(), data=emoji_data) -class MockEmoji(CustomMockMixin, unittest.mock.MagicMock): +class MockEmoji(CustomMockMixin, unittest.mock.NonCallableMagicMock): """ A MagicMock subclass to mock Emoji objects. @@ -837,7 +842,7 @@ def __init__(self, **kwargs) -> None: partial_emoji_instance = discord.PartialEmoji(animated=False, name="guido") -class MockPartialEmoji(CustomMockMixin, unittest.mock.MagicMock): +class MockPartialEmoji(CustomMockMixin, unittest.mock.NonCallableMagicMock): """ A MagicMock subclass to mock PartialEmoji objects. @@ -851,7 +856,7 @@ class MockPartialEmoji(CustomMockMixin, unittest.mock.MagicMock): reaction_instance = discord.Reaction(message=MockMessage(), data={"me": True}, emoji=MockEmoji()) -class MockReaction(CustomMockMixin, unittest.mock.MagicMock): +class MockReaction(CustomMockMixin, unittest.mock.NonCallableMagicMock): """ A MagicMock subclass to mock Reaction objects. @@ -877,7 +882,7 @@ def __init__(self, **kwargs) -> None: webhook_instance = discord.Webhook(data=unittest.mock.MagicMock(), session=unittest.mock.MagicMock()) -class MockAsyncWebhook(CustomMockMixin, unittest.mock.MagicMock): +class MockAsyncWebhook(CustomMockMixin, unittest.mock.NonCallableMagicMock): """ A MagicMock subclass to mock Webhook objects using an AsyncWebhookAdapter. diff --git a/tests/test_mocks.py b/tests/test_mocks.py index 60621f18..b61c774d 100644 --- a/tests/test_mocks.py +++ b/tests/test_mocks.py @@ -435,3 +435,14 @@ async def test_channel_thread_create_returns_thread(self): thread = await channel.create_thread() assert isinstance(thread, discord.Thread) + + +class TestMocksNotCallable: + """All discord.py mocks are not callable objects, so the mocks should not be either .""" + + @pytest.mark.parametrize("factory", test_mocks.COPYABLE_MOCKS.values()) + def test_not_callable(self, factory): + """Assert all mocks aren't callable.""" + instance = factory() + with pytest.raises(TypeError, match=f"'{type(instance).__name__}' object is not callable"): + instance() From 4f76928438364b4e810bbd190db498be2e5912ac Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Tue, 26 Oct 2021 04:06:21 -0400 Subject: [PATCH 59/83] minor: don't use so many functions when a lambda would suffice --- tests/mocks.py | 102 +++++++++---------------------------------------- 1 file changed, 17 insertions(+), 85 deletions(-) diff --git a/tests/mocks.py b/tests/mocks.py index 3490118d..aa6a9162 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -126,100 +126,32 @@ def accent_color(self, color: discord.Colour) -> None: self.accent_colour = color -def generate_mock_webhook(*args, **kwargs): - return MockAsyncWebhook() - - -def generate_mock_attachment(*args, **kwargs): - return MockAttachment() - - -def generate_mock_bot(*args, **kwargs): - return MockBot() - - -def generate_mock_category_channel(*args, **kwargs): - return MockCategoryChannel() - - -def generate_mock_context(*args, **kwargs): - return MockContext() - - -def generate_mock_client_user(*args, **kwargs): - return MockClientUser() - - -def generate_mock_dm_channel(*args, **kwargs): - return MockDMChannel() - - -def generate_mock_emoji(*args, **kwargs): - return MockEmoji() - - -def generate_mock_guild(*args, **kwargs): - return MockGuild() - - -def generate_mock_member(*args, **kwargs): - return MockMember() - - def generate_mock_message(content=unittest.mock.DEFAULT, *args, **kwargs): return MockMessage(content=content) -def generate_mock_partial_emoji(*args, **kwargs): - return MockPartialEmoji() - - -def generate_mock_reaction(*args, **kwargs): - return MockReaction() - - -def generate_mock_role(*args, **kwargs): - return MockRole() - - -def generate_mock_text_channel(*args, **kwargs): - return MockTextChannel() - - -def generate_mock_thread(*args, **kwargs): - return MockThread() - - -def generate_mock_user(*args, **kwargs): - return MockUser() - - -def generate_mock_voice_channel(*args, **kwargs): - return MockVoiceChannel() - - # all of the classes here can be created from a mock object # the key is the object, and the method is a factory method for creating a new instance # some of the factories can factory take their input and pass it to the mock object. COPYABLE_MOCKS = { - discord.Attachment: generate_mock_attachment, - discord.ext.commands.Bot: generate_mock_bot, - discord.CategoryChannel: generate_mock_category_channel, - discord.ext.commands.Context: generate_mock_context, - discord.ClientUser: generate_mock_client_user, - discord.DMChannel: generate_mock_dm_channel, - discord.Emoji: generate_mock_emoji, - discord.Guild: generate_mock_guild, - discord.Member: generate_mock_member, + discord.Attachment: lambda *args, **kwargs: MockAttachment(), + discord.ext.commands.Bot: lambda *args, **kwargs: MockBot(), + discord.CategoryChannel: lambda *args, **kwargs: MockCategoryChannel(), + discord.ext.commands.Context: lambda *args, **kwargs: MockContext(), + discord.ClientUser: lambda *args, **kwargs: MockClientUser(), + discord.DMChannel: lambda *args, **kwargs: MockDMChannel(), + discord.Emoji: lambda *args, **kwargs: MockEmoji(), + discord.Guild: lambda *args, **kwargs: MockGuild(), + discord.Member: lambda *args, **kwargs: MockMember(), discord.Message: generate_mock_message, - discord.PartialEmoji: generate_mock_partial_emoji, - discord.Reaction: generate_mock_reaction, - discord.Role: generate_mock_role, - discord.TextChannel: generate_mock_text_channel, - discord.Thread: generate_mock_thread, - discord.User: generate_mock_user, - discord.VoiceChannel: generate_mock_voice_channel, - discord.Webhook: generate_mock_webhook, + discord.PartialEmoji: lambda *args, **kwargs: MockPartialEmoji(), + discord.Reaction: lambda *args, **kwargs: MockReaction(), + discord.Role: lambda *args, **kwargs: MockRole(), + discord.TextChannel: lambda *args, **kwargs: MockTextChannel(), + discord.Thread: lambda *args, **kwargs: MockThread(), + discord.User: lambda *args, **kwargs: MockUser(), + discord.VoiceChannel: lambda *args, **kwargs: MockVoiceChannel(), + discord.Webhook: lambda *args, **kwargs: MockAsyncWebhook(), } From 04d16c0fe6c97154719065ff28222bee95ce3982 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Tue, 26 Oct 2021 18:06:14 -0400 Subject: [PATCH 60/83] chore: don't make unnecessary 'as' import --- tests/test_mocks.py | 142 ++++++++++++++++++++++---------------------- 1 file changed, 71 insertions(+), 71 deletions(-) diff --git a/tests/test_mocks.py b/tests/test_mocks.py index b61c774d..28911707 100644 --- a/tests/test_mocks.py +++ b/tests/test_mocks.py @@ -38,7 +38,7 @@ import discord.ext.commands import pytest -from tests import mocks as test_mocks +from tests import mocks class TestDiscordMocks: @@ -46,7 +46,7 @@ class TestDiscordMocks: def test_mock_role_default_initialization(self): """Test if the default initialization of MockRole results in the correct object.""" - role = test_mocks.MockRole() + role = mocks.MockRole() # The `spec` argument makes sure `isinstance` checks with `discord.Role` pass assert isinstance(role, discord.Role) @@ -57,7 +57,7 @@ def test_mock_role_default_initialization(self): def test_mock_role_alternative_arguments(self): """Test if MockRole initializes with the arguments provided.""" - role = test_mocks.MockRole( + role = mocks.MockRole( name="Admins", id=90210, position=10, @@ -70,7 +70,7 @@ def test_mock_role_alternative_arguments(self): def test_mock_role_accepts_dynamic_arguments(self): """Test if MockRole accepts and sets abitrary keyword arguments.""" - role = test_mocks.MockRole( + role = mocks.MockRole( guild="Dino Man", hoist=True, ) @@ -80,9 +80,9 @@ def test_mock_role_accepts_dynamic_arguments(self): def test_mock_role_uses_position_for_less_than_greater_than(self): """Test if `<` and `>` comparisons for MockRole are based on its position attribute.""" - role_one = test_mocks.MockRole(position=1) - role_two = test_mocks.MockRole(position=2) - role_three = test_mocks.MockRole(position=3) + role_one = mocks.MockRole(position=1) + role_two = mocks.MockRole(position=2) + role_three = mocks.MockRole(position=3) assert role_one < role_two assert role_one < role_three @@ -93,28 +93,28 @@ def test_mock_role_uses_position_for_less_than_greater_than(self): def test_mock_member_default_initialization(self): """Test if the default initialization of Mockmember results in the correct object.""" - member = test_mocks.MockMember() + member = mocks.MockMember() # The `spec` argument makes sure `isinstance` checks with `discord.Member` pass assert isinstance(member, discord.Member) assert member.name == "member" - assert member.roles == [test_mocks.MockRole(name="@everyone", position=1, id=0)] + assert member.roles == [mocks.MockRole(name="@everyone", position=1, id=0)] assert member.mention == "@member" def test_mock_member_alternative_arguments(self): """Test if MockMember initializes with the arguments provided.""" - core_developer = test_mocks.MockRole(name="Core Developer", position=2) - member = test_mocks.MockMember(name="Mark", id=12345, roles=[core_developer]) + core_developer = mocks.MockRole(name="Core Developer", position=2) + member = mocks.MockMember(name="Mark", id=12345, roles=[core_developer]) assert member.name == "Mark" assert member.id == 12345 - assert member.roles == [test_mocks.MockRole(name="@everyone", position=1, id=0), core_developer] + assert member.roles == [mocks.MockRole(name="@everyone", position=1, id=0), core_developer] assert member.mention == "@Mark" def test_mock_member_accepts_dynamic_arguments(self): """Test if MockMember accepts and sets abitrary keyword arguments.""" - member = test_mocks.MockMember( + member = mocks.MockMember( nick="Dino Man", colour=discord.Colour.default(), ) @@ -124,28 +124,28 @@ def test_mock_member_accepts_dynamic_arguments(self): def test_mock_guild_default_initialization(self): """Test if the default initialization of Mockguild results in the correct object.""" - guild = test_mocks.MockGuild() + guild = mocks.MockGuild() # The `spec` argument makes sure `isistance` checks with `discord.Guild` pass assert isinstance(guild, discord.Guild) - assert guild.roles == [test_mocks.MockRole(name="@everyone", position=1, id=0)] + assert guild.roles == [mocks.MockRole(name="@everyone", position=1, id=0)] assert guild.members == [] def test_mock_guild_alternative_arguments(self): """Test if MockGuild initializes with the arguments provided.""" - core_developer = test_mocks.MockRole(name="Core Developer", position=2) - guild = test_mocks.MockGuild( + core_developer = mocks.MockRole(name="Core Developer", position=2) + guild = mocks.MockGuild( roles=[core_developer], - members=[test_mocks.MockMember(id=54321)], + members=[mocks.MockMember(id=54321)], ) - assert guild.roles == [test_mocks.MockRole(name="@everyone", position=1, id=0), core_developer] - assert guild.members == [test_mocks.MockMember(id=54321)] + assert guild.roles == [mocks.MockRole(name="@everyone", position=1, id=0), core_developer] + assert guild.members == [mocks.MockMember(id=54321)] def test_mock_guild_accepts_dynamic_arguments(self): """Test if MockGuild accepts and sets abitrary keyword arguments.""" - guild = test_mocks.MockGuild( + guild = mocks.MockGuild( emojis=(":hyperjoseph:", ":pensive_ela:"), premium_subscription_count=15, ) @@ -155,44 +155,44 @@ def test_mock_guild_accepts_dynamic_arguments(self): def test_mock_bot_default_initialization(self): """Tests if MockBot initializes with the correct values.""" - bot = test_mocks.MockBot() + bot = mocks.MockBot() # The `spec` argument makes sure `isinstance` checks with `discord.ext.commands.Bot` pass assert isinstance(bot, discord.ext.commands.Bot) def test_mock_context_default_initialization(self): """Tests if MockContext initializes with the correct values.""" - context = test_mocks.MockContext() + context = mocks.MockContext() # The `spec` argument makes sure `isinstance` checks with `discord.ext.commands.Context` pass assert isinstance(context, discord.ext.commands.Context) - assert isinstance(context.bot, test_mocks.MockBot) - assert isinstance(context.guild, test_mocks.MockGuild) - assert isinstance(context.author, test_mocks.MockMember) - assert isinstance(context.message, test_mocks.MockMessage) + assert isinstance(context.bot, mocks.MockBot) + assert isinstance(context.guild, mocks.MockGuild) + assert isinstance(context.author, mocks.MockMember) + assert isinstance(context.message, mocks.MockMessage) # ensure that the mocks are the same attributes, like discord.py assert context.message.channel is context.channel assert context.channel.guild is context.guild # ensure the me instance is of the right type and shtuff. - assert isinstance(context.me, test_mocks.MockMember) + assert isinstance(context.me, mocks.MockMember) assert context.me is context.guild.me @pytest.mark.parametrize( ["mock", "valid_attribute"], [ - [test_mocks.MockGuild(), "name"], - [test_mocks.MockRole(), "hoist"], - [test_mocks.MockMember(), "display_name"], - [test_mocks.MockBot(), "user"], - [test_mocks.MockContext(), "invoked_with"], - [test_mocks.MockTextChannel(), "last_message"], - [test_mocks.MockMessage(), "mention_everyone"], + [mocks.MockGuild(), "name"], + [mocks.MockRole(), "hoist"], + [mocks.MockMember(), "display_name"], + [mocks.MockBot(), "user"], + [mocks.MockContext(), "invoked_with"], + [mocks.MockTextChannel(), "last_message"], + [mocks.MockMessage(), "mention_everyone"], ], ) - def test_mocks_allows_access_to_attributes_part_of_spec(self, mock, valid_attribute: str): + def mocks_allows_access_to_attributes_part_of_spec(self, mock, valid_attribute: str): """Accessing attributes that are valid for the objects they mock should succeed.""" try: getattr(mock, valid_attribute) @@ -203,16 +203,16 @@ def test_mocks_allows_access_to_attributes_part_of_spec(self, mock, valid_attrib @pytest.mark.parametrize( ["mock"], [ - [test_mocks.MockGuild()], - [test_mocks.MockRole()], - [test_mocks.MockMember()], - [test_mocks.MockBot()], - [test_mocks.MockContext()], - [test_mocks.MockTextChannel()], - [test_mocks.MockMessage()], + [mocks.MockGuild()], + [mocks.MockRole()], + [mocks.MockMember()], + [mocks.MockBot()], + [mocks.MockContext()], + [mocks.MockTextChannel()], + [mocks.MockMessage()], ], ) - def test_mocks_rejects_access_to_attributes_not_part_of_spec(self, mock): + def mocks_rejects_access_to_attributes_not_part_of_spec(self, mock): """Accessing attributes that are invalid for the objects they mock should fail.""" with pytest.raises(AttributeError): mock.the_cake_is_a_lie @@ -220,13 +220,13 @@ def test_mocks_rejects_access_to_attributes_not_part_of_spec(self, mock): @pytest.mark.parametrize( ["mock_type", "provided_mention"], [ - [test_mocks.MockRole, "role mention"], - [test_mocks.MockMember, "member mention"], - [test_mocks.MockTextChannel, "channel mention"], - [test_mocks.MockUser, "user mention"], + [mocks.MockRole, "role mention"], + [mocks.MockMember, "member mention"], + [mocks.MockTextChannel, "channel mention"], + [mocks.MockUser, "user mention"], ], ) - def test_mocks_use_mention_when_provided_as_kwarg(self, mock_type, provided_mention: str): + def mocks_use_mention_when_provided_as_kwarg(self, mock_type, provided_mention: str): """The mock should use the passed `mention` instead of the default one if present.""" mock = mock_type(mention=provided_mention) assert mock.mention == provided_mention @@ -240,14 +240,14 @@ async def dementati(): # pragma: nocover coroutine_object = dementati() - bot = test_mocks.MockBot() + bot = mocks.MockBot() bot.loop.create_task(coroutine_object) with pytest.raises(RuntimeError) as error: asyncio.run(coroutine_object) assert error.match("cannot reuse already awaited coroutine") -HASHABLE_MOCKS = (test_mocks.MockRole, test_mocks.MockMember, test_mocks.MockGuild) +HASHABLE_MOCKS = (mocks.MockRole, mocks.MockMember, mocks.MockGuild) class TestMockObjects: @@ -256,7 +256,7 @@ class TestMockObjects: def test_colour_mixin(self): """Test if the ColourMixin adds aliasing of color -> colour for child classes.""" - class MockHemlock(unittest.mock.MagicMock, test_mocks.ColourMixin): + class MockHemlock(unittest.mock.MagicMock, mocks.ColourMixin): pass hemlock = MockHemlock() @@ -271,7 +271,7 @@ class MockHemlock(unittest.mock.MagicMock, test_mocks.ColourMixin): def test_hashable_mixin_hash_returns_id(self): """Test the HashableMixing uses the id attribute for hashing.""" - class MockScragly(unittest.mock.Mock, test_mocks.HashableMixin): + class MockScragly(unittest.mock.Mock, mocks.HashableMixin): pass scragly = MockScragly() @@ -281,7 +281,7 @@ class MockScragly(unittest.mock.Mock, test_mocks.HashableMixin): def test_hashable_mixin_uses_id_for_equality_comparison(self): """Test the HashableMixing uses the id attribute for equal comparison.""" - class MockScragly(test_mocks.HashableMixin): + class MockScragly(mocks.HashableMixin): pass scragly = MockScragly() @@ -297,7 +297,7 @@ class MockScragly(test_mocks.HashableMixin): def test_hashable_mixin_uses_id_for_nonequality_comparison(self): """Test if the HashableMixing uses the id attribute for non-equal comparison.""" - class MockScragly(test_mocks.HashableMixin): + class MockScragly(mocks.HashableMixin): pass scragly = MockScragly() @@ -347,7 +347,7 @@ def test_mock_class_with_hashable_mixin_uses_id_for_nonequality(self, mock_class def test_custom_mock_mixin_accepts_mock_seal(self): """The `CustomMockMixin` should support `unittest.mock.seal`.""" - class MyMock(test_mocks.CustomMockMixin, unittest.mock.MagicMock): + class MyMock(mocks.CustomMockMixin, unittest.mock.MagicMock): child_mock_type = unittest.mock.MagicMock pass @@ -362,15 +362,15 @@ class MyMock(test_mocks.CustomMockMixin, unittest.mock.MagicMock): @pytest.mark.parametrize( ["mock_type", "valid_attribute"], [ - (test_mocks.MockGuild, "region"), - (test_mocks.MockRole, "mentionable"), - (test_mocks.MockMember, "display_name"), - (test_mocks.MockBot, "owner_id"), - (test_mocks.MockContext, "command_failed"), - (test_mocks.MockMessage, "mention_everyone"), - (test_mocks.MockEmoji, "managed"), - (test_mocks.MockPartialEmoji, "url"), - (test_mocks.MockReaction, "me"), + (mocks.MockGuild, "region"), + (mocks.MockRole, "mentionable"), + (mocks.MockMember, "display_name"), + (mocks.MockBot, "owner_id"), + (mocks.MockContext, "command_failed"), + (mocks.MockMessage, "mention_everyone"), + (mocks.MockEmoji, "managed"), + (mocks.MockPartialEmoji, "url"), + (mocks.MockReaction, "me"), ], ) def test_spec_propagation_of_mock_subclasses(self, mock_type, valid_attribute: str): @@ -383,7 +383,7 @@ def test_spec_propagation_of_mock_subclasses(self, mock_type, valid_attribute: s def test_custom_mock_mixin_mocks_async_magic_methods_with_async_mock(self): """The CustomMockMixin should mock async magic methods with an AsyncMock.""" - class MyMock(test_mocks.CustomMockMixin, unittest.mock.MagicMock): + class MyMock(mocks.CustomMockMixin, unittest.mock.MagicMock): pass mock = MyMock() @@ -400,7 +400,7 @@ class TestReturnTypes: @pytest.mark.asyncio async def test_message_edit_returns_self(self): """Message editing edits the message in place. We should be returning the message.""" - msg = test_mocks.MockMessage() + msg = mocks.MockMessage() new_msg = await msg.edit() @@ -411,7 +411,7 @@ async def test_message_edit_returns_self(self): @pytest.mark.asyncio async def test_channel_send_returns_message(self): """Ensure that channel objects return mocked messages when sending messages.""" - channel = test_mocks.MockTextChannel() + channel = mocks.MockTextChannel() msg = await channel.send("hi") @@ -421,7 +421,7 @@ async def test_channel_send_returns_message(self): @pytest.mark.asyncio async def test_message_thread_create_returns_thread(self): """Thread create methods should return a MockThread.""" - msg = test_mocks.MockMessage() + msg = mocks.MockMessage() thread = await msg.create_thread() @@ -430,7 +430,7 @@ async def test_message_thread_create_returns_thread(self): @pytest.mark.asyncio async def test_channel_thread_create_returns_thread(self): """Thread create methods should return a MockThread.""" - channel = test_mocks.MockTextChannel() + channel = mocks.MockTextChannel() thread = await channel.create_thread() @@ -440,7 +440,7 @@ async def test_channel_thread_create_returns_thread(self): class TestMocksNotCallable: """All discord.py mocks are not callable objects, so the mocks should not be either .""" - @pytest.mark.parametrize("factory", test_mocks.COPYABLE_MOCKS.values()) + @pytest.mark.parametrize("factory", mocks.COPYABLE_MOCKS.values()) def test_not_callable(self, factory): """Assert all mocks aren't callable.""" instance = factory() From 33122cfde1b951a1f611ed1b2bf1fe519269ad16 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Tue, 26 Oct 2021 18:25:48 -0400 Subject: [PATCH 61/83] chore: allow generating snowflakes from a provided time --- tests/mocks.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/mocks.py b/tests/mocks.py index aa6a9162..4f587896 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -35,6 +35,7 @@ import asyncio import collections +import datetime import itertools import typing import unittest.mock @@ -77,9 +78,15 @@ ] -def generate_realistic_id() -> int: - """Generate realistic id, based from the current time.""" - return discord.utils.time_snowflake(arrow.utcnow()) + next(_snowflake_count) +def generate_realistic_id(time: typing.Union[arrow.Arrow, datetime.datetime] = None, /) -> int: + """ + Generate realistic id, based from the current time. + + If a time is provided, this will use that time for the time generation. + """ + if time is None: + time = arrow.utcnow() + return discord.utils.time_snowflake(time) + next(_snowflake_count) class GenerateID: From 8db04c45300b3576789be1e736bae9851b802af0 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Tue, 26 Oct 2021 21:46:23 -0400 Subject: [PATCH 62/83] cleanup: use pytest parameterize to dedupe some tests --- tests/mocks.py | 2 +- tests/test_mocks.py | 195 ++++++++++++++++++++++---------------------- 2 files changed, 100 insertions(+), 97 deletions(-) diff --git a/tests/mocks.py b/tests/mocks.py index 4f587896..8d85e926 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -565,7 +565,7 @@ class MockVoiceChannel(CustomMockMixin, unittest.mock.NonCallableMock, HashableM spec_set = voice_channel_instance def __init__(self, **kwargs) -> None: - default_kwargs = {"id": next(self.discord_id), "name": "channel", "guild": MockGuild()} + default_kwargs = {"id": next(self.discord_id), "name": "voice_channel", "guild": MockGuild()} super().__init__(**collections.ChainMap(kwargs, default_kwargs)) if "mention" not in kwargs: diff --git a/tests/test_mocks.py b/tests/test_mocks.py index 28911707..1648f1a3 100644 --- a/tests/test_mocks.py +++ b/tests/test_mocks.py @@ -31,6 +31,7 @@ """ import asyncio +import typing import unittest.mock import arrow @@ -44,39 +45,73 @@ class TestDiscordMocks: """Tests for our specialized discord.py mocks.""" - def test_mock_role_default_initialization(self): - """Test if the default initialization of MockRole results in the correct object.""" - role = mocks.MockRole() - - # The `spec` argument makes sure `isinstance` checks with `discord.Role` pass - assert isinstance(role, discord.Role) - - assert role.name == "role" - assert role.position == 1 - assert role.mention == "&role" + @pytest.mark.parametrize( + ["mock_class", "counterpart", "mock_args"], + [ + [mocks.MockRole, discord.Role, {"name": "role", "position": 1, "mention": "&role"}], + [ + mocks.MockMember, + discord.Member, + { + "name": "member", + "roles": [mocks.MockRole(name="@everyone", position=1, id=0)], + "mention": "@member", + }, + ], + [ + mocks.MockGuild, + discord.Guild, + {"roles": [mocks.MockRole(name="@everyone", position=1, id=0)], "members": []}, + ], + [mocks.MockBot, discord.ext.commands.Bot, {}], + ], + ) + def test_mock_obj_default_initialization( + self, mock_class: typing.Any, counterpart: typing.Any, mock_args: dict + ): + """Test if the default initialization of a mock object results in the correct object.""" + obj = mock_class() - def test_mock_role_alternative_arguments(self): - """Test if MockRole initializes with the arguments provided.""" - role = mocks.MockRole( - name="Admins", - id=90210, - position=10, - ) + # The `spec` argument makes sure `isinstance` checks with mocks pass + assert isinstance(obj, counterpart) - assert role.name == "Admins" - assert role.id == 90210 - assert role.position == 10 - assert role.mention == "&Admins" + for k, v in mock_args.items(): + assert getattr(obj, k) == v - def test_mock_role_accepts_dynamic_arguments(self): - """Test if MockRole accepts and sets abitrary keyword arguments.""" - role = mocks.MockRole( - guild="Dino Man", - hoist=True, - ) + @pytest.mark.parametrize( + ["mock_class", "mock_args", "extra_mock_args"], + [ + [ + mocks.MockRole, + { + "name": "Admins", + "position": 10, + "id": mocks.generate_realistic_id(arrow.get(1517133142)), + }, + {"mention": "&Admins"}, + ], + [ + mocks.MockMember, + {"name": "arl", "id": mocks.generate_realistic_id(arrow.get(1620350090))}, + {"mention": "@arl"}, + ], + [ + mocks.MockGuild, + {"members": []}, + {"roles": [mocks.MockRole(name="@everyone", position=1, id=0)]}, + ], + [mocks.MockVoiceChannel, {}, {"mention": "#voice_channel"}], + ], + ) + def test_mock_obj_initialization_with_args( + self, mock_class: typing.Any, mock_args: dict, extra_mock_args: dict + ): + """Test if an initialization of a mock object with keywords results in the correct object.""" + obj = mock_class(**mock_args) - assert role.guild == "Dino Man" - assert role.hoist + mock_args.update(extra_mock_args) + for k, v in mock_args.items(): + assert v == getattr(obj, k) def test_mock_role_uses_position_for_less_than_greater_than(self): """Test if `<` and `>` comparisons for MockRole are based on its position attribute.""" @@ -91,47 +126,6 @@ def test_mock_role_uses_position_for_less_than_greater_than(self): assert role_three > role_one assert role_two > role_one - def test_mock_member_default_initialization(self): - """Test if the default initialization of Mockmember results in the correct object.""" - member = mocks.MockMember() - - # The `spec` argument makes sure `isinstance` checks with `discord.Member` pass - assert isinstance(member, discord.Member) - - assert member.name == "member" - assert member.roles == [mocks.MockRole(name="@everyone", position=1, id=0)] - assert member.mention == "@member" - - def test_mock_member_alternative_arguments(self): - """Test if MockMember initializes with the arguments provided.""" - core_developer = mocks.MockRole(name="Core Developer", position=2) - member = mocks.MockMember(name="Mark", id=12345, roles=[core_developer]) - - assert member.name == "Mark" - assert member.id == 12345 - assert member.roles == [mocks.MockRole(name="@everyone", position=1, id=0), core_developer] - assert member.mention == "@Mark" - - def test_mock_member_accepts_dynamic_arguments(self): - """Test if MockMember accepts and sets abitrary keyword arguments.""" - member = mocks.MockMember( - nick="Dino Man", - colour=discord.Colour.default(), - ) - - assert member.nick == "Dino Man" - assert member.colour == discord.Colour.default() - - def test_mock_guild_default_initialization(self): - """Test if the default initialization of Mockguild results in the correct object.""" - guild = mocks.MockGuild() - - # The `spec` argument makes sure `isistance` checks with `discord.Guild` pass - assert isinstance(guild, discord.Guild) - - assert guild.roles == [mocks.MockRole(name="@everyone", position=1, id=0)] - assert guild.members == [] - def test_mock_guild_alternative_arguments(self): """Test if MockGuild initializes with the arguments provided.""" core_developer = mocks.MockRole(name="Core Developer", position=2) @@ -153,13 +147,6 @@ def test_mock_guild_accepts_dynamic_arguments(self): assert guild.emojis == (":hyperjoseph:", ":pensive_ela:") assert guild.premium_subscription_count == 15 - def test_mock_bot_default_initialization(self): - """Tests if MockBot initializes with the correct values.""" - bot = mocks.MockBot() - - # The `spec` argument makes sure `isinstance` checks with `discord.ext.commands.Bot` pass - assert isinstance(bot, discord.ext.commands.Bot) - def test_mock_context_default_initialization(self): """Tests if MockContext initializes with the correct values.""" context = mocks.MockContext() @@ -183,17 +170,18 @@ def test_mock_context_default_initialization(self): @pytest.mark.parametrize( ["mock", "valid_attribute"], [ - [mocks.MockGuild(), "name"], - [mocks.MockRole(), "hoist"], - [mocks.MockMember(), "display_name"], - [mocks.MockBot(), "user"], - [mocks.MockContext(), "invoked_with"], - [mocks.MockTextChannel(), "last_message"], - [mocks.MockMessage(), "mention_everyone"], + [mocks.MockGuild, "name"], + [mocks.MockRole, "hoist"], + [mocks.MockMember, "display_name"], + [mocks.MockBot, "user"], + [mocks.MockContext, "invoked_with"], + [mocks.MockTextChannel, "last_message"], + [mocks.MockMessage, "mention_everyone"], ], ) - def mocks_allows_access_to_attributes_part_of_spec(self, mock, valid_attribute: str): + def test_mocks_allows_access_to_attributes_part_of_spec(self, mock, valid_attribute: str): """Accessing attributes that are valid for the objects they mock should succeed.""" + mock = mock() try: getattr(mock, valid_attribute) except AttributeError: # pragma: nocover @@ -203,30 +191,39 @@ def mocks_allows_access_to_attributes_part_of_spec(self, mock, valid_attribute: @pytest.mark.parametrize( ["mock"], [ - [mocks.MockGuild()], - [mocks.MockRole()], - [mocks.MockMember()], - [mocks.MockBot()], - [mocks.MockContext()], - [mocks.MockTextChannel()], - [mocks.MockMessage()], + [mocks.MockBot], + [mocks.MockCategoryChannel], + [mocks.MockContext], + [mocks.MockClientUser], + [mocks.MockDMChannel], + [mocks.MockGuild], + [mocks.MockMember], + [mocks.MockMessage], + [mocks.MockRole], + [mocks.MockTextChannel], + [mocks.MockThread], + [mocks.MockUser], + [mocks.MockVoiceChannel], ], ) - def mocks_rejects_access_to_attributes_not_part_of_spec(self, mock): + def test_mocks_rejects_access_to_attributes_not_part_of_spec(self, mock): """Accessing attributes that are invalid for the objects they mock should fail.""" + mock = mock() with pytest.raises(AttributeError): mock.the_cake_is_a_lie @pytest.mark.parametrize( ["mock_type", "provided_mention"], [ - [mocks.MockRole, "role mention"], + [mocks.MockClientUser, "client_user mention"], [mocks.MockMember, "member mention"], + [mocks.MockRole, "role mention"], [mocks.MockTextChannel, "channel mention"], + [mocks.MockThread, "thread mention"], [mocks.MockUser, "user mention"], ], ) - def mocks_use_mention_when_provided_as_kwarg(self, mock_type, provided_mention: str): + def test_mocks_use_mention_when_provided_as_kwarg(self, mock_type, provided_mention: str): """The mock should use the passed `mention` instead of the default one if present.""" mock = mock_type(mention=provided_mention) assert mock.mention == provided_mention @@ -247,7 +244,13 @@ async def dementati(): # pragma: nocover assert error.match("cannot reuse already awaited coroutine") -HASHABLE_MOCKS = (mocks.MockRole, mocks.MockMember, mocks.MockGuild) +HASHABLE_MOCKS = ( + mocks.MockRole, + mocks.MockMember, + mocks.MockGuild, + mocks.MockTextChannel, + mocks.MockVoiceChannel, +) class TestMockObjects: From 6f92e86ffb982a08437f4c4d70cddad722234451 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Tue, 26 Oct 2021 22:09:56 -0400 Subject: [PATCH 63/83] chore: remove typos, add comments, general nitpicks --- tests/mocks.py | 1 + tests/test_mocks.py | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/mocks.py b/tests/mocks.py index 8d85e926..d13cd32d 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -278,6 +278,7 @@ def _get_child_mock(self, **kw): else: klass = self.child_mock_type + # if the mock is sealed, that means that new mocks cannot be created when accessing attributes if self._mock_sealed: attribute = "." + kw["name"] if "name" in kw else "()" mock_name = self._extract_mock_name() + attribute diff --git a/tests/test_mocks.py b/tests/test_mocks.py index 1648f1a3..04a8b952 100644 --- a/tests/test_mocks.py +++ b/tests/test_mocks.py @@ -138,7 +138,7 @@ def test_mock_guild_alternative_arguments(self): assert guild.members == [mocks.MockMember(id=54321)] def test_mock_guild_accepts_dynamic_arguments(self): - """Test if MockGuild accepts and sets abitrary keyword arguments.""" + """Test if MockGuild accepts and sets arbitrary keyword arguments.""" guild = mocks.MockGuild( emojis=(":hyperjoseph:", ":pensive_ela:"), premium_subscription_count=15, @@ -163,7 +163,7 @@ def test_mock_context_default_initialization(self): assert context.message.channel is context.channel assert context.channel.guild is context.guild - # ensure the me instance is of the right type and shtuff. + # ensure the me instance is of the right type and is shared among mock attributes. assert isinstance(context.me, mocks.MockMember) assert context.me is context.guild.me @@ -297,7 +297,7 @@ class MockScragly(mocks.HashableMixin): assert scragly == eevee assert (scragly == python) is False - def test_hashable_mixin_uses_id_for_nonequality_comparison(self): + def test_hashable_mixin_uses_id_for_inequality_comparison(self): """Test if the HashableMixing uses the id attribute for non-equal comparison.""" class MockScragly(mocks.HashableMixin): From aa3b4de92961c69e12cf875fab215c17b448d484 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Tue, 26 Oct 2021 22:19:27 -0400 Subject: [PATCH 64/83] tests: don't always test the mocks The mock file is pretty big, and due to that has a lot of tests The mocks should not need to be edited often, only when the dependency they are mocking is updated. This is currently never, as it is unmaintained. This means that we don't need to run the tests for the mocks all the time. If an upgrade of our dependencies is taken, then the test file should be ran. However, it does not need to run every time the test suite is ran. This should lead to a faster total test suite when running the suite. --- pyproject.toml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index ab59efdc..5281ba95 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,13 +63,13 @@ build-backend = "poetry.core.masonry.api" [tool.coverage.run] branch = true -source_pkgs = ['modmail', 'tests'] +source_pkgs = ['modmail', 'tests.modmail'] omit = ["modmail/plugins/**.*"] [tool.pytest.ini_options] addopts = "--cov --cov-report=" minversion = "6.0" -testpaths = ["tests"] +testpaths = ["tests/modmail"] [tool.black] line-length = 110 @@ -89,3 +89,4 @@ lint = { cmd = "pre-commit run --all-files", help = "Checks all files for CI err precommit = { cmd = "pre-commit install --install-hooks", help = "Installs the precommit hook" } report = { cmd = "coverage report", help = "Show coverage report from previously run tests." } test = { cmd = "pytest -n auto --dist loadfile", help = "Runs tests and save results to a coverage report" } +test_mocks = { cmd = 'pytest tests/test_mocks.py', help = 'Runs the tests on the mock files. They are excluded from the main test suite.' } From 3d6b29d3a6247a813f99a64ea4a8620fb955e722 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Wed, 27 Oct 2021 04:02:46 -0400 Subject: [PATCH 65/83] fix: return types not always returning the correct type --- tests/mocks.py | 12 ++++------ tests/test_mocks.py | 58 ++++++++++++++++++++++++++++----------------- 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/tests/mocks.py b/tests/mocks.py index d13cd32d..fed6b01d 100644 --- a/tests/mocks.py +++ b/tests/mocks.py @@ -57,7 +57,6 @@ "HashableMixin", "CustomMockMixin", "ColourMixin", - "MockAsyncWebhook", "MockAttachment", "MockBot", "MockCategoryChannel", @@ -75,6 +74,7 @@ "MockThread", "MockUser", "MockVoiceChannel", + "MockWebhook", ] @@ -158,7 +158,7 @@ def generate_mock_message(content=unittest.mock.DEFAULT, *args, **kwargs): discord.Thread: lambda *args, **kwargs: MockThread(), discord.User: lambda *args, **kwargs: MockUser(), discord.VoiceChannel: lambda *args, **kwargs: MockVoiceChannel(), - discord.Webhook: lambda *args, **kwargs: MockAsyncWebhook(), + discord.Webhook: lambda *args, **kwargs: MockWebhook(), } @@ -222,10 +222,8 @@ def __init__(self, **kwargs): # this list can be added to as methods are discovered if attr.__name__ == "send": hints["return"] = discord.Message - elif self.__class__ == discord.Message and attr.__name__ == "edit": - # set up message editing to return the same object - mock_config[f"{attr.__name__}.return_value"] = self - continue + elif attr.__name__ == "edit": + hints["return"] = type(self.spec_set) if hints.get("return") is None: continue @@ -822,7 +820,7 @@ def __init__(self, **kwargs) -> None: webhook_instance = discord.Webhook(data=unittest.mock.MagicMock(), session=unittest.mock.MagicMock()) -class MockAsyncWebhook(CustomMockMixin, unittest.mock.NonCallableMagicMock): +class MockWebhook(CustomMockMixin, unittest.mock.NonCallableMagicMock): """ A MagicMock subclass to mock Webhook objects using an AsyncWebhookAdapter. diff --git a/tests/test_mocks.py b/tests/test_mocks.py index 04a8b952..e1b56329 100644 --- a/tests/test_mocks.py +++ b/tests/test_mocks.py @@ -400,42 +400,56 @@ class TestReturnTypes: Eg, ctx.send should return a message object. """ + @pytest.mark.parametrize( + "mock_cls", + [ + mocks.MockClientUser, + mocks.MockGuild, + mocks.MockMember, + mocks.MockMessage, + mocks.MockTextChannel, + mocks.MockVoiceChannel, + mocks.MockWebhook, + ], + ) @pytest.mark.asyncio - async def test_message_edit_returns_self(self): - """Message editing edits the message in place. We should be returning the message.""" - msg = mocks.MockMessage() - - new_msg = await msg.edit() + async def test_edit_returns_same_class(self, mock_cls): + """Edit methods return a new instance of the same type.""" + mock = mock_cls() - assert isinstance(new_msg, discord.Message) + new_mock = await mock.edit() - assert msg is new_msg + assert isinstance(new_mock, type(mock_cls.spec_set)) + @pytest.mark.parametrize( + "mock_cls", + [ + mocks.MockMember, + mocks.MockTextChannel, + mocks.MockThread, + mocks.MockUser, + ], + ) @pytest.mark.asyncio - async def test_channel_send_returns_message(self): + async def test_messageable_send_returns_message(self, mock_cls): """Ensure that channel objects return mocked messages when sending messages.""" - channel = mocks.MockTextChannel() + messageable = mock_cls() - msg = await channel.send("hi") + msg = await messageable.send("hi") print(type(msg)) assert isinstance(msg, discord.Message) + @pytest.mark.parametrize( + "mock_cls", + [mocks.MockMessage, mocks.MockTextChannel], + ) @pytest.mark.asyncio - async def test_message_thread_create_returns_thread(self): - """Thread create methods should return a MockThread.""" - msg = mocks.MockMessage() - - thread = await msg.create_thread() - - assert isinstance(thread, discord.Thread) - - @pytest.mark.asyncio - async def test_channel_thread_create_returns_thread(self): + async def test_thread_create_returns_thread(self, mock_cls): """Thread create methods should return a MockThread.""" - channel = mocks.MockTextChannel() + mock = mock_cls() - thread = await channel.create_thread() + thread = await mock.create_thread() assert isinstance(thread, discord.Thread) From 8164591166e53594dc961bd4b74340acdeaea954 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Mon, 18 Oct 2021 00:26:24 -0400 Subject: [PATCH 66/83] tests: add some error handler tests Signed-off-by: onerandomusername --- tests/modmail/extensions/utils/__init__.py | 0 .../extensions/utils/test_error_handler.py | 174 ++++++++++++++++++ 2 files changed, 174 insertions(+) create mode 100644 tests/modmail/extensions/utils/__init__.py create mode 100644 tests/modmail/extensions/utils/test_error_handler.py diff --git a/tests/modmail/extensions/utils/__init__.py b/tests/modmail/extensions/utils/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/modmail/extensions/utils/test_error_handler.py b/tests/modmail/extensions/utils/test_error_handler.py new file mode 100644 index 00000000..8dbfc676 --- /dev/null +++ b/tests/modmail/extensions/utils/test_error_handler.py @@ -0,0 +1,174 @@ +import inspect +import typing +import unittest.mock + +import discord +import pytest +from discord.ext import commands + +from modmail.extensions.utils import error_handler +from modmail.extensions.utils.error_handler import ErrorHandler +from tests import mocks + + +@pytest.fixture +def cog(): + """Pytest fixture for error_handler.""" + return ErrorHandler(mocks.MockBot()) + + +@pytest.fixture +def ctx(): + """Pytest fixture for MockContext.""" + return mocks.MockContext(channel=mocks.MockTextChannel()) + + +def test_error_embed(): + """Test the error embed method creates the correct embed.""" + title = "Something very drastic went very wrong!" + message = "seven southern seas are ready to collapse." + embed = ErrorHandler.error_embed(title=title, message=message) + + assert embed.title == title + assert embed.description == message + assert embed.colour == error_handler.ERROR_COLOUR + + +@pytest.mark.parametrize( + ["exception_or_str", "expected_str"], + [ + [commands.NSFWChannelRequired(mocks.MockTextChannel()), "NSFW Channel Required"], + [commands.CommandNotFound(), "Command Not Found"], + ["someWEIrdName", "some WE Ird Name"], + ], +) +def test_get_title_from_name(exception_or_str: typing.Union[Exception, str], expected_str: str): + """Test the regex works properly for the title from name.""" + result = ErrorHandler.get_title_from_name(exception_or_str) + assert expected_str == result + + +@pytest.mark.parametrize( + ["error", "title", "description"], + [ + [ + commands.UserInputError("some interesting information."), + "User Input Error", + "some interesting information.", + ], + [ + commands.MissingRequiredArgument(inspect.Parameter("SomethingSpecial", kind=1)), + "Missing Required Argument", + "SomethingSpecial is a required argument that is missing.", + ], + [ + commands.GuildNotFound("ImportantGuild"), + "Guild Not Found", + 'Guild "ImportantGuild" not found.', + ], + ], +) +@pytest.mark.asyncio +async def test_handle_user_input_error( + cog: ErrorHandler, ctx: mocks.MockContext, error: commands.UserInputError, title: str, description: str +): + """Test user input errors are handled properly. Does not test with BadUnionArgument.""" + embed = await cog.handle_user_input_error(ctx=ctx, error=error, reset_cooldown=False) + + assert title == embed.title + assert description == embed.description + + +@pytest.mark.asyncio +async def test_handle_bot_missing_perms(cog: ErrorHandler): + """ + + Test error_handler.handle_bot_missing_perms. + + There are some cases here where the bot is unable to send messages, and that should be clear. + """ + ... + + +@pytest.mark.asyncio +async def test_handle_check_failure(cog: ErrorHandler): + """ + Test check failures. + + In some cases, this method should result in calling a bot_missing_perms method + because the bot cannot send messages. + """ + ... + + +@pytest.mark.asyncio +async def test_on_command_error(cog: ErrorHandler): + """Test the general command error method.""" + ... + + +class TestErrorHandler: + """ + Test class for the error handler. The problem here is a lot of the errors need to be raised. + + Thankfully, most of them do not have extra attributes that we use, and can be easily faked. + """ + + errors = { + commands.CommandError: [ + commands.ConversionError, + { + commands.UserInputError: [ + commands.MissingRequiredArgument, + commands.TooManyArguments, + { + commands.BadArgument: [ + commands.MessageNotFound, + commands.MemberNotFound, + commands.GuildNotFound, + commands.UserNotFound, + commands.ChannelNotFound, + commands.ChannelNotReadable, + commands.BadColourArgument, + commands.RoleNotFound, + commands.BadInviteArgument, + commands.EmojiNotFound, + commands.GuildStickerNotFound, + commands.PartialEmojiConversionFailure, + commands.BadBoolArgument, + commands.ThreadNotFound, + ] + }, + commands.BadUnionArgument, + commands.BadLiteralArgument, + { + commands.ArgumentParsingError: [ + commands.UnexpectedQuoteError, + commands.InvalidEndOfQuotedStringError, + commands.ExpectedClosingQuoteError, + ] + }, + ] + }, + commands.CommandNotFound, + { + commands.CheckFailure: [ + commands.CheckAnyFailure, + commands.PrivateMessageOnly, + commands.NoPrivateMessage, + commands.NotOwner, + commands.MissingPermissions, + commands.BotMissingPermissions, + commands.MissingRole, + commands.BotMissingRole, + commands.MissingAnyRole, + commands.BotMissingAnyRole, + commands.NSFWChannelRequired, + ] + }, + commands.DisabledCommand, + commands.CommandInvokeError, + commands.CommandOnCooldown, + commands.MaxConcurrencyReached, + ] + } From e41125fdf3dfb5c475a779ac371e7c66c2a1b9f5 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Tue, 19 Oct 2021 18:28:28 -0400 Subject: [PATCH 67/83] feat: add timestamp util Signed-off-by: onerandomusername --- modmail/utils/time.py | 39 ++++++++++++++++++++++++++++++++ tests/modmail/utils/test_time.py | 25 ++++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 modmail/utils/time.py create mode 100644 tests/modmail/utils/test_time.py diff --git a/modmail/utils/time.py b/modmail/utils/time.py new file mode 100644 index 00000000..b073574a --- /dev/null +++ b/modmail/utils/time.py @@ -0,0 +1,39 @@ +import datetime +import enum +import typing + +import arrow + + +class TimeStampEnum(enum.Enum): + """ + Timestamp modes for discord. + + Full docs on this format are viewable here: + https://discord.com/developers/docs/reference#message-formatting + """ + + SHORT_TIME = "t" + LONG_TIME = "T" + SHORT_DATE = "d" + LONG_DATE = "D" + SHORT_DATE_TIME = "f" + LONG_DATE_TIME = "F" + RELATIVE_TIME = "R" + + # DEFAULT + DEFAULT = SHORT_DATE_TIME + + +TypeTimes = typing.Union[arrow.Arrow, datetime.datetime] + + +def get_discord_formatted_timestamp( + timestamp: TypeTimes, format: TimeStampEnum = TimeStampEnum.DEFAULT +) -> str: + """ + Return a discord formatted timestamp from a datetime compatiable datatype. + + `format` must be an enum member of TimeStampEnum. Default style is SHORT_DATE_TIME + """ + return f"" diff --git a/tests/modmail/utils/test_time.py b/tests/modmail/utils/test_time.py new file mode 100644 index 00000000..6eb70613 --- /dev/null +++ b/tests/modmail/utils/test_time.py @@ -0,0 +1,25 @@ +import arrow +import pytest + +from modmail.utils import time as utils_time +from modmail.utils.time import TimeStampEnum + + +@pytest.mark.parametrize( + ["timestamp", "expected", "mode"], + [ + [arrow.get(1634593650), "", TimeStampEnum.SHORT_DATE_TIME], + [arrow.get(1), "", TimeStampEnum.SHORT_DATE_TIME], + [arrow.get(12356941), "", TimeStampEnum.RELATIVE_TIME], + ], +) +def test_timestamp(timestamp, expected: str, mode: utils_time.TimeStampEnum): + """Test the timestamp is of the proper form.""" + fmtted_timestamp = utils_time.get_discord_formatted_timestamp(timestamp, mode) + assert expected == fmtted_timestamp + + +def test_enum_default(): + """Ensure that the default mode is of the correct mode, and works properly.""" + assert TimeStampEnum.DEFAULT.name == TimeStampEnum.SHORT_DATE_TIME.name + assert TimeStampEnum.DEFAULT.value == TimeStampEnum.SHORT_DATE_TIME.value From 62fa485b167274d9d8eb23ade4112ed11269ca25 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Wed, 20 Oct 2021 00:20:05 -0400 Subject: [PATCH 68/83] nit: add comments for timestamp modes Signed-off-by: onerandomusername --- modmail/utils/time.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/modmail/utils/time.py b/modmail/utils/time.py index b073574a..429609d4 100644 --- a/modmail/utils/time.py +++ b/modmail/utils/time.py @@ -13,15 +13,17 @@ class TimeStampEnum(enum.Enum): https://discord.com/developers/docs/reference#message-formatting """ - SHORT_TIME = "t" - LONG_TIME = "T" - SHORT_DATE = "d" - LONG_DATE = "D" - SHORT_DATE_TIME = "f" - LONG_DATE_TIME = "F" - RELATIVE_TIME = "R" - - # DEFAULT + # fmt: off + SHORT_TIME = "t" # 16:20 + LONG_TIME = "T" # 16:20:30 + SHORT_DATE = "d" # 20/04/2021 + LONG_DATE = "D" # 20 April 2021 + SHORT_DATE_TIME = "f" # 20 April 2021 16:20 + LONG_DATE_TIME = "F" # Tuesday, 20 April 2021 16:20 + RELATIVE_TIME = "R" # 2 months ago + + # fmt: on + # DEFAULT alised to the default, so for all purposes, it behaves like SHORT_DATE_TIME, including the name DEFAULT = SHORT_DATE_TIME From b721e3a78d293ca794b0ab9aed8de139c280bc1a Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Wed, 20 Oct 2021 00:30:13 -0400 Subject: [PATCH 69/83] tests: add test for datetime instead of arrow object Signed-off-by: onerandomusername --- tests/modmail/utils/test_time.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/modmail/utils/test_time.py b/tests/modmail/utils/test_time.py index 6eb70613..53682643 100644 --- a/tests/modmail/utils/test_time.py +++ b/tests/modmail/utils/test_time.py @@ -9,8 +9,9 @@ ["timestamp", "expected", "mode"], [ [arrow.get(1634593650), "", TimeStampEnum.SHORT_DATE_TIME], - [arrow.get(1), "", TimeStampEnum.SHORT_DATE_TIME], + [arrow.get(1), "", TimeStampEnum.DEFAULT], [arrow.get(12356941), "", TimeStampEnum.RELATIVE_TIME], + [arrow.get(8675309).datetime, "", TimeStampEnum.LONG_DATE], ], ) def test_timestamp(timestamp, expected: str, mode: utils_time.TimeStampEnum): From 6c6e17027387d05b1bb276760e5c38b871d42278 Mon Sep 17 00:00:00 2001 From: Bast Date: Thu, 28 Oct 2021 14:58:21 -0700 Subject: [PATCH 70/83] Put added back on top of changed in the changelog --- docs/changelog.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index a53544f5..509710c5 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -7,13 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Added Dispatcher system, although it is not hooked into important features like thread creation yet. (#71) + ### Changed - Embedified the meta commands so they have a nicer UI (#78) -### Added -- Added Dispatcher system, although it is not hooked into important features like thread creation yet. (#71) - ## [0.2.0] - 2021-09-29 ### Added From de92da0dc6d29228a50c37d11f4c4b99e17f62d8 Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Fri, 29 Oct 2021 13:43:41 +0530 Subject: [PATCH 71/83] fix: Correctly check if author is dependabot --- .github/workflows/update_requirements_dependabot.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/update_requirements_dependabot.yml b/.github/workflows/update_requirements_dependabot.yml index b44eeb7b..a9e7c144 100644 --- a/.github/workflows/update_requirements_dependabot.yml +++ b/.github/workflows/update_requirements_dependabot.yml @@ -6,7 +6,7 @@ on: jobs: regenerate-requirements: runs-on: ubuntu-latest - if: github.event.issue.user.login == 'dependabot[bot]' + if: ${{ github.actor == 'dependabot[bot]' }} steps: # Checks out the repository in the current folder. - name: Checkout repository From 6749b9e5f82f1b80fc6fb590b8a32c8d6174bb38 Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Fri, 29 Oct 2021 13:49:26 +0530 Subject: [PATCH 72/83] feat: Auto label dependabot PRs --- .github/workflows/update_requirements_dependabot.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/update_requirements_dependabot.yml b/.github/workflows/update_requirements_dependabot.yml index a9e7c144..9ede2e36 100644 --- a/.github/workflows/update_requirements_dependabot.yml +++ b/.github/workflows/update_requirements_dependabot.yml @@ -3,6 +3,11 @@ name: Update `requirements.txt` on: pull_request: +permissions: + pull-requests: write + issues: write + repository-projects: write + jobs: regenerate-requirements: runs-on: ubuntu-latest @@ -38,3 +43,8 @@ jobs: git add . git commit --amend --no-edit git push --force origin ${GITHUB_REF##*/} + + - name: Add a label + run: gh pr edit "$PR_URL" --add-label "a{{':'}} dependencies,skip changelog" + env: + PR_URL: ${{github.event.pull_request.html_url}} From a37949ff5c5ee21e8635680ac1932b10984fa527 Mon Sep 17 00:00:00 2001 From: Shivansh-007 Date: Fri, 29 Oct 2021 14:16:54 +0530 Subject: [PATCH 73/83] Switch to single quotes, as they are raw strings --- .github/workflows/update_requirements_dependabot.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/update_requirements_dependabot.yml b/.github/workflows/update_requirements_dependabot.yml index 9ede2e36..db4759d4 100644 --- a/.github/workflows/update_requirements_dependabot.yml +++ b/.github/workflows/update_requirements_dependabot.yml @@ -45,6 +45,6 @@ jobs: git push --force origin ${GITHUB_REF##*/} - name: Add a label - run: gh pr edit "$PR_URL" --add-label "a{{':'}} dependencies,skip changelog" + run: 'gh pr edit "$PR_URL" --add-label "a: dependencies,skip changelog"' env: PR_URL: ${{github.event.pull_request.html_url}} From 3b0376877ee55d64605b5980516d15558f3d57f9 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Wed, 13 Oct 2021 15:26:12 -0400 Subject: [PATCH 74/83] feat: add helper responses module picked from GH-69 Signed-off-by: onerandomusername --- modmail/utils/responses.py | 174 +++++++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 modmail/utils/responses.py diff --git a/modmail/utils/responses.py b/modmail/utils/responses.py new file mode 100644 index 00000000..135ac23d --- /dev/null +++ b/modmail/utils/responses.py @@ -0,0 +1,174 @@ +""" +Helper methods for responses from the bot to the user. + +These help ensure consistency between errors, as they will all be consistent between different uses. +""" +import logging +from random import choice +from typing import List + +import discord +from discord.ext.commands import Context + +from modmail.log import ModmailLogger + + +__all__ = ( + "default_success_color", + "success_headers", + "default_error_color", + "error_headers", + "send_positive_response", + "send_negatory_response", + "send_response", +) + +_UNSET = object() + +logger: ModmailLogger = logging.getLogger(__name__) + + +default_success_color = discord.Colour.green() +success_headers: List[str] = [ + "You got it.", + "Done.", + "Affirmative.", + "As you wish.", + "Okay.", + "Fine by me.", + "There we go.", + "Sure!", + "Your wish is my command.", +] + +default_error_color = discord.Colour.red() +error_headers: List[str] = [ + "Abort!", + "FAIL.", + "I cannot do that.", + "I'm leaving you.", + "Its not me, its you.", + "Hold up!", + "Mistakes were made.", + "Nope.", + "Not happening.", + "Oops.", + "Something went wrong.", + "Sorry, no.", + "This will never work.", + "Uh. No.", + "\U0001f914", + "That is not happening.", + "Whups.", +] + + +async def send_positive_response( + channel: discord.abc.Messageable, + response: str, + embed: discord.Embed = _UNSET, + colour: discord.Colour = _UNSET, + message: discord.Message = None, + **kwargs, +) -> discord.Message: + """ + Send an affirmative response. + + Requires a messageable, and a response. + If embed is set to None, this will send response as a plaintext message, with no allowed_mentions. + If embed is provided, this method will send a response using the provided embed, edited in place. + Extra kwargs are passed to Messageable.send() + + If message is provided, it will attempt to edit that message rather than sending a new one. + """ + kwargs["allowed_mentions"] = kwargs.get("allowed_mentions", discord.AllowedMentions.none()) + + if isinstance(channel, Context): + channel = channel.channel + + logger.debug(f"Requested to send affirmative message to {channel!s}. Response: {response!s}") + + if embed is None: + if message is None: + return await channel.send(response, **kwargs) + else: + return await message.edit(response, **kwargs) + + if colour is _UNSET: + colour = default_success_color + + if embed is _UNSET: + embed = discord.Embed(colour=colour) + embed.title = choice(success_headers) + embed.description = response + + if message is None: + return await channel.send(embed=embed, **kwargs) + else: + return await message.edit(embed=embed, **kwargs) + + +async def send_negatory_response( + channel: discord.abc.Messageable, + response: str, + embed: discord.Embed = _UNSET, + colour: discord.Colour = _UNSET, + message: discord.Message = None, + **kwargs, +) -> discord.Message: + """ + Send a negatory response. + + Requires a messageable, and a response. + If embed is set to None, this will send response as a plaintext message, with no allowed_mentions. + If embed is provided, this method will send a response using the provided embed, edited in place. + Extra kwargs are passed to Messageable.send() + """ + kwargs["allowed_mentions"] = kwargs.get("allowed_mentions", discord.AllowedMentions.none()) + + if isinstance(channel, Context): + channel = channel.channel + + logger.debug(f"Requested to send negatory message to {channel!s}. Response: {response!s}") + + if embed is None: + if message is None: + return await channel.send(response, **kwargs) + else: + return await message.edit(response, **kwargs) + + if colour is _UNSET: + colour = default_error_color + + if embed is _UNSET: + embed = discord.Embed(colour=colour) + embed.title = choice(error_headers) + embed.description = response + + if message is None: + return await channel.send(embed=embed, **kwargs) + else: + return await message.edit(embed=embed, **kwargs) + + +async def send_response( + channel: discord.abc.Messageable, + response: str, + success: bool, + embed: discord.Embed = _UNSET, + colour: discord.Colour = _UNSET, + message: discord.Message = None, + **kwargs, +) -> discord.Message: + """ + Send a response based on success or failure. + + Requires a messageable, and a response. + If embed is set to None, this will send response as a plaintext message, with no allowed_mentions. + If embed is provided, this method will send a response using the provided embed, edited in place. + Extra kwargs are passed to Messageable.send() + """ + if success: + return await send_positive_response(channel, response, embed, colour, message, **kwargs) + else: + return await send_negatory_response(channel, response, embed, colour, message, **kwargs) From a426f8214a89b99ef65800299187139201515a89 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Wed, 13 Oct 2021 18:08:53 -0400 Subject: [PATCH 75/83] fix: make constants SCREAMING_SNAKE_CASE Signed-off-by: onerandomusername --- modmail/utils/responses.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/modmail/utils/responses.py b/modmail/utils/responses.py index 135ac23d..daeb346f 100644 --- a/modmail/utils/responses.py +++ b/modmail/utils/responses.py @@ -14,10 +14,12 @@ __all__ = ( - "default_success_color", - "success_headers", - "default_error_color", - "error_headers", + "DEFAULT_SUCCESS_COLOUR", + "DEFAULT_SUCCESS_COLOR", + "SUCCESS_HEADERS", + "DEFAULT_ERROR_COLOUR", + "DEFAULT_ERROR_COLOR", + "ERROR_HEADERS", "send_positive_response", "send_negatory_response", "send_response", @@ -28,8 +30,9 @@ logger: ModmailLogger = logging.getLogger(__name__) -default_success_color = discord.Colour.green() -success_headers: List[str] = [ +DEFAULT_SUCCESS_COLOUR = discord.Colour.green() +DEFAULT_SUCCESS_COLOR = DEFAULT_SUCCESS_COLOUR +SUCCESS_HEADERS: List[str] = [ "You got it.", "Done.", "Affirmative.", @@ -41,8 +44,9 @@ "Your wish is my command.", ] -default_error_color = discord.Colour.red() -error_headers: List[str] = [ +DEFAULT_ERROR_COLOUR = discord.Colour.red() +DEFAULT_ERROR_COLOR = DEFAULT_ERROR_COLOUR +ERROR_HEADERS: List[str] = [ "Abort!", "FAIL.", "I cannot do that.", @@ -95,11 +99,11 @@ async def send_positive_response( return await message.edit(response, **kwargs) if colour is _UNSET: - colour = default_success_color + colour = DEFAULT_SUCCESS_COLOUR if embed is _UNSET: embed = discord.Embed(colour=colour) - embed.title = choice(success_headers) + embed.title = choice(SUCCESS_HEADERS) embed.description = response if message is None: @@ -138,11 +142,11 @@ async def send_negatory_response( return await message.edit(response, **kwargs) if colour is _UNSET: - colour = default_error_color + colour = DEFAULT_ERROR_COLOUR if embed is _UNSET: embed = discord.Embed(colour=colour) - embed.title = choice(error_headers) + embed.title = choice(ERROR_HEADERS) embed.description = response if message is None: From 1715162c6ead3b5f39510ae463fa193dd092195c Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 17 Oct 2021 18:48:17 -0400 Subject: [PATCH 76/83] nit: don't use "from x" imports Signed-off-by: onerandomusername --- modmail/utils/responses.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/modmail/utils/responses.py b/modmail/utils/responses.py index daeb346f..b6d35bf8 100644 --- a/modmail/utils/responses.py +++ b/modmail/utils/responses.py @@ -4,11 +4,11 @@ These help ensure consistency between errors, as they will all be consistent between different uses. """ import logging -from random import choice -from typing import List +import random +import typing import discord -from discord.ext.commands import Context +from discord.ext import commands from modmail.log import ModmailLogger @@ -32,7 +32,7 @@ DEFAULT_SUCCESS_COLOUR = discord.Colour.green() DEFAULT_SUCCESS_COLOR = DEFAULT_SUCCESS_COLOUR -SUCCESS_HEADERS: List[str] = [ +SUCCESS_HEADERS: typing.List[str] = [ "You got it.", "Done.", "Affirmative.", @@ -46,7 +46,7 @@ DEFAULT_ERROR_COLOUR = discord.Colour.red() DEFAULT_ERROR_COLOR = DEFAULT_ERROR_COLOUR -ERROR_HEADERS: List[str] = [ +ERROR_HEADERS: typing.List[str] = [ "Abort!", "FAIL.", "I cannot do that.", @@ -87,7 +87,7 @@ async def send_positive_response( """ kwargs["allowed_mentions"] = kwargs.get("allowed_mentions", discord.AllowedMentions.none()) - if isinstance(channel, Context): + if isinstance(channel, commands.Context): channel = channel.channel logger.debug(f"Requested to send affirmative message to {channel!s}. Response: {response!s}") @@ -103,7 +103,7 @@ async def send_positive_response( if embed is _UNSET: embed = discord.Embed(colour=colour) - embed.title = choice(SUCCESS_HEADERS) + embed.title = random.choice(SUCCESS_HEADERS) embed.description = response if message is None: @@ -130,7 +130,7 @@ async def send_negatory_response( """ kwargs["allowed_mentions"] = kwargs.get("allowed_mentions", discord.AllowedMentions.none()) - if isinstance(channel, Context): + if isinstance(channel, commands.Context): channel = channel.channel logger.debug(f"Requested to send negatory message to {channel!s}. Response: {response!s}") @@ -146,7 +146,7 @@ async def send_negatory_response( if embed is _UNSET: embed = discord.Embed(colour=colour) - embed.title = choice(ERROR_HEADERS) + embed.title = random.choice(ERROR_HEADERS) embed.description = response if message is None: From 11f55c1d016855cd0dd3f646ed2b85459e1d5fb1 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 17 Oct 2021 21:48:29 -0400 Subject: [PATCH 77/83] fix: restructure responses module to make it easier to use Signed-off-by: onerandomusername --- modmail/utils/responses.py | 113 +++++++++++++++++-------------------- 1 file changed, 53 insertions(+), 60 deletions(-) diff --git a/modmail/utils/responses.py b/modmail/utils/responses.py index b6d35bf8..e2a014a3 100644 --- a/modmail/utils/responses.py +++ b/modmail/utils/responses.py @@ -17,12 +17,12 @@ "DEFAULT_SUCCESS_COLOUR", "DEFAULT_SUCCESS_COLOR", "SUCCESS_HEADERS", - "DEFAULT_ERROR_COLOUR", - "DEFAULT_ERROR_COLOR", - "ERROR_HEADERS", + "DEFAULT_FAILURE_COLOUR", + "DEFAULT_FAILURE_COLOR", + "FAILURE_HEADERS", + "send_general_response", "send_positive_response", "send_negatory_response", - "send_response", ) _UNSET = object() @@ -44,9 +44,9 @@ "Your wish is my command.", ] -DEFAULT_ERROR_COLOUR = discord.Colour.red() -DEFAULT_ERROR_COLOR = DEFAULT_ERROR_COLOUR -ERROR_HEADERS: typing.List[str] = [ +DEFAULT_FAILURE_COLOUR = discord.Colour.red() +DEFAULT_FAILURE_COLOR = DEFAULT_FAILURE_COLOUR +FAILURE_HEADERS: typing.List[str] = [ "Abort!", "FAIL.", "I cannot do that.", @@ -67,30 +67,29 @@ ] -async def send_positive_response( +async def send_general_response( channel: discord.abc.Messageable, response: str, - embed: discord.Embed = _UNSET, - colour: discord.Colour = _UNSET, + *, message: discord.Message = None, + embed: discord.Embed = _UNSET, + colour: discord.Colour = None, + title: str = None, + _kind: typing.Literal["general", "affirmative", "negatory"] = "general", **kwargs, ) -> discord.Message: """ - Send an affirmative response. + Helper method to send a response. - Requires a messageable, and a response. - If embed is set to None, this will send response as a plaintext message, with no allowed_mentions. - If embed is provided, this method will send a response using the provided embed, edited in place. - Extra kwargs are passed to Messageable.send() - - If message is provided, it will attempt to edit that message rather than sending a new one. + Shortcuts are provided as `send_positive_response` and `send_negatory_response` which + fill in the title and colour automatically. """ kwargs["allowed_mentions"] = kwargs.get("allowed_mentions", discord.AllowedMentions.none()) - if isinstance(channel, commands.Context): + if isinstance(channel, commands.Context): # pragma: nocover channel = channel.channel - logger.debug(f"Requested to send affirmative message to {channel!s}. Response: {response!s}") + logger.debug(f"Requested to send {_kind} message to {channel!s}. Response: {response!s}") if embed is None: if message is None: @@ -98,12 +97,12 @@ async def send_positive_response( else: return await message.edit(response, **kwargs) - if colour is _UNSET: - colour = DEFAULT_SUCCESS_COLOUR + if embed is _UNSET: # pragma: no branch + embed = discord.Embed(colour=colour or discord.Embed.Empty) + + if title is not None: + embed.title = title - if embed is _UNSET: - embed = discord.Embed(colour=colour) - embed.title = random.choice(SUCCESS_HEADERS) embed.description = response if message is None: @@ -112,67 +111,61 @@ async def send_positive_response( return await message.edit(embed=embed, **kwargs) -async def send_negatory_response( +async def send_positive_response( channel: discord.abc.Messageable, response: str, - embed: discord.Embed = _UNSET, + *, colour: discord.Colour = _UNSET, - message: discord.Message = None, **kwargs, ) -> discord.Message: """ - Send a negatory response. + Send an affirmative response. Requires a messageable, and a response. If embed is set to None, this will send response as a plaintext message, with no allowed_mentions. If embed is provided, this method will send a response using the provided embed, edited in place. Extra kwargs are passed to Messageable.send() - """ - kwargs["allowed_mentions"] = kwargs.get("allowed_mentions", discord.AllowedMentions.none()) - if isinstance(channel, commands.Context): - channel = channel.channel + If message is provided, it will attempt to edit that message rather than sending a new one. + """ + if colour is _UNSET: # pragma: no branch + colour = DEFAULT_SUCCESS_COLOUR - logger.debug(f"Requested to send negatory message to {channel!s}. Response: {response!s}") + kwargs["title"] = kwargs.get("title", random.choice(SUCCESS_HEADERS)) - if embed is None: - if message is None: - return await channel.send(response, **kwargs) - else: - return await message.edit(response, **kwargs) + return await send_general_response( + channel=channel, + response=response, + colour=colour, + _kind="affirmative", + **kwargs, + ) - if colour is _UNSET: - colour = DEFAULT_ERROR_COLOUR - if embed is _UNSET: - embed = discord.Embed(colour=colour) - embed.title = random.choice(ERROR_HEADERS) - embed.description = response - - if message is None: - return await channel.send(embed=embed, **kwargs) - else: - return await message.edit(embed=embed, **kwargs) - - -async def send_response( +async def send_negatory_response( channel: discord.abc.Messageable, response: str, - success: bool, - embed: discord.Embed = _UNSET, + *, colour: discord.Colour = _UNSET, - message: discord.Message = None, **kwargs, ) -> discord.Message: """ - Send a response based on success or failure. + Send a negatory response. Requires a messageable, and a response. If embed is set to None, this will send response as a plaintext message, with no allowed_mentions. If embed is provided, this method will send a response using the provided embed, edited in place. Extra kwargs are passed to Messageable.send() """ - if success: - return await send_positive_response(channel, response, embed, colour, message, **kwargs) - else: - return await send_negatory_response(channel, response, embed, colour, message, **kwargs) + if colour is _UNSET: # pragma: no branch + colour = DEFAULT_FAILURE_COLOUR + + kwargs["title"] = kwargs.get("title", random.choice(FAILURE_HEADERS)) + + return await send_general_response( + channel=channel, + response=response, + colour=colour, + _kind="negatory", + **kwargs, + ) From 86c337bec306853f71c2c6abd7e3365c45b30b5f Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sun, 17 Oct 2021 21:50:54 -0400 Subject: [PATCH 78/83] tests: add full test suite for utils.responses Signed-off-by: onerandomusername --- tests/modmail/utils/test_responses.py | 101 ++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 tests/modmail/utils/test_responses.py diff --git a/tests/modmail/utils/test_responses.py b/tests/modmail/utils/test_responses.py new file mode 100644 index 00000000..224878c9 --- /dev/null +++ b/tests/modmail/utils/test_responses.py @@ -0,0 +1,101 @@ +import typing +import unittest.mock + +import discord +import pytest + +from modmail.utils import responses +from tests import mocks + + +@pytest.fixture +async def mock_channel() -> mocks.MockTextChannel: + """Fixture for a channel.""" + return mocks.MockTextChannel() + + +@pytest.fixture +async def mock_message() -> mocks.MockMessage: + """Fixture for a message.""" + return mocks.MockMessage() + + +@pytest.mark.asyncio +async def test_general_response_embed(mock_channel: mocks.MockTextChannel) -> None: + """Test the positive response embed is correct when sending a new message.""" + content = "success!" + _ = await responses.send_general_response(mock_channel, content) + + assert len(mock_channel.send.mock_calls) == 1 + + _, _, called_kwargs = mock_channel.send.mock_calls[0] + sent_embed: discord.Embed = called_kwargs["embed"] + + assert content == sent_embed.description + + +@pytest.mark.asyncio +async def test_no_embed(mock_channel: mocks.MockTextChannel): + """Test general response without an embed.""" + content = "Look ma, no embed!" + _ = await responses.send_general_response(mock_channel, content, embed=None) + + assert len(mock_channel.send.mock_calls) == 1 + + _, called_args, _ = mock_channel.send.mock_calls[0] + assert content == called_args[0] + + +@pytest.mark.asyncio +async def test_no_embed_edit(mock_message: mocks.MockMessage): + """Test general response without an embed.""" + content = "Look ma, no embed!" + _ = await responses.send_general_response(None, content, embed=None, message=mock_message) + + assert len(mock_message.edit.mock_calls) == 1 + + _, called_args, _ = mock_message.edit.mock_calls[0] + assert content == called_args[0] + + +@pytest.mark.asyncio +async def test_general_response_embed_edit(mock_message: mocks.MockMessage) -> None: + """Test the positive response embed is correct when editing a message.""" + content = "hello, the code worked I guess!" + _ = await responses.send_general_response(None, content, message=mock_message) + + assert len(mock_message.edit.mock_calls) == 1 + + _, _, called_kwargs = mock_message.edit.mock_calls[0] + sent_embed: discord.Embed = called_kwargs["embed"] + + assert content == sent_embed.description + + +def test_colour_aliases(): + """Test colour aliases are the same.""" + assert responses.DEFAULT_FAILURE_COLOR == responses.DEFAULT_FAILURE_COLOUR + assert responses.DEFAULT_SUCCESS_COLOR == responses.DEFAULT_SUCCESS_COLOUR + + +@pytest.mark.parametrize( + ["coro", "color", "title_list"], + [ + [responses.send_positive_response, responses.DEFAULT_SUCCESS_COLOUR.value, responses.SUCCESS_HEADERS], + [responses.send_negatory_response, responses.DEFAULT_FAILURE_COLOUR.value, responses.FAILURE_HEADERS], + ], +) +@pytest.mark.asyncio +async def test_special_responses( + mock_channel: mocks.MockTextChannel, coro, color: int, title_list: typing.List +): + """Test the positive and negatory response methods.""" + _: unittest.mock.AsyncMock = await coro(mock_channel, "") + + assert len(mock_channel.send.mock_calls) == 1 + + _, _, kwargs = mock_channel.send.mock_calls[0] + embed_dict: dict = kwargs["embed"].to_dict() + + assert color == embed_dict.get("color") + assert embed_dict.get("title") in title_list From 50378c1393824410b78ca2bcf4a45df1b0acaab5 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Wed, 27 Oct 2021 23:09:54 -0400 Subject: [PATCH 79/83] chore: use friendlier tone for error messages Signed-off-by: onerandomusername --- modmail/utils/responses.py | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/modmail/utils/responses.py b/modmail/utils/responses.py index e2a014a3..622dbb52 100644 --- a/modmail/utils/responses.py +++ b/modmail/utils/responses.py @@ -33,37 +33,29 @@ DEFAULT_SUCCESS_COLOUR = discord.Colour.green() DEFAULT_SUCCESS_COLOR = DEFAULT_SUCCESS_COLOUR SUCCESS_HEADERS: typing.List[str] = [ - "You got it.", - "Done.", - "Affirmative.", - "As you wish.", - "Okay.", - "Fine by me.", - "There we go.", + "Affirmative", + "As you wish", + "Done", + "Fine by me", + "There we go", "Sure!", - "Your wish is my command.", + "Okay", + "You got it", + "Your wish is my command", ] DEFAULT_FAILURE_COLOUR = discord.Colour.red() DEFAULT_FAILURE_COLOR = DEFAULT_FAILURE_COLOUR FAILURE_HEADERS: typing.List[str] = [ "Abort!", - "FAIL.", - "I cannot do that.", - "I'm leaving you.", - "Its not me, its you.", + "I cannot do that", "Hold up!", - "Mistakes were made.", - "Nope.", - "Not happening.", - "Oops.", - "Something went wrong.", - "Sorry, no.", - "This will never work.", - "Uh. No.", + "I was unable to interpret that", + "Not understood", + "Oops", + "Something went wrong", "\U0001f914", - "That is not happening.", - "Whups.", + "Unable to complete your command", ] @@ -89,7 +81,7 @@ async def send_general_response( if isinstance(channel, commands.Context): # pragma: nocover channel = channel.channel - logger.debug(f"Requested to send {_kind} message to {channel!s}. Response: {response!s}") + logger.debug(f"Requested to send {_kind} response message to {channel!s}. Response: {response!s}") if embed is None: if message is None: From dd9799cf7d4e73395cb2b827d22f5e4a7b9cf8c1 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Thu, 28 Oct 2021 20:17:28 -0400 Subject: [PATCH 80/83] chore: document the use of the responses helper module better Signed-off-by: onerandomusername --- modmail/utils/responses.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modmail/utils/responses.py b/modmail/utils/responses.py index 622dbb52..2106c706 100644 --- a/modmail/utils/responses.py +++ b/modmail/utils/responses.py @@ -2,6 +2,9 @@ Helper methods for responses from the bot to the user. These help ensure consistency between errors, as they will all be consistent between different uses. + +Note: these are to used for general success or general errors. Typically, the error handler will make a +response if a command raises a discord.ext.commands.CommandError exception. """ import logging import random From 0f6fdcdb945a08dac8359fdeeba42312ea55b4ae Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Thu, 28 Oct 2021 20:23:29 -0400 Subject: [PATCH 81/83] nit: use same error colour as negative response colour Signed-off-by: onerandomusername --- modmail/extensions/utils/error_handler.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modmail/extensions/utils/error_handler.py b/modmail/extensions/utils/error_handler.py index cedd8f1b..181519c6 100644 --- a/modmail/extensions/utils/error_handler.py +++ b/modmail/extensions/utils/error_handler.py @@ -8,6 +8,7 @@ from modmail.bot import ModmailBot from modmail.log import ModmailLogger +from modmail.utils import responses from modmail.utils.cogs import BotModes, ExtMetadata, ModmailCog from modmail.utils.extensions import BOT_MODE @@ -16,7 +17,7 @@ EXT_METADATA = ExtMetadata() -ERROR_COLOUR = discord.Colour.red() +ERROR_COLOUR = responses.DEFAULT_FAILURE_COLOUR ERROR_TITLE_REGEX = re.compile(r"((?<=[a-z])[A-Z]|(?<=[a-zA-Z])[A-Z](?=[a-z]))") From bbef57857ebecdacfe4ed669483644d6ea50495b Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Sat, 30 Oct 2021 15:44:03 -0400 Subject: [PATCH 82/83] minor: make _kind part of the public api --- modmail/utils/responses.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modmail/utils/responses.py b/modmail/utils/responses.py index 2106c706..90a3c7a9 100644 --- a/modmail/utils/responses.py +++ b/modmail/utils/responses.py @@ -70,7 +70,7 @@ async def send_general_response( embed: discord.Embed = _UNSET, colour: discord.Colour = None, title: str = None, - _kind: typing.Literal["general", "affirmative", "negatory"] = "general", + tag_as: typing.Literal["general", "affirmative", "negatory"] = "general", **kwargs, ) -> discord.Message: """ @@ -84,7 +84,7 @@ async def send_general_response( if isinstance(channel, commands.Context): # pragma: nocover channel = channel.channel - logger.debug(f"Requested to send {_kind} response message to {channel!s}. Response: {response!s}") + logger.debug(f"Requested to send {tag_as} response message to {channel!s}. Response: {response!s}") if embed is None: if message is None: @@ -132,7 +132,7 @@ async def send_positive_response( channel=channel, response=response, colour=colour, - _kind="affirmative", + tag_as="affirmative", **kwargs, ) @@ -161,6 +161,6 @@ async def send_negatory_response( channel=channel, response=response, colour=colour, - _kind="negatory", + tag_as="negatory", **kwargs, ) From 5cc28a02b5a6c713db32a22254200d851928d3b2 Mon Sep 17 00:00:00 2001 From: onerandomusername Date: Mon, 1 Nov 2021 04:35:21 -0400 Subject: [PATCH 83/83] fix: only import bot when type checking was causing a recursive error on tests when this was imported --- modmail/utils/cogs.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modmail/utils/cogs.py b/modmail/utils/cogs.py index 2b39e0b7..d8fe5e0b 100644 --- a/modmail/utils/cogs.py +++ b/modmail/utils/cogs.py @@ -1,9 +1,12 @@ from dataclasses import dataclass from enum import IntEnum, auto +from typing import TYPE_CHECKING from discord.ext import commands -import modmail.bot + +if TYPE_CHECKING: # pragma: nocover + import modmail.bot class BitwiseAutoEnum(IntEnum):