From 1f17d0ed894ee6a4c6a9c703d03598d734ffeac2 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sun, 26 Jan 2020 22:20:17 +0100 Subject: Add unit test case for burst antispam rule --- tests/bot/rules/test_burst.py | 69 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 tests/bot/rules/test_burst.py diff --git a/tests/bot/rules/test_burst.py b/tests/bot/rules/test_burst.py new file mode 100644 index 000000000..4da0cc78e --- /dev/null +++ b/tests/bot/rules/test_burst.py @@ -0,0 +1,69 @@ +import unittest + +from bot.rules import burst +from tests.helpers import MockMessage, async_test + + +def make_msg(author: str) -> MockMessage: + """Init a MockMessage instance with author set to `author`. + + This serves as a shorthand / alias to keep the test cases visually clean. + """ + return MockMessage(author=author) + + +class BurstRuleTests(unittest.TestCase): + """Tests the `burst` antispam rule.""" + + def setUp(self): + self.config = {"max": 2, "interval": 10} + + @async_test + async def test_allows_messages_within_limit(self): + """Cases which do not violate the rule.""" + cases = ( + [make_msg("bob"), make_msg("bob")], + [make_msg("bob"), make_msg("alice"), make_msg("bob")], + ) + + for recent_messages in cases: + last_message = recent_messages[0] + + with self.subTest(last_message=last_message, recent_messages=recent_messages, config=self.config): + self.assertIsNone(await burst.apply(last_message, recent_messages, self.config)) + + @async_test + async def test_disallows_messages_beyond_limit(self): + """Cases where the amount of messages exceeds the limit, triggering the rule.""" + cases = ( + ( + [make_msg("bob"), make_msg("bob"), make_msg("bob")], + "bob", + 3, + ), + ( + [make_msg("bob"), make_msg("bob"), make_msg("alice"), make_msg("bob")], + "bob", + 3, + ), + ) + + for recent_messages, culprit, total_msgs in cases: + last_message = recent_messages[0] + relevant_messages = tuple(msg for msg in recent_messages if msg.author == culprit) + expected_output = ( + f"sent {total_msgs} messages in {self.config['interval']}s", + (culprit,), + relevant_messages, + ) + + with self.subTest( + last_message=last_message, + recent_messages=recent_messages, + config=self.config, + expected_output=expected_output, + ): + self.assertTupleEqual( + await burst.apply(last_message, recent_messages, self.config), + expected_output, + ) -- cgit v1.2.3 From b0713be662fae40f58104923534b563b19e79f1d Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sun, 26 Jan 2020 22:20:47 +0100 Subject: Add unit test case for burst shared antispam rule --- tests/bot/rules/test_burst_shared.py | 65 ++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 tests/bot/rules/test_burst_shared.py diff --git a/tests/bot/rules/test_burst_shared.py b/tests/bot/rules/test_burst_shared.py new file mode 100644 index 000000000..1f5662eff --- /dev/null +++ b/tests/bot/rules/test_burst_shared.py @@ -0,0 +1,65 @@ +import unittest + +from bot.rules import burst_shared +from tests.helpers import MockMessage, async_test + + +def make_msg(author: str) -> MockMessage: + """Init a MockMessage instance with the passed arg. + + This serves as a shorthand / alias to keep the test cases visually clean. + """ + return MockMessage(author=author) + + +class BurstSharedRuleTests(unittest.TestCase): + """Tests the `burst_shared` antispam rule.""" + + def setUp(self): + self.config = {"max": 2, "interval": 10} + + @async_test + async def test_allows_messages_within_limit(self): + """Cases that do not violate the rule. + + There really isn't more to test here than a single case. + """ + recent_messages = [make_msg("spongebob"), make_msg("patrick")] + last_message = recent_messages[0] + + self.assertIsNone(await burst_shared.apply(last_message, recent_messages, self.config)) + + @async_test + async def test_disallows_messages_beyond_limit(self): + """Cases where the amount of messages exceeds the limit, triggering the rule.""" + cases = ( + ( + [make_msg("bob"), make_msg("bob"), make_msg("bob")], + {"bob"}, + 3, + ), + ( + [make_msg("bob"), make_msg("bob"), make_msg("alice"), make_msg("bob")], + {"bob", "alice"}, + 4, + ), + ) + + for recent_messages, culprits, total_msgs in cases: + last_message = recent_messages[0] + expected_output = ( + f"sent {total_msgs} messages in {self.config['interval']}s", + culprits, + recent_messages, + ) + + with self.subTest( + last_message=last_message, + recent_messages=recent_messages, + config=self.config, + expected_output=expected_output, + ): + self.assertTupleEqual( + await burst_shared.apply(last_message, recent_messages, self.config), + expected_output, + ) -- cgit v1.2.3 From f15f3184d6e0c2ad4db303528a1954d589d93900 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sun, 26 Jan 2020 22:21:08 +0100 Subject: Add unit test case for chars antispam rule --- tests/bot/rules/test_chars.py | 75 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 tests/bot/rules/test_chars.py diff --git a/tests/bot/rules/test_chars.py b/tests/bot/rules/test_chars.py new file mode 100644 index 000000000..f466a898e --- /dev/null +++ b/tests/bot/rules/test_chars.py @@ -0,0 +1,75 @@ +import unittest + +from bot.rules import chars +from tests.helpers import MockMessage, async_test + + +def make_msg(author: str, n_chars: int) -> MockMessage: + """Build a message with arbitrary content of `n_chars` length.""" + return MockMessage(author=author, content="A" * n_chars) + + +class CharsRuleTests(unittest.TestCase): + """Tests the `chars` antispam rule.""" + + def setUp(self): + self.config = { + "max": 20, # Max allowed sum of chars per user + "interval": 10, + } + + @async_test + async def test_allows_messages_within_limit(self): + """Cases with a total amount of chars within limit.""" + cases = ( + [make_msg("bob", 0)], + [make_msg("bob", 20)], + [make_msg("bob", 15), make_msg("alice", 15)], + ) + + for recent_messages in cases: + last_message = recent_messages[0] + + with self.subTest(last_message=last_message, recent_messages=recent_messages, config=self.config): + self.assertIsNone(await chars.apply(last_message, recent_messages, self.config)) + + @async_test + async def test_disallows_messages_beyond_limit(self): + """Cases where the total amount of chars exceeds the limit, triggering the rule.""" + cases = ( + ( + [make_msg("bob", 21)], + "bob", + 21, + ), + ( + [make_msg("bob", 15), make_msg("bob", 15)], + "bob", + 30, + ), + ( + [make_msg("alice", 15), make_msg("bob", 20), make_msg("alice", 15)], + "alice", + 30, + ), + ) + + for recent_messages, culprit, total_chars in cases: + last_message = recent_messages[0] + relevant_messages = tuple(msg for msg in recent_messages if msg.author == culprit) + expected_output = ( + f"sent {total_chars} characters in {self.config['interval']}s", + (culprit,), + relevant_messages, + ) + + with self.subTest( + last_message=last_message, + recent_messages=recent_messages, + config=self.config, + expected_output=expected_output, + ): + self.assertTupleEqual( + await chars.apply(last_message, recent_messages, self.config), + expected_output, + ) -- cgit v1.2.3 From 4bfe30def137921a4320208b66472dc97c5bd298 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sun, 26 Jan 2020 22:21:28 +0100 Subject: Add unit test case for discord emojis antispam rule --- tests/bot/rules/test_discord_emojis.py | 68 ++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 tests/bot/rules/test_discord_emojis.py diff --git a/tests/bot/rules/test_discord_emojis.py b/tests/bot/rules/test_discord_emojis.py new file mode 100644 index 000000000..1c56c9563 --- /dev/null +++ b/tests/bot/rules/test_discord_emojis.py @@ -0,0 +1,68 @@ +import unittest + +from bot.rules import discord_emojis +from tests.helpers import MockMessage, async_test + +discord_emoji = "<:abcd:1234>" # Discord emojis follow the format <:name:id> + + +def make_msg(author: str, n_emojis: int) -> MockMessage: + """Build a MockMessage instance with content containing `n_emojis` arbitrary emojis.""" + return MockMessage(author=author, content=discord_emoji * n_emojis) + + +class DiscordEmojisRuleTests(unittest.TestCase): + """Tests for the `discord_emojis` antispam rule.""" + + def setUp(self): + self.config = {"max": 2, "interval": 10} + + @async_test + async def test_allows_messages_within_limit(self): + """Cases with a total amount of discord emojis within limit.""" + cases = ( + [make_msg("bob", 2)], + [make_msg("alice", 1), make_msg("bob", 2), make_msg("alice", 1)], + ) + + for recent_messages in cases: + last_message = recent_messages[0] + + with self.subTest(last_message=last_message, recent_messages=recent_messages, config=self.config): + self.assertIsNone(await discord_emojis.apply(last_message, recent_messages, self.config)) + + @async_test + async def test_disallows_messages_beyond_limit(self): + """Cases with more than the allowed amount of discord emojis.""" + cases = ( + ( + [make_msg("bob", 3)], + "bob", + 3, + ), + ( + [make_msg("alice", 2), make_msg("bob", 2), make_msg("alice", 2)], + "alice", + 4, + ), + ) + + for recent_messages, culprit, total_emojis in cases: + last_message = recent_messages[0] + relevant_messages = tuple(msg for msg in recent_messages if msg.author == culprit) + expected_output = ( + f"sent {total_emojis} emojis in {self.config['interval']}s", + (culprit,), + relevant_messages, + ) + + with self.subTest( + last_message=last_message, + recent_messages=recent_messages, + config=self.config, + expected_output=expected_output, + ): + self.assertTupleEqual( + await discord_emojis.apply(last_message, recent_messages, self.config), + expected_output, + ) -- cgit v1.2.3 From f32ffaffd92b0521adec42432121aefb3d596b0e Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sun, 26 Jan 2020 22:21:42 +0100 Subject: Add unit test case for role mentions antispam rule --- tests/bot/rules/test_role_mentions.py | 66 +++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 tests/bot/rules/test_role_mentions.py diff --git a/tests/bot/rules/test_role_mentions.py b/tests/bot/rules/test_role_mentions.py new file mode 100644 index 000000000..6377ffbc8 --- /dev/null +++ b/tests/bot/rules/test_role_mentions.py @@ -0,0 +1,66 @@ +import unittest + +from bot.rules import role_mentions +from tests.helpers import MockMessage, async_test + + +def make_msg(author: str, n_mentions: int) -> MockMessage: + """Build a MockMessage instance with `n_mentions` role mentions.""" + return MockMessage(author=author, role_mentions=[None] * n_mentions) + + +class RoleMentionsRuleTests(unittest.TestCase): + """Tests for the `role_mentions` antispam rule.""" + + def setUp(self): + self.config = {"max": 2, "interval": 10} + + @async_test + async def test_allows_messages_within_limit(self): + """Cases with a total amount of role mentions within limit.""" + cases = ( + [make_msg("bob", 2)], + [make_msg("bob", 1), make_msg("alice", 1), make_msg("bob", 1)], + ) + + for recent_messages in cases: + last_message = recent_messages[0] + + with self.subTest(last_message=last_message, recent_messages=recent_messages, config=self.config): + self.assertIsNone(await role_mentions.apply(last_message, recent_messages, self.config)) + + @async_test + async def test_disallows_messages_beyond_limit(self): + """Cases with more than the allowed amount of role mentions.""" + cases = ( + ( + [make_msg("bob", 3)], + "bob", + 3, + ), + ( + [make_msg("alice", 2), make_msg("bob", 2), make_msg("alice", 2)], + "alice", + 4, + ), + ) + + for recent_messages, culprit, total_mentions in cases: + last_message = recent_messages[0] + relevant_messages = tuple(msg for msg in recent_messages if msg.author == culprit) + expected_output = ( + f"sent {total_mentions} role mentions in {self.config['interval']}s", + (culprit,), + relevant_messages, + ) + + with self.subTest( + last_message=last_message, + recent_messages=recent_messages, + config=self.config, + expected_output=expected_output, + ): + self.assertTupleEqual( + await role_mentions.apply(last_message, recent_messages, self.config), + expected_output, + ) -- cgit v1.2.3 From 4199b12da2dedd8720cf521a75a821811af59cfd Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sun, 26 Jan 2020 22:30:12 +0100 Subject: Fix incorrect config key in attachments antispam rule The rule was incorrectly printing out the maximum amount of allowed attachments instead of the configured interval. This commit also adjusts the rule's unit test case. --- bot/rules/attachments.py | 2 +- tests/bot/rules/test_attachments.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bot/rules/attachments.py b/bot/rules/attachments.py index 00bb2a949..8903c385c 100644 --- a/bot/rules/attachments.py +++ b/bot/rules/attachments.py @@ -19,7 +19,7 @@ async def apply( if total_recent_attachments > config['max']: return ( - f"sent {total_recent_attachments} attachments in {config['max']}s", + f"sent {total_recent_attachments} attachments in {config['interval']}s", (last_message.author,), relevant_messages ) diff --git a/tests/bot/rules/test_attachments.py b/tests/bot/rules/test_attachments.py index d7187f315..0af5ff0dc 100644 --- a/tests/bot/rules/test_attachments.py +++ b/tests/bot/rules/test_attachments.py @@ -20,7 +20,7 @@ class AttachmentRuleTests(unittest.TestCase): """Tests applying the `attachments` antispam rule.""" def setUp(self): - self.config = {"max": 5} + self.config = {"max": 5, "interval": 10} @async_test async def test_allows_messages_without_too_many_attachments(self): @@ -88,7 +88,7 @@ class AttachmentRuleTests(unittest.TestCase): config=self.config ): desired_output = ( - f"sent {total_attachments} attachments in {self.config['max']}s", + f"sent {total_attachments} attachments in {self.config['interval']}s", culprit, relevant_messages ) -- cgit v1.2.3 From aedbd6f697d6c66b3dbefa894795c86c6d0a2628 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sun, 26 Jan 2020 22:34:52 +0100 Subject: Refactor msg helper function name to make_msg The name msg is less descriptive and creates a needless name conflict in local gen exp. --- tests/bot/rules/test_attachments.py | 16 ++++++++-------- tests/bot/rules/test_links.py | 18 +++++++++--------- tests/bot/rules/test_mentions.py | 16 ++++++++-------- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/tests/bot/rules/test_attachments.py b/tests/bot/rules/test_attachments.py index 0af5ff0dc..419336417 100644 --- a/tests/bot/rules/test_attachments.py +++ b/tests/bot/rules/test_attachments.py @@ -11,7 +11,7 @@ class Case(NamedTuple): total_attachments: int -def msg(author: str, total_attachments: int) -> MockMessage: +def make_msg(author: str, total_attachments: int) -> MockMessage: """Builds a message with `total_attachments` attachments.""" return MockMessage(author=author, attachments=list(range(total_attachments))) @@ -26,9 +26,9 @@ class AttachmentRuleTests(unittest.TestCase): async def test_allows_messages_without_too_many_attachments(self): """Messages without too many attachments are allowed as-is.""" cases = ( - [msg("bob", 0), msg("bob", 0), msg("bob", 0)], - [msg("bob", 2), msg("bob", 2)], - [msg("bob", 2), msg("alice", 2), msg("bob", 2)], + [make_msg("bob", 0), make_msg("bob", 0), make_msg("bob", 0)], + [make_msg("bob", 2), make_msg("bob", 2)], + [make_msg("bob", 2), make_msg("alice", 2), make_msg("bob", 2)], ) for recent_messages in cases: @@ -48,22 +48,22 @@ class AttachmentRuleTests(unittest.TestCase): """Messages with too many attachments trigger the rule.""" cases = ( Case( - [msg("bob", 4), msg("bob", 0), msg("bob", 6)], + [make_msg("bob", 4), make_msg("bob", 0), make_msg("bob", 6)], ("bob",), 10 ), Case( - [msg("bob", 4), msg("alice", 6), msg("bob", 2)], + [make_msg("bob", 4), make_msg("alice", 6), make_msg("bob", 2)], ("bob",), 6 ), Case( - [msg("alice", 6)], + [make_msg("alice", 6)], ("alice",), 6 ), ( - [msg("alice", 1) for _ in range(6)], + [make_msg("alice", 1) for _ in range(6)], ("alice",), 6 ), diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index 02a5d5501..b77e01c84 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -11,7 +11,7 @@ class Case(NamedTuple): total_links: int -def msg(author: str, total_links: int) -> MockMessage: +def make_msg(author: str, total_links: int) -> MockMessage: """Makes a message with `total_links` links.""" content = " ".join(["https://pydis.com"] * total_links) return MockMessage(author=author, content=content) @@ -30,11 +30,11 @@ class LinksTests(unittest.TestCase): async def test_links_within_limit(self): """Messages with an allowed amount of links.""" cases = ( - [msg("bob", 0)], - [msg("bob", 2)], - [msg("bob", 3)], # Filter only applies if len(messages_with_links) > 1 - [msg("bob", 1), msg("bob", 1)], - [msg("bob", 2), msg("alice", 2)] # Only messages from latest author count + [make_msg("bob", 0)], + [make_msg("bob", 2)], + [make_msg("bob", 3)], # Filter only applies if len(messages_with_links) > 1 + [make_msg("bob", 1), make_msg("bob", 1)], + [make_msg("bob", 2), make_msg("alice", 2)] # Only messages from latest author count ) for recent_messages in cases: @@ -54,17 +54,17 @@ class LinksTests(unittest.TestCase): """Messages with a a higher than allowed amount of links.""" cases = ( Case( - [msg("bob", 1), msg("bob", 2)], + [make_msg("bob", 1), make_msg("bob", 2)], ("bob",), 3 ), Case( - [msg("alice", 1), msg("alice", 1), msg("alice", 1)], + [make_msg("alice", 1), make_msg("alice", 1), make_msg("alice", 1)], ("alice",), 3 ), Case( - [msg("alice", 2), msg("bob", 3), msg("alice", 1)], + [make_msg("alice", 2), make_msg("bob", 3), make_msg("alice", 1)], ("alice",), 3 ) diff --git a/tests/bot/rules/test_mentions.py b/tests/bot/rules/test_mentions.py index ad49ead32..43211f097 100644 --- a/tests/bot/rules/test_mentions.py +++ b/tests/bot/rules/test_mentions.py @@ -11,7 +11,7 @@ class Case(NamedTuple): total_mentions: int -def msg(author: str, total_mentions: int) -> MockMessage: +def make_msg(author: str, total_mentions: int) -> MockMessage: """Makes a message with `total_mentions` mentions.""" return MockMessage(author=author, mentions=list(range(total_mentions))) @@ -29,10 +29,10 @@ class TestMentions(unittest.TestCase): async def test_mentions_within_limit(self): """Messages with an allowed amount of mentions.""" cases = ( - [msg("bob", 0)], - [msg("bob", 2)], - [msg("bob", 1), msg("bob", 1)], - [msg("bob", 1), msg("alice", 2)] + [make_msg("bob", 0)], + [make_msg("bob", 2)], + [make_msg("bob", 1), make_msg("bob", 1)], + [make_msg("bob", 1), make_msg("alice", 2)] ) for recent_messages in cases: @@ -52,17 +52,17 @@ class TestMentions(unittest.TestCase): """Messages with a higher than allowed amount of mentions.""" cases = ( Case( - [msg("bob", 3)], + [make_msg("bob", 3)], ("bob",), 3 ), Case( - [msg("alice", 2), msg("alice", 0), msg("alice", 1)], + [make_msg("alice", 2), make_msg("alice", 0), make_msg("alice", 1)], ("alice",), 3 ), Case( - [msg("bob", 2), msg("alice", 3), msg("bob", 2)], + [make_msg("bob", 2), make_msg("alice", 3), make_msg("bob", 2)], ("bob",), 4 ) -- cgit v1.2.3 From 8456331140bdd6bdc1d2c8bf395b69febef45ad8 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Fri, 31 Jan 2020 20:14:43 +0100 Subject: Adjust multi-line docstrings to prevailing style --- tests/bot/rules/test_burst.py | 3 ++- tests/bot/rules/test_burst_shared.py | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/bot/rules/test_burst.py b/tests/bot/rules/test_burst.py index 4da0cc78e..afcc5554d 100644 --- a/tests/bot/rules/test_burst.py +++ b/tests/bot/rules/test_burst.py @@ -5,7 +5,8 @@ from tests.helpers import MockMessage, async_test def make_msg(author: str) -> MockMessage: - """Init a MockMessage instance with author set to `author`. + """ + Init a MockMessage instance with author set to `author`. This serves as a shorthand / alias to keep the test cases visually clean. """ diff --git a/tests/bot/rules/test_burst_shared.py b/tests/bot/rules/test_burst_shared.py index 1f5662eff..401e0b666 100644 --- a/tests/bot/rules/test_burst_shared.py +++ b/tests/bot/rules/test_burst_shared.py @@ -5,7 +5,8 @@ from tests.helpers import MockMessage, async_test def make_msg(author: str) -> MockMessage: - """Init a MockMessage instance with the passed arg. + """ + Init a MockMessage instance with the passed arg. This serves as a shorthand / alias to keep the test cases visually clean. """ @@ -20,7 +21,8 @@ class BurstSharedRuleTests(unittest.TestCase): @async_test async def test_allows_messages_within_limit(self): - """Cases that do not violate the rule. + """ + Cases that do not violate the rule. There really isn't more to test here than a single case. """ -- cgit v1.2.3 From 93d19f378e206135286e376377e7fc8f5bdcc7a2 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sun, 2 Feb 2020 18:25:07 +0100 Subject: Implement RuleTest ABC This will serve as an ABC for tests for individual rules. The base class provides runners for allowed and disallowed cases, and the children classes then only provide the cases and implementations of helper methods specific to each rule. --- tests/bot/rules/__init__.py | 76 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/tests/bot/rules/__init__.py b/tests/bot/rules/__init__.py index e69de29bb..d7cd7b66b 100644 --- a/tests/bot/rules/__init__.py +++ b/tests/bot/rules/__init__.py @@ -0,0 +1,76 @@ +import unittest +from abc import abstractmethod +from typing import Callable, Dict, Iterable, List, NamedTuple, Tuple + +from tests.helpers import MockMessage + + +class DisallowedCase(NamedTuple): + """Encapsulation for test cases expected to fail.""" + recent_messages: List[MockMessage] + culprits: Iterable[str] + n_violations: int + + +class RuleTest(unittest.TestCase): + """ + Abstract class for antispam rule test cases. + + Tests for specific rules should inherit from `RuleTest` and implement + `relevant_messages` and `get_report`. Each instance should also set the + `apply` and `config` attributes as necessary. + + The execution of test cases can then be delegated to the `run_allowed` + and `run_disallowed` methods. + """ + + apply: Callable # The tested rule's apply function + config: Dict[str, int] + + async def run_allowed(self, cases: Tuple[List[MockMessage], ...]) -> None: + """Run all `cases` against `self.apply` expecting them to pass.""" + for recent_messages in cases: + last_message = recent_messages[0] + + with self.subTest( + last_message=last_message, + recent_messages=recent_messages, + config=self.config, + ): + self.assertIsNone( + await self.apply(last_message, recent_messages, self.config) + ) + + async def run_disallowed(self, cases: Tuple[DisallowedCase, ...]) -> None: + """Run all `cases` against `self.apply` expecting them to fail.""" + for case in cases: + recent_messages, culprits, n_violations = case + last_message = recent_messages[0] + relevant_messages = self.relevant_messages(case) + desired_output = ( + self.get_report(case), + culprits, + relevant_messages, + ) + + with self.subTest( + last_message=last_message, + recent_messages=recent_messages, + relevant_messages=relevant_messages, + n_violations=n_violations, + config=self.config, + ): + self.assertTupleEqual( + await self.apply(last_message, recent_messages, self.config), + desired_output, + ) + + @abstractmethod + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: + """Give expected relevant messages for `case`.""" + raise NotImplementedError + + @abstractmethod + def get_report(self, case: DisallowedCase) -> str: + """Give expected error report for `case`.""" + raise NotImplementedError -- cgit v1.2.3 From b89f9c55329aa44448d55963e22ce4f7a6ec0ff6 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sun, 2 Feb 2020 18:31:50 +0100 Subject: Adjust existing tests to inherit from RuleTest ABC --- tests/bot/rules/test_attachments.py | 79 +++++++++++----------------------- tests/bot/rules/test_burst.py | 44 +++++++------------ tests/bot/rules/test_burst_shared.py | 40 +++++++---------- tests/bot/rules/test_chars.py | 53 ++++++++++------------- tests/bot/rules/test_discord_emojis.py | 44 +++++++------------ tests/bot/rules/test_links.py | 66 ++++++++-------------------- tests/bot/rules/test_mentions.py | 76 +++++++++++--------------------- tests/bot/rules/test_role_mentions.py | 49 +++++++++------------ 8 files changed, 157 insertions(+), 294 deletions(-) diff --git a/tests/bot/rules/test_attachments.py b/tests/bot/rules/test_attachments.py index 419336417..e54b4b5b8 100644 --- a/tests/bot/rules/test_attachments.py +++ b/tests/bot/rules/test_attachments.py @@ -1,25 +1,20 @@ -import unittest -from typing import List, NamedTuple, Tuple +from typing import Iterable from bot.rules import attachments +from tests.bot.rules import DisallowedCase, RuleTest from tests.helpers import MockMessage, async_test -class Case(NamedTuple): - recent_messages: List[MockMessage] - culprit: Tuple[str] - total_attachments: int - - def make_msg(author: str, total_attachments: int) -> MockMessage: """Builds a message with `total_attachments` attachments.""" return MockMessage(author=author, attachments=list(range(total_attachments))) -class AttachmentRuleTests(unittest.TestCase): +class AttachmentRuleTests(RuleTest): """Tests applying the `attachments` antispam rule.""" def setUp(self): + self.apply = attachments.apply self.config = {"max": 5, "interval": 10} @async_test @@ -31,68 +26,46 @@ class AttachmentRuleTests(unittest.TestCase): [make_msg("bob", 2), make_msg("alice", 2), make_msg("bob", 2)], ) - for recent_messages in cases: - last_message = recent_messages[0] - - with self.subTest( - last_message=last_message, - recent_messages=recent_messages, - config=self.config - ): - self.assertIsNone( - await attachments.apply(last_message, recent_messages, self.config) - ) + await self.run_allowed(cases) @async_test async def test_disallows_messages_with_too_many_attachments(self): """Messages with too many attachments trigger the rule.""" cases = ( - Case( + DisallowedCase( [make_msg("bob", 4), make_msg("bob", 0), make_msg("bob", 6)], ("bob",), - 10 + 10, ), - Case( + DisallowedCase( [make_msg("bob", 4), make_msg("alice", 6), make_msg("bob", 2)], ("bob",), - 6 + 6, ), - Case( + DisallowedCase( [make_msg("alice", 6)], ("alice",), - 6 + 6, ), - ( + DisallowedCase( [make_msg("alice", 1) for _ in range(6)], ("alice",), - 6 + 6, ), ) - for recent_messages, culprit, total_attachments in cases: - last_message = recent_messages[0] - relevant_messages = tuple( - msg - for msg in recent_messages - if ( - msg.author == last_message.author - and len(msg.attachments) > 0 - ) + await self.run_disallowed(cases) + + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: + last_message = case.recent_messages[0] + return tuple( + msg + for msg in case.recent_messages + if ( + msg.author == last_message.author + and len(msg.attachments) > 0 ) + ) - with self.subTest( - last_message=last_message, - recent_messages=recent_messages, - relevant_messages=relevant_messages, - total_attachments=total_attachments, - config=self.config - ): - desired_output = ( - f"sent {total_attachments} attachments in {self.config['interval']}s", - culprit, - relevant_messages - ) - self.assertTupleEqual( - await attachments.apply(last_message, recent_messages, self.config), - desired_output - ) + def get_report(self, case: DisallowedCase) -> str: + return f"sent {case.n_violations} attachments in {self.config['interval']}s" diff --git a/tests/bot/rules/test_burst.py b/tests/bot/rules/test_burst.py index afcc5554d..72f0be0c7 100644 --- a/tests/bot/rules/test_burst.py +++ b/tests/bot/rules/test_burst.py @@ -1,6 +1,7 @@ -import unittest +from typing import Iterable from bot.rules import burst +from tests.bot.rules import DisallowedCase, RuleTest from tests.helpers import MockMessage, async_test @@ -13,10 +14,11 @@ def make_msg(author: str) -> MockMessage: return MockMessage(author=author) -class BurstRuleTests(unittest.TestCase): +class BurstRuleTests(RuleTest): """Tests the `burst` antispam rule.""" def setUp(self): + self.apply = burst.apply self.config = {"max": 2, "interval": 10} @async_test @@ -27,44 +29,28 @@ class BurstRuleTests(unittest.TestCase): [make_msg("bob"), make_msg("alice"), make_msg("bob")], ) - for recent_messages in cases: - last_message = recent_messages[0] - - with self.subTest(last_message=last_message, recent_messages=recent_messages, config=self.config): - self.assertIsNone(await burst.apply(last_message, recent_messages, self.config)) + await self.run_allowed(cases) @async_test async def test_disallows_messages_beyond_limit(self): """Cases where the amount of messages exceeds the limit, triggering the rule.""" cases = ( - ( + DisallowedCase( [make_msg("bob"), make_msg("bob"), make_msg("bob")], - "bob", + ("bob",), 3, ), - ( + DisallowedCase( [make_msg("bob"), make_msg("bob"), make_msg("alice"), make_msg("bob")], - "bob", + ("bob",), 3, ), ) - for recent_messages, culprit, total_msgs in cases: - last_message = recent_messages[0] - relevant_messages = tuple(msg for msg in recent_messages if msg.author == culprit) - expected_output = ( - f"sent {total_msgs} messages in {self.config['interval']}s", - (culprit,), - relevant_messages, - ) + await self.run_disallowed(cases) + + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: + return tuple(msg for msg in case.recent_messages if msg.author in case.culprits) - with self.subTest( - last_message=last_message, - recent_messages=recent_messages, - config=self.config, - expected_output=expected_output, - ): - self.assertTupleEqual( - await burst.apply(last_message, recent_messages, self.config), - expected_output, - ) + def get_report(self, case: DisallowedCase) -> str: + return f"sent {case.n_violations} messages in {self.config['interval']}s" diff --git a/tests/bot/rules/test_burst_shared.py b/tests/bot/rules/test_burst_shared.py index 401e0b666..47367a5f8 100644 --- a/tests/bot/rules/test_burst_shared.py +++ b/tests/bot/rules/test_burst_shared.py @@ -1,6 +1,7 @@ -import unittest +from typing import Iterable from bot.rules import burst_shared +from tests.bot.rules import DisallowedCase, RuleTest from tests.helpers import MockMessage, async_test @@ -13,10 +14,11 @@ def make_msg(author: str) -> MockMessage: return MockMessage(author=author) -class BurstSharedRuleTests(unittest.TestCase): +class BurstSharedRuleTests(RuleTest): """Tests the `burst_shared` antispam rule.""" def setUp(self): + self.apply = burst_shared.apply self.config = {"max": 2, "interval": 10} @async_test @@ -26,42 +28,32 @@ class BurstSharedRuleTests(unittest.TestCase): There really isn't more to test here than a single case. """ - recent_messages = [make_msg("spongebob"), make_msg("patrick")] - last_message = recent_messages[0] + cases = ( + [make_msg("spongebob"), make_msg("patrick")], + ) - self.assertIsNone(await burst_shared.apply(last_message, recent_messages, self.config)) + await self.run_allowed(cases) @async_test async def test_disallows_messages_beyond_limit(self): """Cases where the amount of messages exceeds the limit, triggering the rule.""" cases = ( - ( + DisallowedCase( [make_msg("bob"), make_msg("bob"), make_msg("bob")], {"bob"}, 3, ), - ( + DisallowedCase( [make_msg("bob"), make_msg("bob"), make_msg("alice"), make_msg("bob")], {"bob", "alice"}, 4, ), ) - for recent_messages, culprits, total_msgs in cases: - last_message = recent_messages[0] - expected_output = ( - f"sent {total_msgs} messages in {self.config['interval']}s", - culprits, - recent_messages, - ) + await self.run_disallowed(cases) + + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: + return case.recent_messages - with self.subTest( - last_message=last_message, - recent_messages=recent_messages, - config=self.config, - expected_output=expected_output, - ): - self.assertTupleEqual( - await burst_shared.apply(last_message, recent_messages, self.config), - expected_output, - ) + def get_report(self, case: DisallowedCase) -> str: + return f"sent {case.n_violations} messages in {self.config['interval']}s" diff --git a/tests/bot/rules/test_chars.py b/tests/bot/rules/test_chars.py index f466a898e..7cc36f49e 100644 --- a/tests/bot/rules/test_chars.py +++ b/tests/bot/rules/test_chars.py @@ -1,6 +1,7 @@ -import unittest +from typing import Iterable from bot.rules import chars +from tests.bot.rules import DisallowedCase, RuleTest from tests.helpers import MockMessage, async_test @@ -9,10 +10,11 @@ def make_msg(author: str, n_chars: int) -> MockMessage: return MockMessage(author=author, content="A" * n_chars) -class CharsRuleTests(unittest.TestCase): +class CharsRuleTests(RuleTest): """Tests the `chars` antispam rule.""" def setUp(self): + self.apply = chars.apply self.config = { "max": 20, # Max allowed sum of chars per user "interval": 10, @@ -27,49 +29,38 @@ class CharsRuleTests(unittest.TestCase): [make_msg("bob", 15), make_msg("alice", 15)], ) - for recent_messages in cases: - last_message = recent_messages[0] - - with self.subTest(last_message=last_message, recent_messages=recent_messages, config=self.config): - self.assertIsNone(await chars.apply(last_message, recent_messages, self.config)) + await self.run_allowed(cases) @async_test async def test_disallows_messages_beyond_limit(self): """Cases where the total amount of chars exceeds the limit, triggering the rule.""" cases = ( - ( + DisallowedCase( [make_msg("bob", 21)], - "bob", + ("bob",), 21, ), - ( + DisallowedCase( [make_msg("bob", 15), make_msg("bob", 15)], - "bob", + ("bob",), 30, ), - ( + DisallowedCase( [make_msg("alice", 15), make_msg("bob", 20), make_msg("alice", 15)], - "alice", + ("alice",), 30, ), ) - for recent_messages, culprit, total_chars in cases: - last_message = recent_messages[0] - relevant_messages = tuple(msg for msg in recent_messages if msg.author == culprit) - expected_output = ( - f"sent {total_chars} characters in {self.config['interval']}s", - (culprit,), - relevant_messages, - ) + await self.run_disallowed(cases) + + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: + last_message = case.recent_messages[0] + return tuple( + msg + for msg in case.recent_messages + if msg.author == last_message.author + ) - with self.subTest( - last_message=last_message, - recent_messages=recent_messages, - config=self.config, - expected_output=expected_output, - ): - self.assertTupleEqual( - await chars.apply(last_message, recent_messages, self.config), - expected_output, - ) + def get_report(self, case: DisallowedCase) -> str: + return f"sent {case.n_violations} characters in {self.config['interval']}s" diff --git a/tests/bot/rules/test_discord_emojis.py b/tests/bot/rules/test_discord_emojis.py index 1c56c9563..0239b0b00 100644 --- a/tests/bot/rules/test_discord_emojis.py +++ b/tests/bot/rules/test_discord_emojis.py @@ -1,6 +1,7 @@ -import unittest +from typing import Iterable from bot.rules import discord_emojis +from tests.bot.rules import DisallowedCase, RuleTest from tests.helpers import MockMessage, async_test discord_emoji = "<:abcd:1234>" # Discord emojis follow the format <:name:id> @@ -11,10 +12,11 @@ def make_msg(author: str, n_emojis: int) -> MockMessage: return MockMessage(author=author, content=discord_emoji * n_emojis) -class DiscordEmojisRuleTests(unittest.TestCase): +class DiscordEmojisRuleTests(RuleTest): """Tests for the `discord_emojis` antispam rule.""" def setUp(self): + self.apply = discord_emojis.apply self.config = {"max": 2, "interval": 10} @async_test @@ -25,44 +27,28 @@ class DiscordEmojisRuleTests(unittest.TestCase): [make_msg("alice", 1), make_msg("bob", 2), make_msg("alice", 1)], ) - for recent_messages in cases: - last_message = recent_messages[0] - - with self.subTest(last_message=last_message, recent_messages=recent_messages, config=self.config): - self.assertIsNone(await discord_emojis.apply(last_message, recent_messages, self.config)) + await self.run_allowed(cases) @async_test async def test_disallows_messages_beyond_limit(self): """Cases with more than the allowed amount of discord emojis.""" cases = ( - ( + DisallowedCase( [make_msg("bob", 3)], - "bob", + ("bob",), 3, ), - ( + DisallowedCase( [make_msg("alice", 2), make_msg("bob", 2), make_msg("alice", 2)], - "alice", + ("alice",), 4, ), ) - for recent_messages, culprit, total_emojis in cases: - last_message = recent_messages[0] - relevant_messages = tuple(msg for msg in recent_messages if msg.author == culprit) - expected_output = ( - f"sent {total_emojis} emojis in {self.config['interval']}s", - (culprit,), - relevant_messages, - ) + await self.run_disallowed(cases) + + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: + return tuple(msg for msg in case.recent_messages if msg.author in case.culprits) - with self.subTest( - last_message=last_message, - recent_messages=recent_messages, - config=self.config, - expected_output=expected_output, - ): - self.assertTupleEqual( - await discord_emojis.apply(last_message, recent_messages, self.config), - expected_output, - ) + def get_report(self, case: DisallowedCase) -> str: + return f"sent {case.n_violations} emojis in {self.config['interval']}s" diff --git a/tests/bot/rules/test_links.py b/tests/bot/rules/test_links.py index b77e01c84..3c3f90e5f 100644 --- a/tests/bot/rules/test_links.py +++ b/tests/bot/rules/test_links.py @@ -1,26 +1,21 @@ -import unittest -from typing import List, NamedTuple, Tuple +from typing import Iterable from bot.rules import links +from tests.bot.rules import DisallowedCase, RuleTest from tests.helpers import MockMessage, async_test -class Case(NamedTuple): - recent_messages: List[MockMessage] - culprit: Tuple[str] - total_links: int - - def make_msg(author: str, total_links: int) -> MockMessage: """Makes a message with `total_links` links.""" content = " ".join(["https://pydis.com"] * total_links) return MockMessage(author=author, content=content) -class LinksTests(unittest.TestCase): +class LinksTests(RuleTest): """Tests applying the `links` rule.""" def setUp(self): + self.apply = links.apply self.config = { "max": 2, "interval": 10 @@ -37,61 +32,38 @@ class LinksTests(unittest.TestCase): [make_msg("bob", 2), make_msg("alice", 2)] # Only messages from latest author count ) - for recent_messages in cases: - last_message = recent_messages[0] - - with self.subTest( - last_message=last_message, - recent_messages=recent_messages, - config=self.config - ): - self.assertIsNone( - await links.apply(last_message, recent_messages, self.config) - ) + await self.run_allowed(cases) @async_test async def test_links_exceeding_limit(self): """Messages with a a higher than allowed amount of links.""" cases = ( - Case( + DisallowedCase( [make_msg("bob", 1), make_msg("bob", 2)], ("bob",), 3 ), - Case( + DisallowedCase( [make_msg("alice", 1), make_msg("alice", 1), make_msg("alice", 1)], ("alice",), 3 ), - Case( + DisallowedCase( [make_msg("alice", 2), make_msg("bob", 3), make_msg("alice", 1)], ("alice",), 3 ) ) - for recent_messages, culprit, total_links in cases: - last_message = recent_messages[0] - relevant_messages = tuple( - msg - for msg in recent_messages - if msg.author == last_message.author - ) + await self.run_disallowed(cases) + + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: + last_message = case.recent_messages[0] + return tuple( + msg + for msg in case.recent_messages + if msg.author == last_message.author + ) - with self.subTest( - last_message=last_message, - recent_messages=recent_messages, - relevant_messages=relevant_messages, - culprit=culprit, - total_links=total_links, - config=self.config - ): - desired_output = ( - f"sent {total_links} links in {self.config['interval']}s", - culprit, - relevant_messages - ) - self.assertTupleEqual( - await links.apply(last_message, recent_messages, self.config), - desired_output - ) + def get_report(self, case: DisallowedCase) -> str: + return f"sent {case.n_violations} links in {self.config['interval']}s" diff --git a/tests/bot/rules/test_mentions.py b/tests/bot/rules/test_mentions.py index 43211f097..ebcdabac6 100644 --- a/tests/bot/rules/test_mentions.py +++ b/tests/bot/rules/test_mentions.py @@ -1,28 +1,23 @@ -import unittest -from typing import List, NamedTuple, Tuple +from typing import Iterable from bot.rules import mentions +from tests.bot.rules import DisallowedCase, RuleTest from tests.helpers import MockMessage, async_test -class Case(NamedTuple): - recent_messages: List[MockMessage] - culprit: Tuple[str] - total_mentions: int - - def make_msg(author: str, total_mentions: int) -> MockMessage: """Makes a message with `total_mentions` mentions.""" return MockMessage(author=author, mentions=list(range(total_mentions))) -class TestMentions(unittest.TestCase): +class TestMentions(RuleTest): """Tests applying the `mentions` antispam rule.""" def setUp(self): + self.apply = mentions.apply self.config = { "max": 2, - "interval": 10 + "interval": 10, } @async_test @@ -32,64 +27,41 @@ class TestMentions(unittest.TestCase): [make_msg("bob", 0)], [make_msg("bob", 2)], [make_msg("bob", 1), make_msg("bob", 1)], - [make_msg("bob", 1), make_msg("alice", 2)] + [make_msg("bob", 1), make_msg("alice", 2)], ) - for recent_messages in cases: - last_message = recent_messages[0] - - with self.subTest( - last_message=last_message, - recent_messages=recent_messages, - config=self.config - ): - self.assertIsNone( - await mentions.apply(last_message, recent_messages, self.config) - ) + await self.run_allowed(cases) @async_test async def test_mentions_exceeding_limit(self): """Messages with a higher than allowed amount of mentions.""" cases = ( - Case( + DisallowedCase( [make_msg("bob", 3)], ("bob",), - 3 + 3, ), - Case( + DisallowedCase( [make_msg("alice", 2), make_msg("alice", 0), make_msg("alice", 1)], ("alice",), - 3 + 3, ), - Case( + DisallowedCase( [make_msg("bob", 2), make_msg("alice", 3), make_msg("bob", 2)], ("bob",), - 4 + 4, ) ) - for recent_messages, culprit, total_mentions in cases: - last_message = recent_messages[0] - relevant_messages = tuple( - msg - for msg in recent_messages - if msg.author == last_message.author - ) + await self.run_disallowed(cases) + + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: + last_message = case.recent_messages[0] + return tuple( + msg + for msg in case.recent_messages + if msg.author == last_message.author + ) - with self.subTest( - last_message=last_message, - recent_messages=recent_messages, - relevant_messages=relevant_messages, - culprit=culprit, - total_mentions=total_mentions, - cofig=self.config - ): - desired_output = ( - f"sent {total_mentions} mentions in {self.config['interval']}s", - culprit, - relevant_messages - ) - self.assertTupleEqual( - await mentions.apply(last_message, recent_messages, self.config), - desired_output - ) + def get_report(self, case: DisallowedCase) -> str: + return f"sent {case.n_violations} mentions in {self.config['interval']}s" diff --git a/tests/bot/rules/test_role_mentions.py b/tests/bot/rules/test_role_mentions.py index 6377ffbc8..b339cccf7 100644 --- a/tests/bot/rules/test_role_mentions.py +++ b/tests/bot/rules/test_role_mentions.py @@ -1,6 +1,7 @@ -import unittest +from typing import Iterable from bot.rules import role_mentions +from tests.bot.rules import DisallowedCase, RuleTest from tests.helpers import MockMessage, async_test @@ -9,10 +10,11 @@ def make_msg(author: str, n_mentions: int) -> MockMessage: return MockMessage(author=author, role_mentions=[None] * n_mentions) -class RoleMentionsRuleTests(unittest.TestCase): +class RoleMentionsRuleTests(RuleTest): """Tests for the `role_mentions` antispam rule.""" def setUp(self): + self.apply = role_mentions.apply self.config = {"max": 2, "interval": 10} @async_test @@ -23,44 +25,33 @@ class RoleMentionsRuleTests(unittest.TestCase): [make_msg("bob", 1), make_msg("alice", 1), make_msg("bob", 1)], ) - for recent_messages in cases: - last_message = recent_messages[0] - - with self.subTest(last_message=last_message, recent_messages=recent_messages, config=self.config): - self.assertIsNone(await role_mentions.apply(last_message, recent_messages, self.config)) + await self.run_allowed(cases) @async_test async def test_disallows_messages_beyond_limit(self): """Cases with more than the allowed amount of role mentions.""" cases = ( - ( + DisallowedCase( [make_msg("bob", 3)], - "bob", + ("bob",), 3, ), - ( + DisallowedCase( [make_msg("alice", 2), make_msg("bob", 2), make_msg("alice", 2)], - "alice", + ("alice",), 4, ), ) - for recent_messages, culprit, total_mentions in cases: - last_message = recent_messages[0] - relevant_messages = tuple(msg for msg in recent_messages if msg.author == culprit) - expected_output = ( - f"sent {total_mentions} role mentions in {self.config['interval']}s", - (culprit,), - relevant_messages, - ) + await self.run_disallowed(cases) + + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: + last_message = case.recent_messages[0] + return tuple( + msg + for msg in case.recent_messages + if msg.author == last_message.author + ) - with self.subTest( - last_message=last_message, - recent_messages=recent_messages, - config=self.config, - expected_output=expected_output, - ): - self.assertTupleEqual( - await role_mentions.apply(last_message, recent_messages, self.config), - expected_output, - ) + def get_report(self, case: DisallowedCase) -> str: + return f"sent {case.n_violations} role mentions in {self.config['interval']}s" -- cgit v1.2.3 From 9758e32beba0f88cdb1ee80c2e4bc5539013740e Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sun, 2 Feb 2020 18:36:36 +0100 Subject: Make RuleTest use ABCMeta This will prevent child classes to be instantiated unless they implement all abstract methods, leading to a more descriptive error message. --- tests/bot/rules/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bot/rules/__init__.py b/tests/bot/rules/__init__.py index d7cd7b66b..36c986fe1 100644 --- a/tests/bot/rules/__init__.py +++ b/tests/bot/rules/__init__.py @@ -1,5 +1,5 @@ import unittest -from abc import abstractmethod +from abc import ABCMeta, abstractmethod from typing import Callable, Dict, Iterable, List, NamedTuple, Tuple from tests.helpers import MockMessage @@ -12,7 +12,7 @@ class DisallowedCase(NamedTuple): n_violations: int -class RuleTest(unittest.TestCase): +class RuleTest(unittest.TestCase, metaclass=ABCMeta): """ Abstract class for antispam rule test cases. -- cgit v1.2.3 From 54e8c98f6ceb4572df5c73719649624613adeda6 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Feb 2020 21:49:46 +0100 Subject: Add unit test for duplicates antispam rule --- tests/bot/rules/test_duplicates.py | 66 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 tests/bot/rules/test_duplicates.py diff --git a/tests/bot/rules/test_duplicates.py b/tests/bot/rules/test_duplicates.py new file mode 100644 index 000000000..59e0fb6ef --- /dev/null +++ b/tests/bot/rules/test_duplicates.py @@ -0,0 +1,66 @@ +from typing import Iterable + +from bot.rules import duplicates +from tests.bot.rules import DisallowedCase, RuleTest +from tests.helpers import MockMessage, async_test + + +def make_msg(author: str, content: str) -> MockMessage: + """Give a MockMessage instance with `author` and `content` attrs.""" + return MockMessage(author=author, content=content) + + +class DuplicatesRuleTests(RuleTest): + """Tests the `duplicates` antispam rule.""" + + def setUp(self): + self.apply = duplicates.apply + self.config = {"max": 2, "interval": 10} + + @async_test + async def test_allows_messages_within_limit(self): + """Cases which do not violate the rule.""" + cases = ( + [make_msg("alice", "A"), make_msg("alice", "A")], + [make_msg("alice", "A"), make_msg("alice", "B"), make_msg("alice", "C")], # Non-duplicate + [make_msg("alice", "A"), make_msg("bob", "A"), make_msg("alice", "A")], # Different author + ) + + await self.run_allowed(cases) + + @async_test + async def test_disallows_messages_beyond_limit(self): + """Cases with too many duplicate messages from the same author.""" + cases = ( + DisallowedCase( + [make_msg("alice", "A"), make_msg("alice", "A"), make_msg("alice", "A")], + ("alice",), + 3, + ), + DisallowedCase( + [make_msg("bob", "A"), make_msg("alice", "A"), make_msg("bob", "A"), make_msg("bob", "A")], + ("bob",), + 3, # 4 duplicate messages, but only 3 from bob + ), + DisallowedCase( + [make_msg("bob", "A"), make_msg("bob", "B"), make_msg("bob", "A"), make_msg("bob", "A")], + ("bob",), + 3, # 4 message from bob, but only 3 duplicates + ), + ) + + await self.run_disallowed(cases) + + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: + last_message = case.recent_messages[0] + return tuple( + msg + for msg in case.recent_messages + if ( + msg.author == last_message.author + and msg.content == last_message.content + ) + ) + + def get_report(self, case: DisallowedCase) -> str: + return f"sent {case.n_violations} duplicated messages in {self.config['interval']}s" -- cgit v1.2.3 From cae7f25a735516d92af34353715ed23604b1d71d Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Feb 2020 21:51:25 +0100 Subject: Add unit test for newlines antispam rule --- tests/bot/rules/test_newlines.py | 105 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 tests/bot/rules/test_newlines.py diff --git a/tests/bot/rules/test_newlines.py b/tests/bot/rules/test_newlines.py new file mode 100644 index 000000000..d61c4609d --- /dev/null +++ b/tests/bot/rules/test_newlines.py @@ -0,0 +1,105 @@ +from typing import Iterable, List + +from bot.rules import newlines +from tests.bot.rules import DisallowedCase, RuleTest +from tests.helpers import MockMessage, async_test + + +def make_msg(author: str, newline_groups: List[int]) -> MockMessage: + """Init a MockMessage instance with `author` and content configured by `newline_groups". + + Configure content by passing a list of ints, where each int `n` will generate + a separate group of `n` newlines. + + Example: + newline_groups=[3, 1, 2] -> content="\n\n\n \n \n\n" + """ + content = " ".join("\n" * n for n in newline_groups) + return MockMessage(author=author, content=content) + + +class TotalNewlinesRuleTests(RuleTest): + """Tests the `newlines` antispam rule against allowed cases and total newline count violations.""" + + def setUp(self): + self.apply = newlines.apply + self.config = { + "max": 5, # Max sum of newlines in relevant messages + "max_consecutive": 3, # Max newlines in one group, in one message + "interval": 10, + } + + @async_test + async def test_allows_messages_within_limit(self): + """Cases which do not violate the rule.""" + cases = ( + [make_msg("alice", [])], # Single message with no newlines + [make_msg("alice", [1, 2]), make_msg("alice", [1, 1])], # 5 newlines in 2 messages + [make_msg("alice", [2, 2, 1]), make_msg("bob", [2, 3])], # 5 newlines from each author + [make_msg("bob", [1]), make_msg("alice", [5])], # Alice breaks the rule, but only bob is relevant + ) + + await self.run_allowed(cases) + + @async_test + async def test_disallows_messages_total(self): + """Cases which violate the rule by having too many newlines in total.""" + cases = ( + DisallowedCase( # Alice sends a total of 6 newlines (disallowed) + [make_msg("alice", [2, 2]), make_msg("alice", [2])], + ("alice",), + 6, + ), + DisallowedCase( # Here we test that only alice's newlines count in the sum + [make_msg("alice", [2, 2]), make_msg("bob", [3]), make_msg("alice", [3])], + ("alice",), + 7, + ), + ) + + await self.run_disallowed(cases) + + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: + last_author = case.recent_messages[0].author + return tuple(msg for msg in case.recent_messages if msg.author == last_author) + + def get_report(self, case: DisallowedCase) -> str: + return f"sent {case.n_violations} newlines in {self.config['interval']}s" + + +class GroupNewlinesRuleTests(RuleTest): + """ + Tests the `newlines` antispam rule against max consecutive newline violations. + + As these violations yield a different error report, they require a different + `get_report` implementation. + """ + + def setUp(self): + self.apply = newlines.apply + self.config = {"max": 5, "max_consecutive": 3, "interval": 10} + + @async_test + async def test_disallows_messages_consecutive(self): + """Cases which violate the rule due to having too many consecutive newlines.""" + cases = ( + DisallowedCase( # Bob sends a group of newlines too large + [make_msg("bob", [4])], + ("bob",), + 4, + ), + DisallowedCase( # Alice sends 5 in total (allowed), but 4 in one group (disallowed) + [make_msg("alice", [1]), make_msg("alice", [4])], + ("alice",), + 4, + ), + ) + + await self.run_disallowed(cases) + + def relevant_messages(self, case: DisallowedCase) -> Iterable[MockMessage]: + last_author = case.recent_messages[0].author + return tuple(msg for msg in case.recent_messages if msg.author == last_author) + + def get_report(self, case: DisallowedCase) -> str: + return f"sent {case.n_violations} consecutive newlines in {self.config['interval']}s" -- cgit v1.2.3 From debe73add8bf5c5f9b33e32201f1ce212758c8e4 Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 21:44:56 +0100 Subject: No longer check if every role is @everyone; just skip the first element in the list --- bot/cogs/information.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 125d7ce24..bc2deae8f 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -32,8 +32,7 @@ class Information(Cog): async def roles_info(self, ctx: Context) -> None: """Returns a list of all roles and their corresponding IDs.""" # Sort the roles alphabetically and remove the @everyone role - roles = sorted(ctx.guild.roles, key=lambda role: role.name) - roles = [role for role in roles if role.name != "@everyone"] + roles = sorted(ctx.guild.roles[1:], key=lambda role: role.name) # Build a string role_string = "" @@ -202,7 +201,7 @@ class Information(Cog): name = f"{user.nick} ({name})" joined = time_since(user.joined_at, precision="days") - roles = ", ".join(role.mention for role in user.roles if role.name != "@everyone") + roles = ", ".join(role.mention for role in user.roles[1:]) description = [ textwrap.dedent(f""" -- cgit v1.2.3 From 766331588ebad2ac74ccde572d241803db77f70c Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 21:45:18 +0100 Subject: Roles cannot return None because everyone has the Developer role by default, and non-verified users cannot use this command. --- bot/cogs/information.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index bc2deae8f..21e3cfc39 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -212,7 +212,7 @@ class Information(Cog): {custom_status} **Member Information** Joined: {joined} - Roles: {roles or None} + Roles: {roles} """).strip() ] -- cgit v1.2.3 From 515b490fca1f151e654ff72a9fcf8f3113f239c3 Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 21:46:45 +0100 Subject: Remove some a lot of unneccesary newlines that arguably make it harder to read --- bot/cogs/information.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 21e3cfc39..68614d2c4 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -45,7 +45,6 @@ class Information(Cog): colour=Colour.blurple(), description=role_string ) - embed.set_footer(text=f"Total roles: {len(roles)}") await ctx.send(embed=embed) @@ -75,23 +74,17 @@ class Information(Cog): parsed_roles.append(role) for role in parsed_roles: + h, s, v = colorsys.rgb_to_hsv(*role.colour.to_rgb()) + embed = Embed( title=f"{role.name} info", colour=role.colour, ) - embed.add_field(name="ID", value=role.id, inline=True) - embed.add_field(name="Colour (RGB)", value=f"#{role.colour.value:0>6x}", inline=True) - - h, s, v = colorsys.rgb_to_hsv(*role.colour.to_rgb()) - embed.add_field(name="Colour (HSV)", value=f"{h:.2f} {s:.2f} {v}", inline=True) - embed.add_field(name="Member count", value=len(role.members), inline=True) - embed.add_field(name="Position", value=role.position) - embed.add_field(name="Permission code", value=role.permissions.value, inline=True) await ctx.send(embed=embed) -- cgit v1.2.3 From 5f799b68316e03cd0a565af484f7dee3f79ed35e Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 21:48:26 +0100 Subject: Refactor how channels and statuses are counted; using Counter() - way cleaner. --- bot/cogs/information.py | 52 ++++++++++++++++--------------------------------- 1 file changed, 17 insertions(+), 35 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 68614d2c4..412447835 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -3,7 +3,7 @@ import logging import pprint import textwrap import typing -from collections import defaultdict +from collections import Counter, defaultdict from typing import Any, Mapping, Optional import discord @@ -96,36 +96,18 @@ class Information(Cog): features = ", ".join(ctx.guild.features) region = ctx.guild.region - # How many of each type of channel? roles = len(ctx.guild.roles) - channels = ctx.guild.channels - text_channels = 0 - category_channels = 0 - voice_channels = 0 - for channel in channels: - if type(channel) == TextChannel: - text_channels += 1 - elif type(channel) == CategoryChannel: - category_channels += 1 - elif type(channel) == VoiceChannel: - voice_channels += 1 + member_count = ctx.guild.member_count + + # How many of each type of channel? + channels = Counter({TextChannel: 0, VoiceChannel: 0, CategoryChannel: 0}) + for channel in ctx.guild.channels: + channels[channel.__class__] += 1 # How many of each user status? - member_count = ctx.guild.member_count - members = ctx.guild.members - online = 0 - dnd = 0 - idle = 0 - offline = 0 - for member in members: - if str(member.status) == "online": - online += 1 - elif str(member.status) == "offline": - offline += 1 - elif str(member.status) == "idle": - idle += 1 - elif str(member.status) == "dnd": - dnd += 1 + statuses = Counter({status.value: 0 for status in Status}) + for member in ctx.guild.members: + statuses[member.status.value] += 1 embed = Embed( colour=Colour.blurple(), @@ -138,15 +120,15 @@ class Information(Cog): **Counts** Members: {member_count:,} Roles: {roles} - Text: {text_channels} - Voice: {voice_channels} - Channel categories: {category_channels} + Text Channels: {channels[TextChannel]} + Voice Channels: {channels[VoiceChannel]} + Channel categories: {channels[CategoryChannel]} **Members** - {constants.Emojis.status_online} {online} - {constants.Emojis.status_idle} {idle} - {constants.Emojis.status_dnd} {dnd} - {constants.Emojis.status_offline} {offline} + {constants.Emojis.status_online} {statuses['online']} + {constants.Emojis.status_idle} {statuses['idle']} + {constants.Emojis.status_dnd} {statuses['dnd']} + {constants.Emojis.status_offline} {statuses['offline']} """) ) -- cgit v1.2.3 From 79fb50772065e23827396911682da51e24440787 Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 21:48:39 +0100 Subject: Update tests to reflect status changes --- tests/bot/cogs/test_information.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 4496a2ae0..519d2622b 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -125,10 +125,10 @@ class InformationCogTests(unittest.TestCase): ) ], members=[ - *(helpers.MockMember(status='online') for _ in range(2)), - *(helpers.MockMember(status='idle') for _ in range(1)), - *(helpers.MockMember(status='dnd') for _ in range(4)), - *(helpers.MockMember(status='offline') for _ in range(3)), + *(helpers.MockMember(status=discord.Status.online) for _ in range(2)), + *(helpers.MockMember(status=discord.Status.idle) for _ in range(1)), + *(helpers.MockMember(status=discord.Status.dnd) for _ in range(4)), + *(helpers.MockMember(status=discord.Status.offline) for _ in range(3)), ], member_count=1_234, icon_url='a-lemon.jpg', -- cgit v1.2.3 From 90dd064f2a8cfe66e5cefbe7b679dac38f6f7845 Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 21:50:10 +0100 Subject: Instead of sending a message everytime a role can't be converted, append it to a list, and then send them it at once (less spammy) --- bot/cogs/information.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 412447835..bc67ab5c2 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -58,6 +58,7 @@ class Information(Cog): To specify multiple roles just add to the arguments, delimit roles with spaces in them using quotation marks. """ parsed_roles = [] + failed_roles = [] for role_name in roles: if isinstance(role_name, Role): @@ -68,11 +69,17 @@ class Information(Cog): role = utils.find(lambda r: r.name.lower() == role_name.lower(), ctx.guild.roles) if not role: - await ctx.send(f":x: Could not convert `{role_name}` to a role") + failed_roles.append(role_name) continue parsed_roles.append(role) + if failed_roles: + await ctx.send( + ":x: I could not convert the following role names to a role: \n- " + "\n- ".join(failed_roles) + ) + for role in parsed_roles: h, s, v = colorsys.rgb_to_hsv(*role.colour.to_rgb()) -- cgit v1.2.3 From 8f1f8055383e5cbf017f4f2cec7074518dab95fd Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 21:59:46 +0100 Subject: Fix up imports a bit; there's no need to import all of discord and typing for just 1 or 2 uses (e.g. Union, and Message). --- bot/cogs/information.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index bc67ab5c2..3b8a88309 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -2,14 +2,11 @@ import colorsys import logging import pprint import textwrap -import typing from collections import Counter, defaultdict -from typing import Any, Mapping, Optional +from typing import Any, Mapping, Optional, Union -import discord -from discord import CategoryChannel, Colour, Embed, Member, Role, TextChannel, VoiceChannel, utils -from discord.ext import commands -from discord.ext.commands import BucketType, Cog, Context, command, group +from discord import CategoryChannel, Colour, Embed, Member, Message, Role, Status, TextChannel, VoiceChannel, utils +from discord.ext.commands import BucketType, Cog, Context, Paginator, command, group from discord.utils import escape_markdown from bot import constants @@ -51,7 +48,7 @@ class Information(Cog): @with_role(*constants.MODERATION_ROLES) @command(name="role") - async def role_info(self, ctx: Context, *roles: typing.Union[Role, str]) -> None: + async def role_info(self, ctx: Context, *roles: Union[Role, str]) -> None: """ Return information on a role or list of roles. @@ -337,13 +334,13 @@ class Information(Cog): @cooldown_with_role_bypass(2, 60 * 3, BucketType.member, bypass_roles=constants.STAFF_ROLES) @group(invoke_without_command=True) @in_channel(constants.Channels.bot, bypass_roles=constants.STAFF_ROLES) - async def raw(self, ctx: Context, *, message: discord.Message, json: bool = False) -> None: + async def raw(self, ctx: Context, *, message: Message, json: bool = False) -> None: """Shows information about the raw API response.""" # I *guess* it could be deleted right as the command is invoked but I felt like it wasn't worth handling # doing this extra request is also much easier than trying to convert everything back into a dictionary again raw_data = await ctx.bot.http.get_message(message.channel.id, message.id) - paginator = commands.Paginator() + paginator = Paginator() def add_content(title: str, content: str) -> None: paginator.add_line(f'== {title} ==\n') @@ -371,7 +368,7 @@ class Information(Cog): await ctx.send(page) @raw.command() - async def json(self, ctx: Context, message: discord.Message) -> None: + async def json(self, ctx: Context, message: Message) -> None: """Shows information about the raw API response in a copy-pasteable Python format.""" await ctx.invoke(self.raw, message=message, json=True) -- cgit v1.2.3 From 4653dadbbe929055355892b322e4b0cfd3e09ab6 Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 22:00:13 +0100 Subject: Change if statement to elif; if the first if statement returns true, the second cannot be true making it unneccesary to check --- bot/cogs/information.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 3b8a88309..e1a68ee63 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -147,7 +147,7 @@ class Information(Cog): user = ctx.author # Do a role check if this is being executed on someone other than the caller - if user != ctx.author and not with_role_check(ctx, *constants.MODERATION_ROLES): + elif user != ctx.author and not with_role_check(ctx, *constants.MODERATION_ROLES): await ctx.send("You may not use this command on users other than yourself.") return -- cgit v1.2.3 From 24136095302d192b25d83430a1b9607f05f6059c Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 6 Feb 2020 22:37:03 +0100 Subject: Fix some of the testing for information.py; I think this should be it. (hopefully). --- tests/bot/cogs/test_information.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 519d2622b..296c3c556 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -153,8 +153,8 @@ class InformationCogTests(unittest.TestCase): **Counts** Members: {self.ctx.guild.member_count:,} Roles: {len(self.ctx.guild.roles)} - Text: 1 - Voice: 1 + Text Channels: 1 + Voice Channels: 1 Channel categories: 1 **Members** -- cgit v1.2.3 From b0fe5841f58d72a12b3f3ddfd5de7b648770fd58 Mon Sep 17 00:00:00 2001 From: Deniz Date: Sat, 8 Feb 2020 18:48:32 +0100 Subject: Use the enum itself instead of its string value --- bot/cogs/information.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index e1a68ee63..efe660851 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -109,9 +109,9 @@ class Information(Cog): channels[channel.__class__] += 1 # How many of each user status? - statuses = Counter({status.value: 0 for status in Status}) + statuses = Counter({status: 0 for status in Status}) for member in ctx.guild.members: - statuses[member.status.value] += 1 + statuses[member.status] += 1 embed = Embed( colour=Colour.blurple(), @@ -129,10 +129,10 @@ class Information(Cog): Channel categories: {channels[CategoryChannel]} **Members** - {constants.Emojis.status_online} {statuses['online']} - {constants.Emojis.status_idle} {statuses['idle']} - {constants.Emojis.status_dnd} {statuses['dnd']} - {constants.Emojis.status_offline} {statuses['offline']} + {constants.Emojis.status_online} {statuses[Status.online]} + {constants.Emojis.status_idle} {statuses[Status.idle]} + {constants.Emojis.status_dnd} {statuses[Status.dnd]} + {constants.Emojis.status_offline} {statuses[Status.offline]} """) ) -- cgit v1.2.3 From bc932aa09848bc10683d66b7e7d9f6054e9958c6 Mon Sep 17 00:00:00 2001 From: Deniz Date: Wed, 12 Feb 2020 20:33:31 +0100 Subject: Use collections.Counter properly. Use the ChannelType enum instead of the __class__ attribute, and re-add the None check for !user roles. --- bot/cogs/information.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index efe660851..2461cc749 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -5,7 +5,7 @@ import textwrap from collections import Counter, defaultdict from typing import Any, Mapping, Optional, Union -from discord import CategoryChannel, Colour, Embed, Member, Message, Role, Status, TextChannel, VoiceChannel, utils +from discord import Colour, Embed, Member, Message, Role, Status, utils from discord.ext.commands import BucketType, Cog, Context, Paginator, command, group from discord.utils import escape_markdown @@ -104,14 +104,11 @@ class Information(Cog): member_count = ctx.guild.member_count # How many of each type of channel? - channels = Counter({TextChannel: 0, VoiceChannel: 0, CategoryChannel: 0}) - for channel in ctx.guild.channels: - channels[channel.__class__] += 1 + channels = Counter(c.type for c in ctx.guild.channels) + channel_counts = "".join(sorted(f"{str(ch).title()} channels: {channels[ch]} \n" for ch in channels)).strip() # How many of each user status? - statuses = Counter({status: 0 for status in Status}) - for member in ctx.guild.members: - statuses[member.status] += 1 + statuses = Counter(member.status for member in ctx.guild.members) embed = Embed( colour=Colour.blurple(), @@ -124,9 +121,7 @@ class Information(Cog): **Counts** Members: {member_count:,} Roles: {roles} - Text Channels: {channels[TextChannel]} - Voice Channels: {channels[VoiceChannel]} - Channel categories: {channels[CategoryChannel]} + {channel_counts} **Members** {constants.Emojis.status_online} {statuses[Status.online]} @@ -135,7 +130,6 @@ class Information(Cog): {constants.Emojis.status_offline} {statuses[Status.offline]} """) ) - embed.set_thumbnail(url=ctx.guild.icon_url) await ctx.send(embed=embed) @@ -191,7 +185,7 @@ class Information(Cog): {custom_status} **Member Information** Joined: {joined} - Roles: {roles} + Roles: {roles or None} """).strip() ] -- cgit v1.2.3 From 6703e3b7db972017764f91232a82c163be2cd588 Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 13 Feb 2020 17:35:34 +0100 Subject: Update the tests accordingly to reflect the new changes --- tests/bot/cogs/test_information.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 296c3c556..deae7ebad 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -153,9 +153,9 @@ class InformationCogTests(unittest.TestCase): **Counts** Members: {self.ctx.guild.member_count:,} Roles: {len(self.ctx.guild.roles)} - Text Channels: 1 - Voice Channels: 1 - Channel categories: 1 + Category channels: 1 + Text channels: 1 + Voice channels: 1 **Members** {constants.Emojis.status_online} 2 -- cgit v1.2.3 From b26122d72c1a41a4919c95642c6bc16e82d535a3 Mon Sep 17 00:00:00 2001 From: Deniz Date: Thu, 13 Feb 2020 17:35:56 +0100 Subject: Add thousand separators to Members count, closes #744 --- bot/cogs/information.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 2461cc749..c8b5eb5ad 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -124,10 +124,10 @@ class Information(Cog): {channel_counts} **Members** - {constants.Emojis.status_online} {statuses[Status.online]} - {constants.Emojis.status_idle} {statuses[Status.idle]} - {constants.Emojis.status_dnd} {statuses[Status.dnd]} - {constants.Emojis.status_offline} {statuses[Status.offline]} + {constants.Emojis.status_online} {statuses[Status.online]:,} + {constants.Emojis.status_idle} {statuses[Status.idle]:,} + {constants.Emojis.status_dnd} {statuses[Status.dnd]:,} + {constants.Emojis.status_offline} {statuses[Status.offline]:,} """) ) embed.set_thumbnail(url=ctx.guild.icon_url) -- cgit v1.2.3 From 5c70f10a08c7d8621b862c9f975bc56c80607b49 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Fri, 14 Feb 2020 10:41:18 +0100 Subject: Stop scheduling expiration of permanent infractions on edit https://github.com/python-discord/bot/issues/751 The infraction edit command defined in `bot.cogs.moderation.management` contained a bug causing it to attempt to schedule an expiration task when turning a temporary infraction into a permanent infraction. Since the "expires_at" field of a permanent infractions is `None`, this caused an exception to occur in the scheduler: Traceback (most recent call last): File "/bot/bot/cogs/moderation/scheduler.py", line 415, in _scheduled_task expiry = dateutil.parser.isoparse(infraction["expires_at"]).replace(tzinfo=None) File "/usr/local/lib/python3.7/site-packages/dateutil/parser/isoparser.py", line 37, in func return f(self, str_in, *args, **kwargs) File "/usr/local/lib/python3.7/site-packages/dateutil/parser/isoparser.py", line 134, in isoparse components, pos = self._parse_isodate(dt_str) File "/usr/local/lib/python3.7/site-packages/dateutil/parser/isoparser.py", line 208, in _parse_isodate return self._parse_isodate_common(dt_str) File "/usr/local/lib/python3.7/site-packages/dateutil/parser/isoparser.py", line 213, in _parse_isodate_common len_str = len(dt_str) TypeError: object of type 'NoneType' has no len() I have solved this by adding a check that makes sure we only schedule an expiration task when the `"expires_at"` field has a truthy value (which all valid datetime strings are) using `if request_data['expires_at']`. IMPORTANT NOTE: While it's tempting to just skip the entire scheduling block for permanent infractions, it's essential to unschedule existing expiration tasks for this infraction as we're changing a temporary infraction to a permanent infraction. This commit closes #751 --- bot/cogs/moderation/management.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation/management.py b/bot/cogs/moderation/management.py index 0636422d3..f2964cd78 100644 --- a/bot/cogs/moderation/management.py +++ b/bot/cogs/moderation/management.py @@ -130,8 +130,11 @@ class ModManagement(commands.Cog): # Re-schedule infraction if the expiration has been updated if 'expires_at' in request_data: self.infractions_cog.cancel_task(new_infraction['id']) - loop = asyncio.get_event_loop() - self.infractions_cog.schedule_task(loop, new_infraction['id'], new_infraction) + + # If the infraction was not marked as permanent, schedule a new expiration task + if request_data['expires_at']: + loop = asyncio.get_event_loop() + self.infractions_cog.schedule_task(loop, new_infraction['id'], new_infraction) log_text += f""" Previous expiry: {old_infraction['expires_at'] or "Permanent"} -- cgit v1.2.3 From b4d870add68fafa00420a1d75136b6264be3ff0a Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 15 Feb 2020 18:22:30 -0800 Subject: Utils: refactor format_infraction_with_duration and support absolute val The absolute value is useful for preventing negative values in the case that date_to is in the past relative to date_from. * Add an absolute parameter to take the absolute value of the duration * Rename expiry to date_to * Rewrite the docstring --- bot/utils/time.py | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/bot/utils/time.py b/bot/utils/time.py index 7416f36e0..77060143c 100644 --- a/bot/utils/time.py +++ b/bot/utils/time.py @@ -114,30 +114,40 @@ def format_infraction(timestamp: str) -> str: def format_infraction_with_duration( - expiry: Optional[str], + date_to: Optional[str], date_from: Optional[datetime.datetime] = None, - max_units: int = 2 + max_units: int = 2, + absolute: bool = True ) -> Optional[str]: """ - Format an infraction timestamp to a more readable ISO 8601 format WITH the duration. + Return `date_to` formatted as a readable ISO-8601 with the humanized duration since `date_from`. - Returns a human-readable version of the duration between datetime.utcnow() and an expiry. - Unlike `humanize_delta`, this function will force the `precision` to be `seconds` by not passing it. - `max_units` specifies the maximum number of units of time to include (e.g. 1 may include days but not hours). - By default, max_units is 2. + `date_from` must be an ISO-8601 formatted timestamp. The duration is calculated as from + `date_from` until `date_to` with a precision of seconds. If `date_from` is unspecified, the + current time is used. + + `max_units` specifies the maximum number of units of time to include in the duration. For + example, a value of 1 may include days but not hours. + + If `absolute` is True, the absolute value of the duration delta is used. This prevents negative + values in the case that `date_to` is in the past relative to `date_from`. """ - if not expiry: + if not date_to: return None + date_to_formatted = format_infraction(date_to) + date_from = date_from or datetime.datetime.utcnow() - date_to = dateutil.parser.isoparse(expiry).replace(tzinfo=None, microsecond=0) + date_to = dateutil.parser.isoparse(date_to).replace(tzinfo=None, microsecond=0) - expiry_formatted = format_infraction(expiry) + delta = relativedelta(date_to, date_from) + if absolute: + delta = abs(delta) - duration = humanize_delta(relativedelta(date_to, date_from), max_units=max_units) - duration_formatted = f" ({duration})" if duration else '' + duration = humanize_delta(delta, max_units=max_units) + duration_formatted = f" ({duration})" if duration else "" - return f"{expiry_formatted}{duration_formatted}" + return f"{date_to_formatted}{duration_formatted}" def until_expiration( -- cgit v1.2.3 From 2a655699b5e387a3854472210c5bf6ee95d076cc Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 15 Feb 2020 18:40:54 -0800 Subject: Moderation: add creation date & duration to expired infraction log Closes #685 --- bot/cogs/moderation/scheduler.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index e14c302cb..6a0ed71a0 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -311,6 +311,11 @@ class InfractionScheduler(Scheduler): user_id = infraction["user"] type_ = infraction["type"] id_ = infraction["id"] + inserted_at = infraction["inserted_at"] + expiry = infraction["expires_at"] + + expiry = dateutil.parser.isoparse(expiry).replace(tzinfo=None) if expiry else None + created = time.format_infraction_with_duration(inserted_at, expiry) log.info(f"Marking infraction #{id_} as inactive (expired).") @@ -318,7 +323,8 @@ class InfractionScheduler(Scheduler): log_text = { "Member": str(user_id), "Actor": str(self.bot.user), - "Reason": infraction["reason"] + "Reason": infraction["reason"], + "Created": created, } try: -- cgit v1.2.3 From 9f3bcc57d06db73b0aeeeb0959431531ceafd46a Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 15 Feb 2020 19:00:20 -0800 Subject: Moderation: show correct actor in expired infraction log The bot user was always being used instead of using the actor field of the infraction. --- bot/cogs/moderation/scheduler.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 6a0ed71a0..e3edc960b 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -309,20 +309,21 @@ class InfractionScheduler(Scheduler): guild = self.bot.get_guild(constants.Guild.id) mod_role = guild.get_role(constants.Roles.moderator) user_id = infraction["user"] + actor = infraction["actor"] type_ = infraction["type"] id_ = infraction["id"] inserted_at = infraction["inserted_at"] expiry = infraction["expires_at"] + log.info(f"Marking infraction #{id_} as inactive (expired).") + expiry = dateutil.parser.isoparse(expiry).replace(tzinfo=None) if expiry else None created = time.format_infraction_with_duration(inserted_at, expiry) - log.info(f"Marking infraction #{id_} as inactive (expired).") - log_content = None log_text = { "Member": str(user_id), - "Actor": str(self.bot.user), + "Actor": str(self.bot.get_user(actor) or actor), "Reason": infraction["reason"], "Created": created, } -- cgit v1.2.3 From 695ea44f314da052f0540127fb360077525483e3 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 15 Feb 2020 19:03:13 -0800 Subject: Moderation: show member as a mention in expired infraction log --- bot/cogs/moderation/scheduler.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index e3edc960b..e45f196a4 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -322,7 +322,7 @@ class InfractionScheduler(Scheduler): log_content = None log_text = { - "Member": str(user_id), + "Member": f"<@{user_id}>", "Actor": str(self.bot.get_user(actor) or actor), "Reason": infraction["reason"], "Created": created, -- cgit v1.2.3 From c0bc8d03804739d8ff025f4bf71846b09569b75c Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 10:59:37 -0800 Subject: Fix missing Django logs when using Docker Compose Fixed by allocating a pseudo-tty to the web and bot services in Docker Compose. --- docker-compose.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docker-compose.yml b/docker-compose.yml index 7281c7953..11deceae8 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -23,6 +23,7 @@ services: - staff.web ports: - "127.0.0.1:8000:8000" + tty: true depends_on: - postgres environment: @@ -37,6 +38,7 @@ services: volumes: - ./logs:/bot/logs - .:/bot:ro + tty: true depends_on: - web environment: -- cgit v1.2.3 From d955ecc449eee54cfd28504403e569f58707588e Mon Sep 17 00:00:00 2001 From: scragly <29337040+scragly@users.noreply.github.com> Date: Mon, 17 Feb 2020 16:51:00 +1000 Subject: Change snekbox api url to internal docker domain. NGINX has been dockerised, and proxy passes now reference internal container domains rather than referencing host or external domains. This will have a few extra benefits: - Less external factors involved for resolving the service address - Can work with the same address on development envs - Snekbox can be closed down entirely so it's inaccessible from external networks. --- config-default.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config-default.yml b/config-default.yml index fda14b511..3345e6f2a 100644 --- a/config-default.yml +++ b/config-default.yml @@ -302,7 +302,7 @@ urls: paste_service: !JOIN [*SCHEMA, *PASTE, "/{key}"] # Snekbox - snekbox_eval_api: "https://snekbox.pythondiscord.com/eval" + snekbox_eval_api: "http://snekbox:8060/eval" # Discord API URLs discord_api: &DISCORD_API "https://discordapp.com/api/v7/" -- cgit v1.2.3 From 62d198beb6aff28d25098d85d0236e5895444428 Mon Sep 17 00:00:00 2001 From: F4zi <44242259+F4zi780@users.noreply.github.com> Date: Mon, 17 Feb 2020 12:52:06 +0200 Subject: Pagination migrations - Emoji Data Structure Modified Changed the pagination emoji collection from list to tuple This change was suggested since this collection is constant --- bot/pagination.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/pagination.py b/bot/pagination.py index e82763912..4c2976e17 100644 --- a/bot/pagination.py +++ b/bot/pagination.py @@ -14,7 +14,7 @@ RIGHT_EMOJI = "\u27A1" # [:arrow_right:] LAST_EMOJI = "\u23ED" # [:track_next:] DELETE_EMOJI = constants.Emojis.trashcan # [:trashcan:] -PAGINATION_EMOJI = [FIRST_EMOJI, LEFT_EMOJI, RIGHT_EMOJI, LAST_EMOJI, DELETE_EMOJI] +PAGINATION_EMOJI = (FIRST_EMOJI, LEFT_EMOJI, RIGHT_EMOJI, LAST_EMOJI, DELETE_EMOJI) log = logging.getLogger(__name__) -- cgit v1.2.3 From 66901fc6d12046751bbe2bdf37ea0192cf478607 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Wed, 19 Feb 2020 09:13:26 -0800 Subject: Moderation: show avatar in infraction deactivation log --- bot/cogs/moderation/scheduler.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index e45f196a4..c0de0e4da 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -391,14 +391,19 @@ class InfractionScheduler(Scheduler): if send_log: log_title = f"expiration failed" if "Failure" in log_text else "expired" + user = self.bot.get_user(user_id) + avatar = user.avatar_url_as(static_format="png") if user else None + log.trace(f"Sending deactivation mod log for infraction #{id_}.") await self.mod_log.send_log_message( icon_url=utils.INFRACTION_ICONS[type_][1], colour=Colours.soft_green, title=f"Infraction {log_title}: {type_}", + thumbnail=avatar, text="\n".join(f"{k}: {v}" for k, v in log_text.items()), footer=f"ID: {id_}", content=log_content, + ) return log_text -- cgit v1.2.3 From ebead84ae0eb62ab64b329dfd7fff61ff5de628d Mon Sep 17 00:00:00 2001 From: scragly <29337040+scragly@users.noreply.github.com> Date: Thu, 20 Feb 2020 22:40:25 +1000 Subject: Change to explicit logs, remove logmatic. --- Pipfile | 1 - Pipfile.lock | 152 ++++++++++++++++++++++---------------------------------- bot/__init__.py | 83 +++++++------------------------ 3 files changed, 76 insertions(+), 160 deletions(-) diff --git a/Pipfile b/Pipfile index 7fd3efae8..7e63bde66 100644 --- a/Pipfile +++ b/Pipfile @@ -6,7 +6,6 @@ name = "pypi" [packages] discord-py = "~=1.3.1" aiodns = "~=2.0" -logmatic-python = "~=0.1" aiohttp = "~=3.5" sphinx = "~=2.2" markdownify = "~=0.4" diff --git a/Pipfile.lock b/Pipfile.lock index bf8ff47e9..cca6e1791 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "0a0354a8cbd25b19c61b68f928493a445e737dc6447c97f4c4b52fbf72d887ac" + "sha256": "fe6d96850817a250e3c3601044bf8927c0553b73f4cd5cb228a89f04458a93af" }, "pipfile-spec": 6, "requires": { @@ -18,11 +18,11 @@ "default": { "aio-pika": { "hashes": [ - "sha256:a5837277e53755078db3a9e8c45bbca605c8ba9ecba7a02d74a7a1779f444723", - "sha256:fa32e33b4b7d0804dcf439ae6ff24d2f0a83d1ba280ee9f555e647d71d394ff5" + "sha256:4199122a450dffd8303b7857a9d82657bf1487fe329e489520833b40fbe92406", + "sha256:fe85c7456e5c060bce4eb9cffab5b2c4d3c563cb72177977b3556c54c8e3aeb6" ], "index": "pypi", - "version": "==6.4.1" + "version": "==6.5.2" }, "aiodns": { "hashes": [ @@ -52,10 +52,10 @@ }, "aiormq": { "hashes": [ - "sha256:8c215a970133ab5ee7c478decac55b209af7731050f52d11439fe910fa0f9e9d", - "sha256:9210f3389200aee7d8067f6435f4a9eff2d3a30b88beb5eaae406ccc11c0fc01" + "sha256:286e0b0772075580466e45f98f051b9728a9316b9c36f0c14c7bc1409be375b0", + "sha256:7ed7d6df6b57af7f8bce7d1ebcbdfc32b676192e46703e81e9e217316e56b5bd" ], - "version": "==3.2.0" + "version": "==3.2.1" }, "alabaster": { "hashes": [ @@ -164,18 +164,18 @@ }, "fuzzywuzzy": { "hashes": [ - "sha256:5ac7c0b3f4658d2743aa17da53a55598144edbc5bee3c6863840636e6926f254", - "sha256:6f49de47db00e1c71d40ad16da42284ac357936fa9b66bea1df63fed07122d62" + "sha256:45016e92264780e58972dca1b3d939ac864b78437422beecebb3095f8efd00e8", + "sha256:928244b28db720d1e0ee7587acf660ea49d7e4c632569cad4f1cd7e68a5f0993" ], "index": "pypi", - "version": "==0.17.0" + "version": "==0.18.0" }, "idna": { "hashes": [ - "sha256:c357b3f628cf53ae2c4c05627ecc484553142ca23264e593d327bcde5e9c3407", - "sha256:ea8b7f6188e6fa117537c3df7da9fc686d485087abf6ac197f9c46432f7e4a3c" + "sha256:7588d1c14ae4c77d74036e8c22ff447b26d0fde8f007354fd48a7814db15b7cb", + "sha256:a068a21ceac8a4d63dbfd964670474107f541babbd2250d61922f029858365fa" ], - "version": "==2.8" + "version": "==2.9" }, "imagesize": { "hashes": [ @@ -191,13 +191,6 @@ ], "version": "==2.11.1" }, - "logmatic-python": { - "hashes": [ - "sha256:0c15ac9f5faa6a60059b28910db642c3dc7722948c3cc940923f8c9039604342" - ], - "index": "pypi", - "version": "==0.1.7" - }, "lxml": { "hashes": [ "sha256:06d4e0bbb1d62e38ae6118406d7cdb4693a3fa34ee3762238bcb96c9e36a93cd", @@ -362,7 +355,8 @@ }, "pycparser": { "hashes": [ - "sha256:a988718abfad80b6b157acce7bf130a30876d27603738ac39f140993246b25b3" + "sha256:a988718abfad80b6b157acce7bf130a30876d27603738ac39f140993246b25b3", + "sha256:cebad02308a8c080ff0a3d3c9eec420a280dc4341cf8681381b444a176406a8e" ], "version": "==2.19" }, @@ -388,12 +382,6 @@ "index": "pypi", "version": "==2.8.1" }, - "python-json-logger": { - "hashes": [ - "sha256:b7a31162f2a01965a5efb94453ce69230ed208468b0bbc7fdfc56e6d8df2e281" - ], - "version": "==0.1.11" - }, "pytz": { "hashes": [ "sha256:1c557d7d0e871de1f5ccd5833f60fb2550652da6be2693c1e02300743d21500d", @@ -420,11 +408,11 @@ }, "requests": { "hashes": [ - "sha256:11e007a8a2aa0323f5a921e9e6a2d7e4e67d9877e85773fba9ba6419025cbeb4", - "sha256:9cf5292fcd0f598c671cfc1e0d7d1a7f13bb8085e9a590f48c010551dc6c4b31" + "sha256:43999036bfa82904b6af1d99e4882b560e5e2c68e5c4b0aa03b655f3d7d73fee", + "sha256:b3f43d496c6daba4493e7c431722aeb7dbc6288f52a6e04e7b6023b0247817e6" ], "index": "pypi", - "version": "==2.22.0" + "version": "==2.23.0" }, "six": { "hashes": [ @@ -449,11 +437,11 @@ }, "sphinx": { "hashes": [ - "sha256:298537cb3234578b2d954ff18c5608468229e116a9757af3b831c2b2b4819159", - "sha256:e6e766b74f85f37a5f3e0773a1e1be8db3fcb799deb58ca6d18b70b0b44542a5" + "sha256:525527074f2e0c2585f68f73c99b4dc257c34bbe308b27f5f8c7a6e20642742f", + "sha256:543d39db5f82d83a5c1aa0c10c88f2b6cff2da3e711aa849b2c627b4b403bbd9" ], "index": "pypi", - "version": "==2.3.1" + "version": "==2.4.2" }, "sphinxcontrib-applehelp": { "hashes": [ @@ -556,6 +544,13 @@ } }, "develop": { + "appdirs": { + "hashes": [ + "sha256:9e5896d1372858f8dd3344faf4e5014d21849c756c8d5701f78f8a103b372d92", + "sha256:d8b24664561d0d34ddfaec54636d502d7cea6e29c3eaf68f3df6180863e2166e" + ], + "version": "==1.4.3" + }, "aspy.yaml": { "hashes": [ "sha256:463372c043f70160a9ec950c3f1e4c3a82db5fca01d334b6bc89c7164d744bdc", @@ -579,10 +574,10 @@ }, "cfgv": { "hashes": [ - "sha256:edb387943b665bf9c434f717bf630fa78aecd53d5900d2e05da6ad6048553144", - "sha256:fbd93c9ab0a523bf7daec408f3be2ed99a980e20b2d19b50fc184ca6b820d289" + "sha256:04b093b14ddf9fd4d17c53ebfd55582d27b76ed30050193c14e560770c5360eb", + "sha256:f22b426ed59cd2ab2b54ff96608d846c33dfb8766a67f0b4a6ce130ce244414f" ], - "version": "==2.0.1" + "version": "==3.0.0" }, "chardet": { "hashes": [ @@ -636,6 +631,12 @@ "index": "pypi", "version": "==4.5.4" }, + "distlib": { + "hashes": [ + "sha256:2e166e231a26b36d6dfe35a48c4464346620f8645ed0ace01ee31822b288de21" + ], + "version": "==0.3.0" + }, "dodgy": { "hashes": [ "sha256:28323cbfc9352139fdd3d316fa17f325cc0e9ac74438cbba51d70f9b48f86c3a", @@ -658,6 +659,13 @@ ], "version": "==0.3" }, + "filelock": { + "hashes": [ + "sha256:18d82244ee114f543149c66a6e0c14e9c4f8a1044b5cdaadd0f82159d6a6ff59", + "sha256:929b7d63ec5b7d6b71b0fa5ac14e030b3f70b75747cef1b10da9b879fef15836" + ], + "version": "==3.0.12" + }, "flake8": { "hashes": [ "sha256:45681a117ecc81e870cbf1262835ae4af5e7a8b08e40b944a8a6e6b895914cfb", @@ -668,11 +676,11 @@ }, "flake8-annotations": { "hashes": [ - "sha256:05b85538014c850a86dce7374bb6621c64481c24e35e8e90af1315f4d7a3dbaa", - "sha256:43e5233a76fda002b91a54a7cc4510f099c4bfd6279502ec70164016250eebd1" + "sha256:47705be09c6e56e9e3ac1656e8f5ed70862a4657116dc472f5a56c1bdc5172b1", + "sha256:564702ace354e1059252755be79d082a70ae1851c86044ae1a96d0f5453280e9" ], "index": "pypi", - "version": "==1.1.3" + "version": "==1.2.0" }, "flake8-bugbear": { "hashes": [ @@ -700,11 +708,11 @@ }, "flake8-string-format": { "hashes": [ - "sha256:68ea72a1a5b75e7018cae44d14f32473c798cf73d75cbaed86c6a9a907b770b2", - "sha256:774d56103d9242ed968897455ef49b7d6de272000cfa83de5814273a868832f1" + "sha256:65f3da786a1461ef77fca3780b314edb2853c377f2e35069723348c8917deaa2", + "sha256:812ff431f10576a74c89be4e85b8e075a705be39bc40c4b4278b5b13e2afa9af" ], "index": "pypi", - "version": "==0.2.3" + "version": "==0.3.0" }, "flake8-tidy-imports": { "hashes": [ @@ -730,18 +738,10 @@ }, "idna": { "hashes": [ - "sha256:c357b3f628cf53ae2c4c05627ecc484553142ca23264e593d327bcde5e9c3407", - "sha256:ea8b7f6188e6fa117537c3df7da9fc686d485087abf6ac197f9c46432f7e4a3c" + "sha256:7588d1c14ae4c77d74036e8c22ff447b26d0fde8f007354fd48a7814db15b7cb", + "sha256:a068a21ceac8a4d63dbfd964670474107f541babbd2250d61922f029858365fa" ], - "version": "==2.8" - }, - "importlib-metadata": { - "hashes": [ - "sha256:06f5b3a99029c7134207dd882428a66992a9de2bef7c2b699b5641f9886c3302", - "sha256:b97607a1a18a5100839aec1dc26a1ea17ee0d93b20b0f008d80a5a050afb200b" - ], - "markers": "python_version < '3.8'", - "version": "==1.5.0" + "version": "==2.9" }, "mccabe": { "hashes": [ @@ -818,11 +818,11 @@ }, "requests": { "hashes": [ - "sha256:11e007a8a2aa0323f5a921e9e6a2d7e4e67d9877e85773fba9ba6419025cbeb4", - "sha256:9cf5292fcd0f598c671cfc1e0d7d1a7f13bb8085e9a590f48c010551dc6c4b31" + "sha256:43999036bfa82904b6af1d99e4882b560e5e2c68e5c4b0aa03b655f3d7d73fee", + "sha256:b3f43d496c6daba4493e7c431722aeb7dbc6288f52a6e04e7b6023b0247817e6" ], "index": "pypi", - "version": "==2.22.0" + "version": "==2.23.0" }, "safety": { "hashes": [ @@ -853,33 +853,6 @@ ], "version": "==0.10.0" }, - "typed-ast": { - "hashes": [ - "sha256:0666aa36131496aed8f7be0410ff974562ab7eeac11ef351def9ea6fa28f6355", - "sha256:0c2c07682d61a629b68433afb159376e24e5b2fd4641d35424e462169c0a7919", - "sha256:249862707802d40f7f29f6e1aad8d84b5aa9e44552d2cc17384b209f091276aa", - "sha256:24995c843eb0ad11a4527b026b4dde3da70e1f2d8806c99b7b4a7cf491612652", - "sha256:269151951236b0f9a6f04015a9004084a5ab0d5f19b57de779f908621e7d8b75", - "sha256:4083861b0aa07990b619bd7ddc365eb7fa4b817e99cf5f8d9cf21a42780f6e01", - "sha256:498b0f36cc7054c1fead3d7fc59d2150f4d5c6c56ba7fb150c013fbc683a8d2d", - "sha256:4e3e5da80ccbebfff202a67bf900d081906c358ccc3d5e3c8aea42fdfdfd51c1", - "sha256:6daac9731f172c2a22ade6ed0c00197ee7cc1221aa84cfdf9c31defeb059a907", - "sha256:715ff2f2df46121071622063fc7543d9b1fd19ebfc4f5c8895af64a77a8c852c", - "sha256:73d785a950fc82dd2a25897d525d003f6378d1cb23ab305578394694202a58c3", - "sha256:8c8aaad94455178e3187ab22c8b01a3837f8ee50e09cf31f1ba129eb293ec30b", - "sha256:8ce678dbaf790dbdb3eba24056d5364fb45944f33553dd5869b7580cdbb83614", - "sha256:aaee9905aee35ba5905cfb3c62f3e83b3bec7b39413f0a7f19be4e547ea01ebb", - "sha256:bcd3b13b56ea479b3650b82cabd6b5343a625b0ced5429e4ccad28a8973f301b", - "sha256:c9e348e02e4d2b4a8b2eedb48210430658df6951fa484e59de33ff773fbd4b41", - "sha256:d205b1b46085271b4e15f670058ce182bd1199e56b317bf2ec004b6a44f911f6", - "sha256:d43943ef777f9a1c42bf4e552ba23ac77a6351de620aa9acf64ad54933ad4d34", - "sha256:d5d33e9e7af3b34a40dc05f498939f0ebf187f07c385fd58d591c533ad8562fe", - "sha256:fc0fea399acb12edbf8a628ba8d2312f583bdbdb3335635db062fa98cf71fca4", - "sha256:fe460b922ec15dd205595c9b5b99e2f056fd98ae8f9f56b888e7a17dc2b757e7" - ], - "markers": "python_version < '3.8'", - "version": "==1.4.1" - }, "unittest-xml-reporting": { "hashes": [ "sha256:358bbdaf24a26d904cc1c26ef3078bca7fc81541e0a54c8961693cc96a6f35e0", @@ -898,17 +871,10 @@ }, "virtualenv": { "hashes": [ - "sha256:0d62c70883c0342d59c11d0ddac0d954d0431321a41ab20851facf2b222598f3", - "sha256:55059a7a676e4e19498f1aad09b8313a38fcc0cdbe4fdddc0e9b06946d21b4bb" - ], - "version": "==16.7.9" - }, - "zipp": { - "hashes": [ - "sha256:ccc94ed0909b58ffe34430ea5451f07bc0c76467d7081619a454bf5c98b89e28", - "sha256:feae2f18633c32fc71f2de629bfb3bd3c9325cd4419642b1f1da42ee488d9b98" + "sha256:08f3623597ce73b85d6854fb26608a6f39ee9d055c81178dc6583803797f8994", + "sha256:de2cbdd5926c48d7b84e0300dea9e8f276f61d186e8e49223d71d91250fbaebd" ], - "version": "==2.1.0" + "version": "==20.0.4" } } } diff --git a/bot/__init__.py b/bot/__init__.py index 789ace5c0..923ef517b 100644 --- a/bot/__init__.py +++ b/bot/__init__.py @@ -4,11 +4,8 @@ import sys from logging import Logger, StreamHandler, handlers from pathlib import Path -from logmatic import JsonFormatter - - -logging.TRACE = 5 -logging.addLevelName(logging.TRACE, "TRACE") +TRACE_LEVEL = logging.TRACE = 5 +logging.addLevelName(TRACE_LEVEL, "TRACE") def monkeypatch_trace(self: logging.Logger, msg: str, *args, **kwargs) -> None: @@ -20,75 +17,29 @@ def monkeypatch_trace(self: logging.Logger, msg: str, *args, **kwargs) -> None: logger.trace("Houston, we have an %s", "interesting problem", exc_info=1) """ - if self.isEnabledFor(logging.TRACE): - self._log(logging.TRACE, msg, args, **kwargs) + if self.isEnabledFor(TRACE_LEVEL): + self._log(TRACE_LEVEL, msg, args, **kwargs) Logger.trace = monkeypatch_trace -# Set up logging -logging_handlers = [] - -# We can't import this yet, so we have to define it ourselves -DEBUG_MODE = True if 'local' in os.environ.get("SITE_URL", "local") else False - -LOG_DIR = Path("logs") -LOG_DIR.mkdir(exist_ok=True) - -if DEBUG_MODE: - logging_handlers.append(StreamHandler(stream=sys.stdout)) - - json_handler = logging.FileHandler(filename=Path(LOG_DIR, "log.json"), mode="w") - json_handler.formatter = JsonFormatter() - logging_handlers.append(json_handler) -else: - - logfile = Path(LOG_DIR, "bot.log") - megabyte = 1048576 - - filehandler = handlers.RotatingFileHandler(logfile, maxBytes=(megabyte*5), backupCount=7) - logging_handlers.append(filehandler) - - json_handler = logging.StreamHandler(stream=sys.stdout) - json_handler.formatter = JsonFormatter() - logging_handlers.append(json_handler) - - -logging.basicConfig( - format="%(asctime)s Bot: | %(name)33s | %(levelname)8s | %(message)s", - datefmt="%b %d %H:%M:%S", - level=logging.TRACE if DEBUG_MODE else logging.INFO, - handlers=logging_handlers -) - -log = logging.getLogger(__name__) - - -for key, value in logging.Logger.manager.loggerDict.items(): - # Force all existing loggers to the correct level and handlers - # This happens long before we instantiate our loggers, so - # those should still have the expected level - - if key == "bot": - continue - - if not isinstance(value, logging.Logger): - # There might be some logging.PlaceHolder objects in there - continue +DEBUG_MODE = 'local' in os.environ.get("SITE_URL", "local") - if DEBUG_MODE: - value.setLevel(logging.DEBUG) - else: - value.setLevel(logging.INFO) +log_format = logging.Formatter("%(asctime)s | %(name)s | %(levelname)s | %(message)s") - for handler in value.handlers.copy(): - value.removeHandler(handler) +stream_handler = StreamHandler(stream=sys.stdout) +stream_handler.setFormatter(log_format) - for handler in logging_handlers: - value.addHandler(handler) +log_file = Path("logs", "bot.log") +log_file.parent.mkdir(exist_ok=True) +file_handler = handlers.RotatingFileHandler(log_file, maxBytes=5242880, backupCount=7) +file_handler.setFormatter(log_format) +root_log = logging.getLogger() +root_log.setLevel(TRACE_LEVEL if DEBUG_MODE else logging.INFO) +root_log.addHandler(stream_handler) +root_log.addHandler(file_handler) -# Silence irrelevant loggers -logging.getLogger("aio_pika").setLevel(logging.ERROR) logging.getLogger("discord").setLevel(logging.ERROR) logging.getLogger("websockets").setLevel(logging.ERROR) +logging.getLogger(__name__).setLevel(TRACE_LEVEL) -- cgit v1.2.3 From 4919bb955fa3804e21084f0c451171b7779f2993 Mon Sep 17 00:00:00 2001 From: scragly <29337040+scragly@users.noreply.github.com> Date: Thu, 20 Feb 2020 23:15:42 +1000 Subject: Update flake8-annotations to v2.0. --- Pipfile | 2 +- Pipfile.lock | 53 +++++++++++++++++++++++++++++++++++++++++++++++------ tox.ini | 6 +++--- 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/Pipfile b/Pipfile index 7e63bde66..11d5e7deb 100644 --- a/Pipfile +++ b/Pipfile @@ -22,7 +22,7 @@ urllib3 = ">=1.24.2,<1.25" [dev-packages] coverage = "~=4.5" flake8 = "~=3.7" -flake8-annotations = "~=1.1" +flake8-annotations = "~=2.0" flake8-bugbear = "~=19.8" flake8-docstrings = "~=1.4" flake8-import-order = "~=0.18" diff --git a/Pipfile.lock b/Pipfile.lock index cca6e1791..633e62887 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "fe6d96850817a250e3c3601044bf8927c0553b73f4cd5cb228a89f04458a93af" + "sha256": "560a5168f32bafd9e5173d49502e10aeb278fe64ed50262da29be208f4a66739" }, "pipfile-spec": 6, "requires": { @@ -355,8 +355,7 @@ }, "pycparser": { "hashes": [ - "sha256:a988718abfad80b6b157acce7bf130a30876d27603738ac39f140993246b25b3", - "sha256:cebad02308a8c080ff0a3d3c9eec420a280dc4341cf8681381b444a176406a8e" + "sha256:a988718abfad80b6b157acce7bf130a30876d27603738ac39f140993246b25b3" ], "version": "==2.19" }, @@ -676,11 +675,11 @@ }, "flake8-annotations": { "hashes": [ - "sha256:47705be09c6e56e9e3ac1656e8f5ed70862a4657116dc472f5a56c1bdc5172b1", - "sha256:564702ace354e1059252755be79d082a70ae1851c86044ae1a96d0f5453280e9" + "sha256:19a6637a5da1bb7ea7948483ca9e2b9e15b213e687e7bf5ff8c1bfc91c185006", + "sha256:bb033b72cdd3a2b0a530bbdf2081f12fbea7d70baeaaebb5899723a45f424b8e" ], "index": "pypi", - "version": "==1.2.0" + "version": "==2.0.0" }, "flake8-bugbear": { "hashes": [ @@ -743,6 +742,14 @@ ], "version": "==2.9" }, + "importlib-metadata": { + "hashes": [ + "sha256:06f5b3a99029c7134207dd882428a66992a9de2bef7c2b699b5641f9886c3302", + "sha256:b97607a1a18a5100839aec1dc26a1ea17ee0d93b20b0f008d80a5a050afb200b" + ], + "markers": "python_version < '3.8'", + "version": "==1.5.0" + }, "mccabe": { "hashes": [ "sha256:ab8a6258860da4b6677da4bd2fe5dc2c659cff31b3ee4f7f5d64e79735b80d42", @@ -853,6 +860,33 @@ ], "version": "==0.10.0" }, + "typed-ast": { + "hashes": [ + "sha256:0666aa36131496aed8f7be0410ff974562ab7eeac11ef351def9ea6fa28f6355", + "sha256:0c2c07682d61a629b68433afb159376e24e5b2fd4641d35424e462169c0a7919", + "sha256:249862707802d40f7f29f6e1aad8d84b5aa9e44552d2cc17384b209f091276aa", + "sha256:24995c843eb0ad11a4527b026b4dde3da70e1f2d8806c99b7b4a7cf491612652", + "sha256:269151951236b0f9a6f04015a9004084a5ab0d5f19b57de779f908621e7d8b75", + "sha256:4083861b0aa07990b619bd7ddc365eb7fa4b817e99cf5f8d9cf21a42780f6e01", + "sha256:498b0f36cc7054c1fead3d7fc59d2150f4d5c6c56ba7fb150c013fbc683a8d2d", + "sha256:4e3e5da80ccbebfff202a67bf900d081906c358ccc3d5e3c8aea42fdfdfd51c1", + "sha256:6daac9731f172c2a22ade6ed0c00197ee7cc1221aa84cfdf9c31defeb059a907", + "sha256:715ff2f2df46121071622063fc7543d9b1fd19ebfc4f5c8895af64a77a8c852c", + "sha256:73d785a950fc82dd2a25897d525d003f6378d1cb23ab305578394694202a58c3", + "sha256:8c8aaad94455178e3187ab22c8b01a3837f8ee50e09cf31f1ba129eb293ec30b", + "sha256:8ce678dbaf790dbdb3eba24056d5364fb45944f33553dd5869b7580cdbb83614", + "sha256:aaee9905aee35ba5905cfb3c62f3e83b3bec7b39413f0a7f19be4e547ea01ebb", + "sha256:bcd3b13b56ea479b3650b82cabd6b5343a625b0ced5429e4ccad28a8973f301b", + "sha256:c9e348e02e4d2b4a8b2eedb48210430658df6951fa484e59de33ff773fbd4b41", + "sha256:d205b1b46085271b4e15f670058ce182bd1199e56b317bf2ec004b6a44f911f6", + "sha256:d43943ef777f9a1c42bf4e552ba23ac77a6351de620aa9acf64ad54933ad4d34", + "sha256:d5d33e9e7af3b34a40dc05f498939f0ebf187f07c385fd58d591c533ad8562fe", + "sha256:fc0fea399acb12edbf8a628ba8d2312f583bdbdb3335635db062fa98cf71fca4", + "sha256:fe460b922ec15dd205595c9b5b99e2f056fd98ae8f9f56b888e7a17dc2b757e7" + ], + "markers": "python_version < '3.8'", + "version": "==1.4.1" + }, "unittest-xml-reporting": { "hashes": [ "sha256:358bbdaf24a26d904cc1c26ef3078bca7fc81541e0a54c8961693cc96a6f35e0", @@ -875,6 +909,13 @@ "sha256:de2cbdd5926c48d7b84e0300dea9e8f276f61d186e8e49223d71d91250fbaebd" ], "version": "==20.0.4" + }, + "zipp": { + "hashes": [ + "sha256:12248a63bbdf7548f89cb4c7cda4681e537031eda29c02ea29674bc6854460c2", + "sha256:7c0f8e91abc0dc07a5068f315c52cb30c66bfbc581e5b50704c8a2f6ebae794a" + ], + "version": "==3.0.0" } } } diff --git a/tox.ini b/tox.ini index d14819d57..b8293a3b6 100644 --- a/tox.ini +++ b/tox.ini @@ -3,7 +3,7 @@ max-line-length=120 docstring-convention=all import-order-style=pycharm application_import_names=bot,tests -exclude=.cache,.venv,constants.py +exclude=.cache,.venv,.git,constants.py ignore= B311,W503,E226,S311,T000 # Missing Docstrings @@ -15,5 +15,5 @@ ignore= # Docstring Content D400,D401,D402,D404,D405,D406,D407,D408,D409,D410,D411,D412,D413,D414,D416,D417 # Type Annotations - TYP002,TYP003,TYP101,TYP102,TYP204,TYP206 -per-file-ignores=tests/*:D,TYP + ANN002,ANN003,ANN101,ANN102,ANN204,ANN206 +per-file-ignores=tests/*:D,ANN -- cgit v1.2.3 From 7cfb72f0b25647da2b2b5e83daedc22ca5bdfe23 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 02:51:21 +0000 Subject: Implement basic Sentry SDK usage --- Pipfile | 1 + Pipfile.lock | 10 +++++++++- bot/__main__.py | 2 ++ bot/constants.py | 2 +- config-default.yml | 1 + 5 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Pipfile b/Pipfile index 11d5e7deb..400e64c18 100644 --- a/Pipfile +++ b/Pipfile @@ -18,6 +18,7 @@ deepdiff = "~=4.0" requests = "~=2.22" more_itertools = "~=7.2" urllib3 = ">=1.24.2,<1.25" +sentry-sdk = "~=0.14" [dev-packages] coverage = "~=4.5" diff --git a/Pipfile.lock b/Pipfile.lock index 633e62887..fa29bf995 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "560a5168f32bafd9e5173d49502e10aeb278fe64ed50262da29be208f4a66739" + "sha256": "c7706a61eb96c06d073898018ea2dbcf5bd3b15d007496e2d60120a65647f31e" }, "pipfile-spec": 6, "requires": { @@ -413,6 +413,14 @@ "index": "pypi", "version": "==2.23.0" }, + "sentry-sdk": { + "hashes": [ + "sha256:b06dd27391fd11fb32f84fe054e6a64736c469514a718a99fb5ce1dff95d6b28", + "sha256:e023da07cfbead3868e1e2ba994160517885a32dfd994fc455b118e37989479b" + ], + "index": "pypi", + "version": "==0.14.1" + }, "six": { "hashes": [ "sha256:236bdbdce46e6e6a3d61a337c0f8b763ca1e8717c03b369e87a7ec7ce1319c0a", diff --git a/bot/__main__.py b/bot/__main__.py index 84bc7094b..4f51a08cc 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -1,10 +1,12 @@ import discord +import sentry_sdk from discord.ext.commands import when_mentioned_or from bot import patches from bot.bot import Bot from bot.constants import Bot as BotConfig, DEBUG_MODE +sentry_sdk.init(BotConfig.sentry_dsn) bot = Bot( command_prefix=when_mentioned_or(BotConfig.prefix), diff --git a/bot/constants.py b/bot/constants.py index fe8e57322..a4c65a1f8 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -193,7 +193,7 @@ class Bot(metaclass=YAMLGetter): prefix: str token: str - + sentry_dsn: str class Filter(metaclass=YAMLGetter): section = "filter" diff --git a/config-default.yml b/config-default.yml index 3345e6f2a..2eaf8ee06 100644 --- a/config-default.yml +++ b/config-default.yml @@ -1,6 +1,7 @@ bot: prefix: "!" token: !ENV "BOT_TOKEN" + sentry_dsn: !ENV "BOT_SENTRY_DSN" cooldowns: # Per channel, per tag. -- cgit v1.2.3 From 23829807867c37c293f7ab4d998b86fbc548b980 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 03:04:58 +0000 Subject: Set BOT_SENTRY_DSN environment variable for tests --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 0400ac4d2..874364a6f 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -30,7 +30,7 @@ jobs: - script: python -m flake8 displayName: 'Run linter' - - script: BOT_API_KEY=foo BOT_TOKEN=bar WOLFRAM_API_KEY=baz REDDIT_CLIENT_ID=spam REDDIT_SECRET=ham coverage run -m xmlrunner + - script: BOT_API_KEY=foo BOT_SENTRY_DSN=blah BOT_TOKEN=bar WOLFRAM_API_KEY=baz REDDIT_CLIENT_ID=spam REDDIT_SECRET=ham coverage run -m xmlrunner displayName: Run tests - script: coverage report -m && coverage xml -o coverage.xml -- cgit v1.2.3 From 03175ae8bfcb70b1b2c798267fdd23b48534b2b7 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 03:05:23 +0000 Subject: Attach extra information on a command error --- bot/cogs/error_handler.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 52893b2ee..021753081 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -15,6 +15,7 @@ from discord.ext.commands import ( UserInputError, ) from discord.ext.commands import Cog, Context +from sentry_sdk import configure_scope from bot.api import ResponseCodeError from bot.bot import Bot @@ -150,7 +151,18 @@ class ErrorHandler(Cog): log.error( f"Error executing command invoked by {ctx.message.author}: {ctx.message.content}" ) - raise e + + with configure_scope() as scope: + scope.user = { + "username": ctx.author.username, + "id": ctx.author.id + } + + scope.set_tag("command", ctx.command.qualified_name) + scope.set_tag("message_id", ctx.message.id) + scope.set_extra("full_message", ctx.message.content) + + raise e def setup(bot: Bot) -> None: -- cgit v1.2.3 From 215483e390b7f5636cf1d46c96c216ef0d05cb03 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 11:14:03 +0000 Subject: Add basic sentry usage for command errors giving user information and command context. --- bot/__main__.py | 6 +++- bot/api.py | 74 ----------------------------------------------- bot/bot.py | 2 -- bot/cogs/error_handler.py | 13 ++++----- 4 files changed, 10 insertions(+), 85 deletions(-) diff --git a/bot/__main__.py b/bot/__main__.py index 4f51a08cc..51bba6191 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -1,12 +1,16 @@ import discord import sentry_sdk from discord.ext.commands import when_mentioned_or +from sentry_sdk.integrations.aiohttp import AioHttpIntegration from bot import patches from bot.bot import Bot from bot.constants import Bot as BotConfig, DEBUG_MODE -sentry_sdk.init(BotConfig.sentry_dsn) +sentry_sdk.init( + dsn=BotConfig.sentry_dsn, + integrations=[AioHttpIntegration()] +) bot = Bot( command_prefix=when_mentioned_or(BotConfig.prefix), diff --git a/bot/api.py b/bot/api.py index 56db99828..fb126b384 100644 --- a/bot/api.py +++ b/bot/api.py @@ -141,77 +141,3 @@ def loop_is_running() -> bool: except RuntimeError: return False return True - - -class APILoggingHandler(logging.StreamHandler): - """Site API logging handler.""" - - def __init__(self, client: APIClient): - logging.StreamHandler.__init__(self) - self.client = client - - # internal batch of shipoff tasks that must not be scheduled - # on the event loop yet - scheduled when the event loop is ready. - self.queue = [] - - async def ship_off(self, payload: dict) -> None: - """Ship log payload to the logging API.""" - try: - await self.client.post('logs', json=payload) - except ResponseCodeError as err: - log.warning( - "Cannot send logging record to the site, got code %d.", - err.response.status, - extra={'via_handler': True} - ) - except Exception as err: - log.warning( - "Cannot send logging record to the site: %r", - err, - extra={'via_handler': True} - ) - - def emit(self, record: logging.LogRecord) -> None: - """ - Determine if a log record should be shipped to the logging API. - - If the asyncio event loop is not yet running, log records will instead be put in a queue - which will be consumed once the event loop is running. - - The following two conditions are set: - 1. Do not log anything below DEBUG (only applies to the monkeypatched `TRACE` level) - 2. Ignore log records originating from this logging handler itself to prevent infinite recursion - """ - if ( - record.levelno >= logging.DEBUG - and not record.__dict__.get('via_handler') - ): - payload = { - 'application': 'bot', - 'logger_name': record.name, - 'level': record.levelname.lower(), - 'module': record.module, - 'line': record.lineno, - 'message': self.format(record) - } - - task = self.ship_off(payload) - if not loop_is_running(): - self.queue.append(task) - else: - asyncio.create_task(task) - self.schedule_queued_tasks() - - def schedule_queued_tasks(self) -> None: - """Consume the queue and schedule the logging of each queued record.""" - for task in self.queue: - asyncio.create_task(task) - - if self.queue: - log.debug( - "Scheduled %d pending logging tasks.", - len(self.queue), - extra={'via_handler': True} - ) - - self.queue.clear() diff --git a/bot/bot.py b/bot/bot.py index 8f808272f..cecee7b68 100644 --- a/bot/bot.py +++ b/bot/bot.py @@ -27,8 +27,6 @@ class Bot(commands.Bot): self.http_session: Optional[aiohttp.ClientSession] = None self.api_client = api.APIClient(loop=self.loop, connector=self.connector) - log.addHandler(api.APILoggingHandler(self.api_client)) - def add_cog(self, cog: commands.Cog) -> None: """Adds a "cog" to the bot and logs the operation.""" super().add_cog(cog) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 021753081..e76f1a086 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -15,7 +15,7 @@ from discord.ext.commands import ( UserInputError, ) from discord.ext.commands import Cog, Context -from sentry_sdk import configure_scope +from sentry_sdk import capture_exception, configure_scope from bot.api import ResponseCodeError from bot.bot import Bot @@ -148,21 +148,18 @@ class ErrorHandler(Cog): f"Sorry, an unexpected error occurred. Please let us know!\n\n" f"```{e.__class__.__name__}: {e}```" ) - log.error( - f"Error executing command invoked by {ctx.message.author}: {ctx.message.content}" - ) with configure_scope() as scope: scope.user = { - "username": ctx.author.username, - "id": ctx.author.id + "id": ctx.author.id, + "username": str(ctx.author) } scope.set_tag("command", ctx.command.qualified_name) scope.set_tag("message_id", ctx.message.id) scope.set_extra("full_message", ctx.message.content) - - raise e + + capture_exception(e) def setup(bot: Bot) -> None: -- cgit v1.2.3 From 7b619a2dd17e00464d93b56717391fe53e0fe6bb Mon Sep 17 00:00:00 2001 From: Deniz Date: Fri, 21 Feb 2020 17:48:10 +0100 Subject: Use the code provided by sco1 to fix the checks failing. --- bot/cogs/information.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index c8b5eb5ad..fd49e2828 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -3,6 +3,7 @@ import logging import pprint import textwrap from collections import Counter, defaultdict +from string import Template from typing import Any, Mapping, Optional, Union from discord import Colour, Embed, Member, Message, Role, Status, utils @@ -109,10 +110,14 @@ class Information(Cog): # How many of each user status? statuses = Counter(member.status for member in ctx.guild.members) + embed = Embed(colour=Colour.blurple()) - embed = Embed( - colour=Colour.blurple(), - description=textwrap.dedent(f""" + # Because channel_counts lacks leading whitespace, it breaks the dedent if it's inserted directly by the + # f-string. While this is correctly formated by Discord, it makes unit testing difficult. To keep the formatting + # without joining a tuple of strings we can use a Template string to insert the already-formatted channel_counts + # after the dedent is made. + embed.description = Template( + textwrap.dedent(f""" **Server information** Created: {created} Voice region: {region} @@ -121,7 +126,7 @@ class Information(Cog): **Counts** Members: {member_count:,} Roles: {roles} - {channel_counts} + $channel_counts **Members** {constants.Emojis.status_online} {statuses[Status.online]:,} @@ -129,7 +134,7 @@ class Information(Cog): {constants.Emojis.status_dnd} {statuses[Status.dnd]:,} {constants.Emojis.status_offline} {statuses[Status.offline]:,} """) - ) + ).substitute({"channel_counts": channel_counts}) embed.set_thumbnail(url=ctx.guild.icon_url) await ctx.send(embed=embed) -- cgit v1.2.3 From c46498ad20c463d72e6d5da05852371f9ab20e6c Mon Sep 17 00:00:00 2001 From: Deniz Date: Fri, 21 Feb 2020 17:57:36 +0100 Subject: Remove the space that makes the test fail --- bot/cogs/information.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/information.py b/bot/cogs/information.py index fd49e2828..13c8aabaa 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -106,7 +106,7 @@ class Information(Cog): # How many of each type of channel? channels = Counter(c.type for c in ctx.guild.channels) - channel_counts = "".join(sorted(f"{str(ch).title()} channels: {channels[ch]} \n" for ch in channels)).strip() + channel_counts = "".join(sorted(f"{str(ch).title()} channels: {channels[ch]}\n" for ch in channels)).strip() # How many of each user status? statuses = Counter(member.status for member in ctx.guild.members) -- cgit v1.2.3 From aa12d72c7ad6ad54f82f113da1e916cee1903eaf Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 17:22:23 +0000 Subject: Remove tests for custom bot log --- tests/bot/test_api.py | 64 ++------------------------------------------------- 1 file changed, 2 insertions(+), 62 deletions(-) diff --git a/tests/bot/test_api.py b/tests/bot/test_api.py index 5a88adc5c..bdfcc73e4 100644 --- a/tests/bot/test_api.py +++ b/tests/bot/test_api.py @@ -1,9 +1,7 @@ -import logging import unittest -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock from bot import api -from tests.base import LoggingTestCase from tests.helpers import async_test @@ -34,7 +32,7 @@ class APIClientTests(unittest.TestCase): self.assertEqual(error.response_text, "") self.assertIs(error.response, self.error_api_response) - def test_responde_code_error_string_representation_default_initialization(self): + def test_response_code_error_string_representation_default_initialization(self): """Test the string representation of `ResponseCodeError` initialized without text or json.""" error = api.ResponseCodeError(response=self.error_api_response) self.assertEqual(str(error), f"Status: {self.error_api_response.status} Response: ") @@ -76,61 +74,3 @@ class APIClientTests(unittest.TestCase): response_text=text_data ) self.assertEqual(str(error), f"Status: {self.error_api_response.status} Response: {text_data}") - - -class LoggingHandlerTests(LoggingTestCase): - """Tests the bot's API Log Handler.""" - - @classmethod - def setUpClass(cls): - cls.debug_log_record = logging.LogRecord( - name='my.logger', level=logging.DEBUG, - pathname='my/logger.py', lineno=666, - msg="Lemon wins", args=(), - exc_info=None - ) - - cls.trace_log_record = logging.LogRecord( - name='my.logger', level=logging.TRACE, - pathname='my/logger.py', lineno=666, - msg="This will not be logged", args=(), - exc_info=None - ) - - def setUp(self): - self.log_handler = api.APILoggingHandler(None) - - def test_emit_appends_to_queue_with_stopped_event_loop(self): - """Test if `APILoggingHandler.emit` appends to queue when the event loop is not running.""" - with patch("bot.api.APILoggingHandler.ship_off") as ship_off: - # Patch `ship_off` to ease testing against the return value of this coroutine. - ship_off.return_value = 42 - self.log_handler.emit(self.debug_log_record) - - self.assertListEqual(self.log_handler.queue, [42]) - - def test_emit_ignores_less_than_debug(self): - """`APILoggingHandler.emit` should not queue logs with a log level lower than DEBUG.""" - self.log_handler.emit(self.trace_log_record) - self.assertListEqual(self.log_handler.queue, []) - - def test_schedule_queued_tasks_for_empty_queue(self): - """`APILoggingHandler` should not schedule anything when the queue is empty.""" - with self.assertNotLogs(level=logging.DEBUG): - self.log_handler.schedule_queued_tasks() - - def test_schedule_queued_tasks_for_nonempty_queue(self): - """`APILoggingHandler` should schedule logs when the queue is not empty.""" - log = logging.getLogger("bot.api") - - with self.assertLogs(logger=log, level=logging.DEBUG) as logs, patch('asyncio.create_task') as create_task: - self.log_handler.queue = [555] - self.log_handler.schedule_queued_tasks() - self.assertListEqual(self.log_handler.queue, []) - create_task.assert_called_once_with(555) - - [record] = logs.records - self.assertEqual(record.message, "Scheduled 1 pending logging tasks.") - self.assertEqual(record.levelno, logging.DEBUG) - self.assertEqual(record.name, 'bot.api') - self.assertIn('via_handler', record.__dict__) -- cgit v1.2.3 From 5c0819890259e446273707ec2dbb4b8312c9e19f Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 17:36:05 +0000 Subject: Remove AIOHTTP integration from Sentry since it is intended for AIOHTTP servers. --- bot/__main__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bot/__main__.py b/bot/__main__.py index 51bba6191..2ed078903 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -1,15 +1,13 @@ import discord import sentry_sdk from discord.ext.commands import when_mentioned_or -from sentry_sdk.integrations.aiohttp import AioHttpIntegration from bot import patches from bot.bot import Bot from bot.constants import Bot as BotConfig, DEBUG_MODE sentry_sdk.init( - dsn=BotConfig.sentry_dsn, - integrations=[AioHttpIntegration()] + dsn=BotConfig.sentry_dsn ) bot = Bot( -- cgit v1.2.3 From 24ddab52ac1db77f4dd061d22ab4d622e60da237 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 17:51:09 +0000 Subject: Use log.error instead of capture_exception --- bot/cogs/error_handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index e76f1a086..8261633ee 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -15,7 +15,7 @@ from discord.ext.commands import ( UserInputError, ) from discord.ext.commands import Cog, Context -from sentry_sdk import capture_exception, configure_scope +from sentry_sdk import configure_scope from bot.api import ResponseCodeError from bot.bot import Bot @@ -159,7 +159,7 @@ class ErrorHandler(Cog): scope.set_tag("message_id", ctx.message.id) scope.set_extra("full_message", ctx.message.content) - capture_exception(e) + log.error(f"Error executing command invoked by {ctx.message.author}: {ctx.message.content}", exc_info=e) def setup(bot: Bot) -> None: -- cgit v1.2.3 From cc623e96965d511e7408590d36b7017ea9d6b495 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 18:01:12 +0000 Subject: Use push_scope instead of configure_scope --- bot/cogs/error_handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 8261633ee..589448b9a 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -15,7 +15,7 @@ from discord.ext.commands import ( UserInputError, ) from discord.ext.commands import Cog, Context -from sentry_sdk import configure_scope +from sentry_sdk import push_scope from bot.api import ResponseCodeError from bot.bot import Bot @@ -149,7 +149,7 @@ class ErrorHandler(Cog): f"```{e.__class__.__name__}: {e}```" ) - with configure_scope() as scope: + with push_scope() as scope: scope.user = { "id": ctx.author.id, "username": str(ctx.author) -- cgit v1.2.3 From 969f1eea95a9bd44de65bd08b8a551538b7b6581 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 18:01:35 +0000 Subject: Attach jump_to url to the event --- bot/cogs/error_handler.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 589448b9a..0abb7e521 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -157,8 +157,16 @@ class ErrorHandler(Cog): scope.set_tag("command", ctx.command.qualified_name) scope.set_tag("message_id", ctx.message.id) + scope.set_tag("channel_id", ctx.channel.id) + scope.set_extra("full_message", ctx.message.content) + if ctx.guild is not None: + scope.set_extra( + "jump_to", + f"https://discordapp.com/channels/{ctx.guild.id}/{ctx.channel.id}/{ctx.message.id}" + ) + log.error(f"Error executing command invoked by {ctx.message.author}: {ctx.message.content}", exc_info=e) -- cgit v1.2.3 From 282a11ef2f52cf91464c1de056ac56b0a3a4c436 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 22:48:20 +0000 Subject: Enable logging warnings to Sentry --- bot/__init__.py | 4 ++-- bot/__main__.py | 11 ++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/bot/__init__.py b/bot/__init__.py index 923ef517b..90ab3c348 100644 --- a/bot/__init__.py +++ b/bot/__init__.py @@ -40,6 +40,6 @@ root_log.setLevel(TRACE_LEVEL if DEBUG_MODE else logging.INFO) root_log.addHandler(stream_handler) root_log.addHandler(file_handler) -logging.getLogger("discord").setLevel(logging.ERROR) -logging.getLogger("websockets").setLevel(logging.ERROR) +logging.getLogger("discord").setLevel(logging.WARNING) +logging.getLogger("websockets").setLevel(logging.WARNING) logging.getLogger(__name__).setLevel(TRACE_LEVEL) diff --git a/bot/__main__.py b/bot/__main__.py index 2ed078903..490163739 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -1,13 +1,22 @@ +import logging + import discord import sentry_sdk from discord.ext.commands import when_mentioned_or +from sentry_sdk.integrations.logging import LoggingIntegration from bot import patches from bot.bot import Bot from bot.constants import Bot as BotConfig, DEBUG_MODE +sentry_logging = LoggingIntegration( + level=logging.TRACE, + event_level=logging.WARNING +) + sentry_sdk.init( - dsn=BotConfig.sentry_dsn + dsn=BotConfig.sentry_dsn, + integrations=[sentry_logging] ) bot = Bot( -- cgit v1.2.3 From 22c2d98137f315fc8f558c6b41648c003a0ac328 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 23:03:42 +0000 Subject: Change defcon log levels from warning to info --- bot/cogs/defcon.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/defcon.py b/bot/cogs/defcon.py index 3e7350fcc..a0d8fedd5 100644 --- a/bot/cogs/defcon.py +++ b/bot/cogs/defcon.py @@ -76,12 +76,12 @@ class Defcon(Cog): if data["enabled"]: self.enabled = True self.days = timedelta(days=data["days"]) - log.warning(f"DEFCON enabled: {self.days.days} days") + log.info(f"DEFCON enabled: {self.days.days} days") else: self.enabled = False self.days = timedelta(days=0) - log.warning(f"DEFCON disabled") + log.info(f"DEFCON disabled") await self.update_channel_topic() -- cgit v1.2.3 From b554470d35176246b6a18190230b81d592389056 Mon Sep 17 00:00:00 2001 From: scragly <29337040+scragly@users.noreply.github.com> Date: Sun, 23 Feb 2020 16:12:48 +1000 Subject: Suppress NotFound on react clear, tidy imports. --- bot/pagination.py | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/bot/pagination.py b/bot/pagination.py index e82763912..d6bfe6205 100644 --- a/bot/pagination.py +++ b/bot/pagination.py @@ -1,8 +1,9 @@ import asyncio import logging -from typing import Iterable, List, Optional, Tuple +import typing as t +from contextlib import suppress -from discord import Embed, Member, Message, Reaction +import discord from discord.abc import User from discord.ext.commands import Context, Paginator @@ -89,12 +90,12 @@ class LinePaginator(Paginator): @classmethod async def paginate( cls, - lines: Iterable[str], + lines: t.List[str], ctx: Context, - embed: Embed, + embed: discord.Embed, prefix: str = "", suffix: str = "", - max_lines: Optional[int] = None, + max_lines: t.Optional[int] = None, max_size: int = 500, empty: bool = True, restrict_to_user: User = None, @@ -102,7 +103,7 @@ class LinePaginator(Paginator): footer_text: str = None, url: str = None, exception_on_empty_embed: bool = False - ) -> Optional[Message]: + ) -> t.Optional[discord.Message]: """ Use a paginator and set of reactions to provide pagination over a set of lines. @@ -114,11 +115,11 @@ class LinePaginator(Paginator): Pagination will also be removed automatically if no reaction is added for five minutes (300 seconds). Example: - >>> embed = Embed() + >>> embed = discord.Embed() >>> embed.set_author(name="Some Operation", url=url, icon_url=icon) - >>> await LinePaginator.paginate((line for line in lines), ctx, embed) + >>> await LinePaginator.paginate([line for line in lines], ctx, embed) """ - def event_check(reaction_: Reaction, user_: Member) -> bool: + def event_check(reaction_: discord.Reaction, user_: discord.Member) -> bool: """Make sure that this reaction is what we want to operate on.""" no_restrictions = ( # Pagination is not restricted @@ -281,8 +282,9 @@ class LinePaginator(Paginator): await message.edit(embed=embed) - log.debug("Ending pagination and removing all reactions...") - await message.clear_reactions() + log.debug("Ending pagination and clearing reactions.") + with suppress(discord.NotFound): + await message.clear_reactions() class ImagePaginator(Paginator): @@ -316,13 +318,13 @@ class ImagePaginator(Paginator): @classmethod async def paginate( cls, - pages: List[Tuple[str, str]], - ctx: Context, embed: Embed, + pages: t.List[t.Tuple[str, str]], + ctx: Context, embed: discord.Embed, prefix: str = "", suffix: str = "", timeout: int = 300, exception_on_empty_embed: bool = False - ) -> Optional[Message]: + ) -> t.Optional[discord.Message]: """ Use a paginator and set of reactions to provide pagination over a set of title/image pairs. @@ -334,11 +336,11 @@ class ImagePaginator(Paginator): Note: Pagination will be removed automatically if no reaction is added for five minutes (300 seconds). Example: - >>> embed = Embed() + >>> embed = discord.Embed() >>> embed.set_author(name="Some Operation", url=url, icon_url=icon) >>> await ImagePaginator.paginate(pages, ctx, embed) """ - def check_event(reaction_: Reaction, member: Member) -> bool: + def check_event(reaction_: discord.Reaction, member: discord.Member) -> bool: """Checks each reaction added, if it matches our conditions pass the wait_for.""" return all(( # Reaction is on the same message sent @@ -445,5 +447,6 @@ class ImagePaginator(Paginator): await message.edit(embed=embed) - log.debug("Ending pagination and removing all reactions...") - await message.clear_reactions() + log.debug("Ending pagination and clearing reactions.") + with suppress(discord.NotFound): + await message.clear_reactions() -- cgit v1.2.3 From a18f7ede045f10c6a6a06cae36b9450f869cf5ca Mon Sep 17 00:00:00 2001 From: scragly <29337040+scragly@users.noreply.github.com> Date: Sun, 23 Feb 2020 16:13:10 +1000 Subject: Define `_count` in `__init__`. --- bot/pagination.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/pagination.py b/bot/pagination.py index d6bfe6205..b6525c2d0 100644 --- a/bot/pagination.py +++ b/bot/pagination.py @@ -301,6 +301,7 @@ class ImagePaginator(Paginator): self._current_page = [prefix] self.images = [] self._pages = [] + self._count = 0 def add_line(self, line: str = '', *, empty: bool = False) -> None: """Adds a line to each page.""" -- cgit v1.2.3 From e8ad8bb3cf735e9fef09413b8b118b2eff601797 Mon Sep 17 00:00:00 2001 From: scragly <29337040+scragly@users.noreply.github.com> Date: Sun, 23 Feb 2020 19:42:00 +1000 Subject: Don't set project log level so it uses root level. --- bot/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/__init__.py b/bot/__init__.py index 90ab3c348..f7a410706 100644 --- a/bot/__init__.py +++ b/bot/__init__.py @@ -42,4 +42,4 @@ root_log.addHandler(file_handler) logging.getLogger("discord").setLevel(logging.WARNING) logging.getLogger("websockets").setLevel(logging.WARNING) -logging.getLogger(__name__).setLevel(TRACE_LEVEL) +logging.getLogger(__name__) -- cgit v1.2.3 From a809754a1ad7e920180b8ff841282b7337ec0e4f Mon Sep 17 00:00:00 2001 From: scragly <29337040+scragly@users.noreply.github.com> Date: Sun, 23 Feb 2020 20:32:47 +1000 Subject: Don't log exception traceback on Forbidden for welcomes. --- bot/cogs/verification.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 988e0d49a..f13ccd728 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -1,7 +1,8 @@ import logging +from contextlib import suppress from datetime import datetime -from discord import Colour, Message, NotFound, Object +from discord import Colour, Forbidden, Message, NotFound, Object from discord.ext import tasks from discord.ext.commands import Cog, Context, command @@ -127,17 +128,13 @@ class Verification(Cog): await ctx.author.add_roles(Object(Roles.verified), reason="Accepted the rules") try: await ctx.author.send(WELCOME_MESSAGE) - except Exception: - # Catch the exception, in case they have DMs off or something - log.exception(f"Unable to send welcome message to user {ctx.author}.") - - log.trace(f"Deleting the message posted by {ctx.author}.") - - try: - self.mod_log.ignore(Event.message_delete, ctx.message.id) - await ctx.message.delete() - except NotFound: - log.trace("No message found, it must have been deleted by another bot.") + except Forbidden: + log.info(f"Sending welcome message failed for {ctx.author}.") + finally: + log.trace(f"Deleting accept message by {ctx.author}.") + with suppress(NotFound): + self.mod_log.ignore(Event.message_delete, ctx.message.id) + await ctx.message.delete() @command(name='subscribe') @in_channel(Channels.bot) -- cgit v1.2.3 From cab9d3a6b2e038bbb0fe080cb0f024ad39e42e5d Mon Sep 17 00:00:00 2001 From: scragly <29337040+scragly@users.noreply.github.com> Date: Sun, 23 Feb 2020 21:39:06 +1000 Subject: Check reminder user and channel before send and schedule. --- bot/cogs/reminders.py | 55 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/bot/cogs/reminders.py b/bot/cogs/reminders.py index 45bf9a8f4..105a08465 100644 --- a/bot/cogs/reminders.py +++ b/bot/cogs/reminders.py @@ -2,12 +2,12 @@ import asyncio import logging import random import textwrap +import typing as t from datetime import datetime, timedelta from operator import itemgetter -from typing import Optional +import discord from dateutil.relativedelta import relativedelta -from discord import Colour, Embed, Message from discord.ext.commands import Cog, Context, group from bot.bot import Bot @@ -45,6 +45,10 @@ class Reminders(Scheduler, Cog): loop = asyncio.get_event_loop() for reminder in response: + is_valid, *_ = self.ensure_valid_reminder(reminder) + if not is_valid: + continue + remind_at = datetime.fromisoformat(reminder['expiration'][:-1]) # If the reminder is already overdue ... @@ -55,11 +59,26 @@ class Reminders(Scheduler, Cog): else: self.schedule_task(loop, reminder["id"], reminder) + def ensure_valid_reminder(self, reminder: dict) -> t.Tuple[bool, discord.User, discord.TextChannel]: + """Ensure reminder author and channel can be fetched otherwise delete the reminder.""" + user = self.bot.get_user(reminder['author']) + channel = self.bot.get_channel(reminder['channel_id']) + is_valid = True + if not user or not channel: + is_valid = False + log.info( + f"Reminder {reminder['id']} invalid: " + f"User {reminder['author']}={user}, Channel {reminder['channel_id']}={channel}." + ) + asyncio.create_task(self._delete_reminder(reminder['id'])) + + return is_valid, user, channel + @staticmethod async def _send_confirmation(ctx: Context, on_success: str) -> None: """Send an embed confirming the reminder change was made successfully.""" - embed = Embed() - embed.colour = Colour.green() + embed = discord.Embed() + embed.colour = discord.Colour.green() embed.title = random.choice(POSITIVE_REPLIES) embed.description = on_success await ctx.send(embed=embed) @@ -95,11 +114,13 @@ class Reminders(Scheduler, Cog): async def send_reminder(self, reminder: dict, late: relativedelta = None) -> None: """Send the reminder.""" - channel = self.bot.get_channel(reminder["channel_id"]) - user = self.bot.get_user(reminder["author"]) + is_valid, user, channel = self.ensure_valid_reminder(reminder) + if not is_valid: + await self._delete_reminder(reminder["id"]) + return - embed = Embed() - embed.colour = Colour.blurple() + embed = discord.Embed() + embed.colour = discord.Colour.blurple() embed.set_author( icon_url=Icons.remind_blurple, name="It has arrived!" @@ -111,7 +132,7 @@ class Reminders(Scheduler, Cog): embed.description += f"\n[Jump back to when you created the reminder]({reminder['jump_url']})" if late: - embed.colour = Colour.red() + embed.colour = discord.Colour.red() embed.set_author( icon_url=Icons.remind_red, name=f"Sorry it arrived {humanize_delta(late, max_units=2)} late!" @@ -129,20 +150,20 @@ class Reminders(Scheduler, Cog): await ctx.invoke(self.new_reminder, expiration=expiration, content=content) @remind_group.command(name="new", aliases=("add", "create")) - async def new_reminder(self, ctx: Context, expiration: Duration, *, content: str) -> Optional[Message]: + async def new_reminder(self, ctx: Context, expiration: Duration, *, content: str) -> t.Optional[discord.Message]: """ Set yourself a simple reminder. Expiration is parsed per: http://strftime.org/ """ - embed = Embed() + embed = discord.Embed() # If the user is not staff, we need to verify whether or not to make a reminder at all. if without_role_check(ctx, *STAFF_ROLES): # If they don't have permission to set a reminder in this channel if ctx.channel.id not in WHITELISTED_CHANNELS: - embed.colour = Colour.red() + embed.colour = discord.Colour.red() embed.title = random.choice(NEGATIVE_REPLIES) embed.description = "Sorry, you can't do that here!" @@ -159,7 +180,7 @@ class Reminders(Scheduler, Cog): # Let's limit this, so we don't get 10 000 # reminders from kip or something like that :P if len(active_reminders) > MAXIMUM_REMINDERS: - embed.colour = Colour.red() + embed.colour = discord.Colour.red() embed.title = random.choice(NEGATIVE_REPLIES) embed.description = "You have too many active reminders!" @@ -189,7 +210,7 @@ class Reminders(Scheduler, Cog): self.schedule_task(loop, reminder["id"], reminder) @remind_group.command(name="list") - async def list_reminders(self, ctx: Context) -> Optional[Message]: + async def list_reminders(self, ctx: Context) -> t.Optional[discord.Message]: """View a paginated embed of all reminders for your user.""" # Get all the user's reminders from the database. data = await self.bot.api_client.get( @@ -222,8 +243,8 @@ class Reminders(Scheduler, Cog): lines.append(text) - embed = Embed() - embed.colour = Colour.blurple() + embed = discord.Embed() + embed.colour = discord.Colour.blurple() embed.title = f"Reminders for {ctx.author}" # Remind the user that they have no reminders :^) @@ -232,7 +253,7 @@ class Reminders(Scheduler, Cog): return await ctx.send(embed=embed) # Construct the embed and paginate it. - embed.colour = Colour.blurple() + embed.colour = discord.Colour.blurple() await LinePaginator.paginate( lines, -- cgit v1.2.3 From 0585a5495ea1ee3dea2a256795005942025f6dc5 Mon Sep 17 00:00:00 2001 From: scragly <29337040+scragly@users.noreply.github.com> Date: Sun, 23 Feb 2020 21:51:58 +1000 Subject: Remove call to delete reminder, as ensure method already does it. --- bot/cogs/reminders.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bot/cogs/reminders.py b/bot/cogs/reminders.py index 105a08465..ef46f4f3e 100644 --- a/bot/cogs/reminders.py +++ b/bot/cogs/reminders.py @@ -116,7 +116,6 @@ class Reminders(Scheduler, Cog): """Send the reminder.""" is_valid, user, channel = self.ensure_valid_reminder(reminder) if not is_valid: - await self._delete_reminder(reminder["id"]) return embed = discord.Embed() -- cgit v1.2.3 From 470adee6e9069782189e803752fa2d9ee08465e4 Mon Sep 17 00:00:00 2001 From: scragly <29337040+scragly@users.noreply.github.com> Date: Mon, 24 Feb 2020 01:15:52 +1000 Subject: Reduce log level of tag cooldown notice. --- bot/cogs/tags.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bot/cogs/tags.py b/bot/cogs/tags.py index 54a51921c..b6360dfae 100644 --- a/bot/cogs/tags.py +++ b/bot/cogs/tags.py @@ -116,8 +116,10 @@ class Tags(Cog): if _command_on_cooldown(tag_name): time_left = Cooldowns.tags - (time.time() - self.tag_cooldowns[tag_name]["time"]) - log.warning(f"{ctx.author} tried to get the '{tag_name}' tag, but the tag is on cooldown. " - f"Cooldown ends in {time_left:.1f} seconds.") + log.info( + f"{ctx.author} tried to get the '{tag_name}' tag, but the tag is on cooldown. " + f"Cooldown ends in {time_left:.1f} seconds." + ) return await self._get_tags() -- cgit v1.2.3 From 12fdfde0ce1cbc4df7c7d9184acb7cda4f1d92db Mon Sep 17 00:00:00 2001 From: scragly <29337040+scragly@users.noreply.github.com> Date: Mon, 24 Feb 2020 01:49:44 +1000 Subject: Change verification post log level to info, tidy code. --- bot/cogs/verification.py | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index f13ccd728..582237374 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -93,19 +93,21 @@ class Verification(Cog): ping_everyone=Filter.ping_everyone, ) - ctx = await self.bot.get_context(message) # type: Context - + ctx: Context = await self.bot.get_context(message) if ctx.command is not None and ctx.command.name == "accept": - return # They used the accept command + return - for role in ctx.author.roles: - if role.id == Roles.verified: - log.warning(f"{ctx.author} posted '{ctx.message.content}' " - "in the verification channel, but is already verified.") - return # They're already verified + if any(r.id == Roles.verified for r in ctx.author.roles): + log.info( + f"{ctx.author} posted '{ctx.message.content}' " + "in the verification channel, but is already verified." + ) + return - log.debug(f"{ctx.author} posted '{ctx.message.content}' in the verification " - "channel. We are providing instructions how to verify.") + log.debug( + f"{ctx.author} posted '{ctx.message.content}' in the verification " + "channel. We are providing instructions how to verify." + ) await ctx.send( f"{ctx.author.mention} Please type `!accept` to verify that you accept our rules, " f"and gain access to the rest of the server.", @@ -113,11 +115,8 @@ class Verification(Cog): ) log.trace(f"Deleting the message posted by {ctx.author}") - - try: + with suppress(NotFound): await ctx.message.delete() - except NotFound: - log.trace("No message found, it must have been deleted by another bot.") @command(name='accept', aliases=('verify', 'verified', 'accepted'), hidden=True) @without_role(Roles.verified) -- cgit v1.2.3 From 5c8a4954d9d22e444cfed6f35057d85185100043 Mon Sep 17 00:00:00 2001 From: Leon Sandøy Date: Sun, 23 Feb 2020 18:14:26 +0100 Subject: Add Sentdex server to whitelist --- config-default.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/config-default.yml b/config-default.yml index 2eaf8ee06..ba6ea2742 100644 --- a/config-default.yml +++ b/config-default.yml @@ -217,6 +217,7 @@ filter: - 438622377094414346 # Pyglet - 524691714909274162 # Panda3D - 336642139381301249 # discord.py + - 405403391410438165 # Sentdex domain_blacklist: - pornhub.com -- cgit v1.2.3