From fba165037943fda90039ec9cadf0649cfae0e781 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Mon, 23 Sep 2019 21:13:47 +0200 Subject: Fix failing duration conversion https://github.com/python-discord/bot/issues/446 The current ExpirationDate converter does not convert duration strings to `datetime.datetime` objects correctly. To remedy the problem, I've written a new Duration converter that uses regex matching to extract the relevant duration units and `dateutil.relativedelta.relativedelta` to compute a `datetime.datetime` that's the given duration in the future. I've left the old `ExpirationDate` converter in place for now, since the new Duration converter may not be the most optimal method. However, given the importance of being able to convert durations for moderation tasks, I think it's better to implement Duration now and rethink the approach later. This commit closes #446 --- bot/cogs/antispam.py | 4 +- bot/cogs/moderation.py | 12 +++--- bot/cogs/reminders.py | 8 ++-- bot/cogs/superstarify/__init__.py | 4 +- bot/converters.py | 35 +++++++++++++++++ tests/test_converters.py | 82 +++++++++++++++++++++++++++++++++++++-- 6 files changed, 127 insertions(+), 18 deletions(-) diff --git a/bot/cogs/antispam.py b/bot/cogs/antispam.py index 7a3360436..8dfa0ad05 100644 --- a/bot/cogs/antispam.py +++ b/bot/cogs/antispam.py @@ -17,7 +17,7 @@ from bot.constants import ( Guild as GuildConfig, Icons, STAFF_ROLES, ) -from bot.converters import ExpirationDate +from bot.converters import Duration log = logging.getLogger(__name__) @@ -102,7 +102,7 @@ class AntiSpam(Cog): self.validation_errors = validation_errors role_id = AntiSpamConfig.punishment['role_id'] self.muted_role = Object(role_id) - self.expiration_date_converter = ExpirationDate() + self.expiration_date_converter = Duration() self.message_deletion_queue = dict() self.queue_consumption_tasks = dict() diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py index 81b3864a7..4d651bef7 100644 --- a/bot/cogs/moderation.py +++ b/bot/cogs/moderation.py @@ -14,7 +14,7 @@ from discord.ext.commands import ( from bot import constants from bot.cogs.modlog import ModLog from bot.constants import Colours, Event, Icons, MODERATION_ROLES -from bot.converters import ExpirationDate, InfractionSearchQuery +from bot.converters import Duration, InfractionSearchQuery from bot.decorators import with_role from bot.pagination import LinePaginator from bot.utils.moderation import already_has_active_infraction, post_infraction @@ -279,7 +279,7 @@ class Moderation(Scheduler, Cog): @with_role(*MODERATION_ROLES) @command() - async def tempmute(self, ctx: Context, user: Member, duration: ExpirationDate, *, reason: str = None) -> None: + async def tempmute(self, ctx: Context, user: Member, duration: Duration, *, reason: str = None) -> None: """ Create a temporary mute infraction for a user with the provided expiration and reason. @@ -345,7 +345,7 @@ class Moderation(Scheduler, Cog): @with_role(*MODERATION_ROLES) @command() - async def tempban(self, ctx: Context, user: UserTypes, duration: ExpirationDate, *, reason: str = None) -> None: + async def tempban(self, ctx: Context, user: UserTypes, duration: Duration, *, reason: str = None) -> None: """ Create a temporary ban infraction for a user with the provided expiration and reason. @@ -600,7 +600,7 @@ class Moderation(Scheduler, Cog): @with_role(*MODERATION_ROLES) @command(hidden=True, aliases=["shadowtempmute, stempmute"]) async def shadow_tempmute( - self, ctx: Context, user: Member, duration: ExpirationDate, *, reason: str = None + self, ctx: Context, user: Member, duration: Duration, *, reason: str = None ) -> None: """ Create a temporary mute infraction for a user with the provided reason. @@ -653,7 +653,7 @@ class Moderation(Scheduler, Cog): @with_role(*MODERATION_ROLES) @command(hidden=True, aliases=["shadowtempban, stempban"]) async def shadow_tempban( - self, ctx: Context, user: UserTypes, duration: ExpirationDate, *, reason: str = None + self, ctx: Context, user: UserTypes, duration: Duration, *, reason: str = None ) -> None: """ Create a temporary ban infraction for a user with the provided reason. @@ -884,7 +884,7 @@ class Moderation(Scheduler, Cog): @infraction_edit_group.command(name="duration") async def edit_duration( self, ctx: Context, - infraction_id: int, expires_at: Union[ExpirationDate, str] + infraction_id: int, expires_at: Union[Duration, str] ) -> None: """ Sets the duration of the given infraction, relative to the time of updating. diff --git a/bot/cogs/reminders.py b/bot/cogs/reminders.py index 8460de91f..c37abf21e 100644 --- a/bot/cogs/reminders.py +++ b/bot/cogs/reminders.py @@ -11,7 +11,7 @@ from discord import Colour, Embed, Message from discord.ext.commands import Bot, Cog, Context, group from bot.constants import Channels, Icons, NEGATIVE_REPLIES, POSITIVE_REPLIES, STAFF_ROLES -from bot.converters import ExpirationDate +from bot.converters import Duration from bot.pagination import LinePaginator from bot.utils.checks import without_role_check from bot.utils.scheduling import Scheduler @@ -118,12 +118,12 @@ class Reminders(Scheduler, Cog): await self._delete_reminder(reminder["id"]) @group(name="remind", aliases=("reminder", "reminders"), invoke_without_command=True) - async def remind_group(self, ctx: Context, expiration: ExpirationDate, *, content: str) -> None: + async def remind_group(self, ctx: Context, expiration: Duration, *, content: str) -> None: """Commands for managing your reminders.""" 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: ExpirationDate, *, content: str) -> Optional[Message]: + async def new_reminder(self, ctx: Context, expiration: Duration, *, content: str) -> Optional[Message]: """ Set yourself a simple reminder. @@ -237,7 +237,7 @@ class Reminders(Scheduler, Cog): await ctx.invoke(self.bot.get_command("help"), "reminders", "edit") @edit_reminder_group.command(name="duration", aliases=("time",)) - async def edit_reminder_duration(self, ctx: Context, id_: int, expiration: ExpirationDate) -> None: + async def edit_reminder_duration(self, ctx: Context, id_: int, expiration: Duration) -> None: """ Edit one of your reminder's expiration. diff --git a/bot/cogs/superstarify/__init__.py b/bot/cogs/superstarify/__init__.py index f7d6a269d..b1936ef3a 100644 --- a/bot/cogs/superstarify/__init__.py +++ b/bot/cogs/superstarify/__init__.py @@ -10,7 +10,7 @@ from bot.cogs.moderation import Moderation from bot.cogs.modlog import ModLog from bot.cogs.superstarify.stars import get_nick from bot.constants import Icons, MODERATION_ROLES, POSITIVE_REPLIES -from bot.converters import ExpirationDate +from bot.converters import Duration from bot.decorators import with_role from bot.utils.moderation import post_infraction @@ -153,7 +153,7 @@ class Superstarify(Cog): @command(name='superstarify', aliases=('force_nick', 'star')) @with_role(*MODERATION_ROLES) async def superstarify( - self, ctx: Context, member: Member, expiration: ExpirationDate, reason: str = None + self, ctx: Context, member: Member, expiration: Duration, reason: str = None ) -> None: """ Force a random superstar name (like Taylor Swift) to be the user's nickname for a specified duration. diff --git a/bot/converters.py b/bot/converters.py index 7386187ab..b7340982b 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -1,4 +1,5 @@ import logging +import re from datetime import datetime from ssl import CertificateError from typing import Union @@ -6,6 +7,7 @@ from typing import Union import dateparser import discord from aiohttp import ClientConnectorError +from dateutil.relativedelta import relativedelta from discord.ext.commands import BadArgument, Context, Converter @@ -197,3 +199,36 @@ class ExpirationDate(Converter): expiry = now + (now - expiry) return expiry + + +class Duration(Converter): + """Convert duration strings into UTC datetime.datetime objects.""" + + duration_parser = re.compile( + r"((?P\d+?)(years|year|Y|y))?" + r"((?P\d+?)(months|month|m))?" + r"((?P\d+?)(weeks|week|W|w))?" + r"((?P\d+?)(days|day|D|d))?" + r"((?P\d+?)(hours|hour|H|h))?" + r"((?P\d+?)(minutes|minute|M))?" + r"((?P\d+?)(seconds|second|S|s))?" + ) + + async def convert(self, ctx: Context, duration: str) -> datetime: + """ + Converts a `duration` string to a datetime object that's `duration` in the future. + + The converter supports years (symbols: `years`, `year, `Y`, `y`), months (`months`, `month`, + `m`), weeks (`weeks`, `week`, `W`, `w`), days (`days`, `day`, `D`, `d`), hours (`hours`, + `hour`, `H`, `h`), minutes (`minutes`, `minute`, `M`), and seconds (`seconds`, `second`, + `S`, `s`), The units must be provided in descending order of magnitude. + """ + match = self.duration_parser.fullmatch(duration) + if not match: + raise BadArgument(f"`{duration}` is not a valid duration string.") + + duration_dict = {unit: int(amount) for unit, amount in match.groupdict().items() if amount} + delta = relativedelta(**duration_dict) + now = datetime.utcnow() + + return now + delta diff --git a/tests/test_converters.py b/tests/test_converters.py index 3cf774c80..3cf00035f 100644 --- a/tests/test_converters.py +++ b/tests/test_converters.py @@ -1,11 +1,12 @@ import asyncio -from datetime import datetime -from unittest.mock import MagicMock +import datetime +from unittest.mock import MagicMock, patch import pytest from discord.ext.commands import BadArgument from bot.converters import ( + Duration, ExpirationDate, TagContentConverter, TagNameConverter, @@ -17,10 +18,10 @@ from bot.converters import ( ('value', 'expected'), ( # sorry aliens - ('2199-01-01T00:00:00', datetime(2199, 1, 1)), + ('2199-01-01T00:00:00', datetime.datetime(2199, 1, 1)), ) ) -def test_expiration_date_converter_for_valid(value: str, expected: datetime): +def test_expiration_date_converter_for_valid(value: str, expected: datetime.datetime): converter = ExpirationDate() assert asyncio.run(converter.convert(None, value)) == expected @@ -91,3 +92,76 @@ def test_valid_python_identifier_for_valid(value: str): def test_valid_python_identifier_for_invalid(value: str): with pytest.raises(BadArgument, match=f'`{value}` is not a valid Python identifier'): asyncio.run(ValidPythonIdentifier.convert(None, value)) + + +FIXED_UTC_NOW = datetime.datetime.fromisoformat('2019-01-01T00:00:00') + + +@pytest.mark.parametrize( + ('duration', 'expected'), + ( + # Simple duration strings + ('1Y', datetime.datetime.fromisoformat('2020-01-01T00:00:00')), + ('1y', datetime.datetime.fromisoformat('2020-01-01T00:00:00')), + ('1year', datetime.datetime.fromisoformat('2020-01-01T00:00:00')), + ('1years', datetime.datetime.fromisoformat('2020-01-01T00:00:00')), + ('1m', datetime.datetime.fromisoformat('2019-02-01T00:00:00')), + ('1month', datetime.datetime.fromisoformat('2019-02-01T00:00:00')), + ('1months', datetime.datetime.fromisoformat('2019-02-01T00:00:00')), + ('1w', datetime.datetime.fromisoformat('2019-01-08T00:00:00')), + ('1W', datetime.datetime.fromisoformat('2019-01-08T00:00:00')), + ('1week', datetime.datetime.fromisoformat('2019-01-08T00:00:00')), + ('1weeks', datetime.datetime.fromisoformat('2019-01-08T00:00:00')), + ('1d', datetime.datetime.fromisoformat('2019-01-02T00:00:00')), + ('1D', datetime.datetime.fromisoformat('2019-01-02T00:00:00')), + ('1day', datetime.datetime.fromisoformat('2019-01-02T00:00:00')), + ('1days', datetime.datetime.fromisoformat('2019-01-02T00:00:00')), + ('1h', datetime.datetime.fromisoformat('2019-01-01T01:00:00')), + ('1H', datetime.datetime.fromisoformat('2019-01-01T01:00:00')), + ('1hour', datetime.datetime.fromisoformat('2019-01-01T01:00:00')), + ('1hours', datetime.datetime.fromisoformat('2019-01-01T01:00:00')), + ('1M', datetime.datetime.fromisoformat('2019-01-01T00:01:00')), + ('1minute', datetime.datetime.fromisoformat('2019-01-01T00:01:00')), + ('1minutes', datetime.datetime.fromisoformat('2019-01-01T00:01:00')), + ('1s', datetime.datetime.fromisoformat('2019-01-01T00:00:01')), + ('1S', datetime.datetime.fromisoformat('2019-01-01T00:00:01')), + ('1second', datetime.datetime.fromisoformat('2019-01-01T00:00:01')), + ('1seconds', datetime.datetime.fromisoformat('2019-01-01T00:00:01')), + + # Complex duration strings + ('1y1m1w1d1H1M1S', datetime.datetime.fromisoformat('2020-02-09T01:01:01')), + ('5y100S', datetime.datetime.fromisoformat('2024-01-01T00:01:40')), + ('2w28H', datetime.datetime.fromisoformat('2019-01-16T04:00:00')), + ) +) +def test_duration_converter_for_valid(duration: str, expected: datetime): + converter = Duration() + + with patch('bot.converters.datetime') as mock_datetime: + mock_datetime.utcnow.return_value = FIXED_UTC_NOW + assert asyncio.run(converter.convert(None, duration)) == expected + + +@pytest.mark.parametrize( + ('duration'), + ( + # Units in wrong order + ('1d1w'), + ('1s1y'), + + # Unknown substrings + ('1MVes'), + ('1y3breads'), + + # Missing amount + ('ym'), + + # Garbage + ('Guido van Rossum'), + ('lemon lemon lemon lemon lemon lemon lemon'), + ) +) +def test_duration_converter_for_invalid(duration: str): + converter = Duration() + with pytest.raises(BadArgument, match=f'`{duration}` is not a valid duration string.'): + asyncio.run(converter.convert(None, duration)) -- cgit v1.2.3 From 56a365b24e7f6e145718a13edc410848ab169a06 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Mon, 23 Sep 2019 23:03:58 +0200 Subject: Allow whitespace in duration strings and update tests https://github.com/python-discord/bot/issues/446 After review feedback and a discussion in the dev-core team, I've changed a couple of things: - Allow a space between amount and unit in the duration string; - Allow a space between different units in the duration string; - Remove the old ExpirationDate converter completely; - Remove the dependency `dateparser` from the Pipfile; - Update tests for the two types of optional spaces; - Change the test for valid cases to a more readable format; This PR closes #446 --- Pipfile | 1 - Pipfile.lock | 57 ++++++------------------ bot/converters.py | 39 ++++------------ tests/test_converters.py | 113 +++++++++++++++++++++++++++-------------------- 4 files changed, 87 insertions(+), 123 deletions(-) diff --git a/Pipfile b/Pipfile index 6a58054c1..33f44e9a6 100644 --- a/Pipfile +++ b/Pipfile @@ -17,7 +17,6 @@ aio-pika = "*" python-dateutil = "*" deepdiff = "*" requests = "*" -dateparser = "*" more_itertools = "~=7.2" urllib3 = ">=1.24.2,<1.25" diff --git a/Pipfile.lock b/Pipfile.lock index 9bdcff923..7674acb26 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "29aaaa90a070d544e5b39fb6033410daa9bb7f658077205e44099f3175f6822b" + "sha256": "d582b1e226b1ce675817161d9059352d8f303c1bc1646034a9e73673f6581d12" }, "pipfile-spec": 6, "requires": { @@ -62,10 +62,10 @@ }, "aiormq": { "hashes": [ - "sha256:0b755b748d87d5ec63b4b7f162102333bf0901caf1f8a2bf29467bbdd54e637d", - "sha256:f8eef1f98bc331a266404d925745fac589dab30412688564d740754d6d643863" + "sha256:c3e4dd01a2948a75f739fb637334dbb8c6f1a4cecf74d5ed662dc3bab7f39973", + "sha256:e220d3f9477bb2959b729b79bec815148ddb8a7686fc6c3d05d41c88ebd7c59e" ], - "version": "==2.7.5" + "version": "==2.8.0" }, "alabaster": { "hashes": [ @@ -150,14 +150,6 @@ ], "version": "==3.0.4" }, - "dateparser": { - "hashes": [ - "sha256:983d84b5e3861cb0aa240cad07f12899bb10b62328aae188b9007e04ce37d665", - "sha256:e1eac8ef28de69a554d5fcdb60b172d526d61924b1a40afbbb08df459a36006b" - ], - "index": "pypi", - "version": "==0.7.2" - }, "deepdiff": { "hashes": [ "sha256:1123762580af0904621136d117c8397392a244d3ff0fa0a50de57a7939582476", @@ -342,10 +334,10 @@ }, "packaging": { "hashes": [ - "sha256:a7ac867b97fdc07ee80a8058fe4435ccd274ecc3b0ed61d852d7d53055528cf9", - "sha256:c491ca87294da7cc01902edbe30a5bc6c4c28172b5138ab4e4aa1b9d7bfaeafe" + "sha256:28b924174df7a2fa32c1953825ff29c61e2f5e082343165438812f00d3a7fc47", + "sha256:d9551545c6d761f3def1677baf08ab2a3ca17c56879e70fecba2fc4dde4ed108" ], - "version": "==19.1" + "version": "==19.2" }, "pamqp": { "hashes": [ @@ -432,22 +424,6 @@ "index": "pypi", "version": "==5.1.2" }, - "regex": { - "hashes": [ - "sha256:1e9f9bc44ca195baf0040b1938e6801d2f3409661c15fe57f8164c678cfc663f", - "sha256:587b62d48ca359d2d4f02d486f1f0aa9a20fbaf23a9d4198c4bed72ab2f6c849", - "sha256:835ccdcdc612821edf132c20aef3eaaecfb884c9454fdc480d5887562594ac61", - "sha256:93f6c9da57e704e128d90736430c5c59dd733327882b371b0cae8833106c2a21", - "sha256:a46f27d267665016acb3ec8c6046ec5eae8cf80befe85ba47f43c6f5ec636dcd", - "sha256:c5c8999b3a341b21ac2c6ec704cfcccbc50f1fedd61b6a8ee915ca7fd4b0a557", - "sha256:d4d1829cf97632673aa49f378b0a2c3925acd795148c5ace8ef854217abbee89", - "sha256:d96479257e8e4d1d7800adb26bf9c5ca5bab1648a1eddcac84d107b73dc68327", - "sha256:f20f4912daf443220436759858f96fefbfc6c6ba9e67835fd6e4e9b73582791a", - "sha256:f2b37b5b2c2a9d56d9e88efef200ec09c36c7f323f9d58d0b985a90923df386d", - "sha256:fe765b809a1f7ce642c2edeee351e7ebd84391640031ba4b60af8d91a9045890" - ], - "version": "==2019.8.19" - }, "requests": { "hashes": [ "sha256:11e007a8a2aa0323f5a921e9e6a2d7e4e67d9877e85773fba9ba6419025cbeb4", @@ -526,13 +502,6 @@ ], "version": "==1.1.3" }, - "tzlocal": { - "hashes": [ - "sha256:11c9f16e0a633b4b60e1eede97d8a46340d042e67b670b290ca526576e039048", - "sha256:949b9dd5ba4be17190a80c0268167d7e6c92c62b30026cf9764caf3e308e5590" - ], - "version": "==2.0.0" - }, "urllib3": { "hashes": [ "sha256:2393a695cd12afedd0dcb26fe5d50d0cf248e5a66f75dbd89a3d4eb333a61af4", @@ -800,10 +769,10 @@ }, "packaging": { "hashes": [ - "sha256:a7ac867b97fdc07ee80a8058fe4435ccd274ecc3b0ed61d852d7d53055528cf9", - "sha256:c491ca87294da7cc01902edbe30a5bc6c4c28172b5138ab4e4aa1b9d7bfaeafe" + "sha256:28b924174df7a2fa32c1953825ff29c61e2f5e082343165438812f00d3a7fc47", + "sha256:d9551545c6d761f3def1677baf08ab2a3ca17c56879e70fecba2fc4dde4ed108" ], - "version": "==19.1" + "version": "==19.2" }, "pluggy": { "hashes": [ @@ -857,11 +826,11 @@ }, "pytest": { "hashes": [ - "sha256:95d13143cc14174ca1a01ec68e84d76ba5d9d493ac02716fd9706c949a505210", - "sha256:b78fe2881323bd44fd9bd76e5317173d4316577e7b1cddebae9136a4495ec865" + "sha256:813b99704b22c7d377bbd756ebe56c35252bb710937b46f207100e843440b3c2", + "sha256:cc6620b96bc667a0c8d4fa592a8c9c94178a1bd6cc799dbb057dfd9286d31a31" ], "index": "pypi", - "version": "==5.1.2" + "version": "==5.1.3" }, "pytest-cov": { "hashes": [ diff --git a/bot/converters.py b/bot/converters.py index b7340982b..db3e2a426 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -4,7 +4,6 @@ from datetime import datetime from ssl import CertificateError from typing import Union -import dateparser import discord from aiohttp import ClientConnectorError from dateutil.relativedelta import relativedelta @@ -179,39 +178,17 @@ class TagContentConverter(Converter): return tag_content -class ExpirationDate(Converter): - """Convert relative expiration date into UTC datetime using dateparser.""" - - DATEPARSER_SETTINGS = { - 'PREFER_DATES_FROM': 'future', - 'TIMEZONE': 'UTC', - 'TO_TIMEZONE': 'UTC' - } - - async def convert(self, ctx: Context, expiration_string: str) -> datetime: - """Convert relative expiration date into UTC datetime.""" - expiry = dateparser.parse(expiration_string, settings=self.DATEPARSER_SETTINGS) - if expiry is None: - raise BadArgument(f"Failed to parse expiration date from `{expiration_string}`") - - now = datetime.utcnow() - if expiry < now: - expiry = now + (now - expiry) - - return expiry - - class Duration(Converter): """Convert duration strings into UTC datetime.datetime objects.""" duration_parser = re.compile( - r"((?P\d+?)(years|year|Y|y))?" - r"((?P\d+?)(months|month|m))?" - r"((?P\d+?)(weeks|week|W|w))?" - r"((?P\d+?)(days|day|D|d))?" - r"((?P\d+?)(hours|hour|H|h))?" - r"((?P\d+?)(minutes|minute|M))?" - r"((?P\d+?)(seconds|second|S|s))?" + r"((?P\d+?) ?(years|year|Y|y) ?)?" + r"((?P\d+?) ?(months|month|m) ?)?" + r"((?P\d+?) ?(weeks|week|W|w) ?)?" + r"((?P\d+?) ?(days|day|D|d) ?)?" + r"((?P\d+?) ?(hours|hour|H|h) ?)?" + r"((?P\d+?) ?(minutes|minute|M) ?)?" + r"((?P\d+?) ?(seconds|second|S|s))?" ) async def convert(self, ctx: Context, duration: str) -> datetime: @@ -227,7 +204,7 @@ class Duration(Converter): if not match: raise BadArgument(f"`{duration}` is not a valid duration string.") - duration_dict = {unit: int(amount) for unit, amount in match.groupdict().items() if amount} + duration_dict = {unit: int(amount) for unit, amount in match.groupdict(default=0).items()} delta = relativedelta(**duration_dict) now = datetime.utcnow() diff --git a/tests/test_converters.py b/tests/test_converters.py index 3cf00035f..35fc5d88e 100644 --- a/tests/test_converters.py +++ b/tests/test_converters.py @@ -3,29 +3,17 @@ import datetime from unittest.mock import MagicMock, patch import pytest +from dateutil.relativedelta import relativedelta from discord.ext.commands import BadArgument from bot.converters import ( Duration, - ExpirationDate, TagContentConverter, TagNameConverter, ValidPythonIdentifier, ) -@pytest.mark.parametrize( - ('value', 'expected'), - ( - # sorry aliens - ('2199-01-01T00:00:00', datetime.datetime(2199, 1, 1)), - ) -) -def test_expiration_date_converter_for_valid(value: str, expected: datetime.datetime): - converter = ExpirationDate() - assert asyncio.run(converter.convert(None, value)) == expected - - @pytest.mark.parametrize( ('value', 'expected'), ( @@ -97,46 +85,68 @@ def test_valid_python_identifier_for_invalid(value: str): FIXED_UTC_NOW = datetime.datetime.fromisoformat('2019-01-01T00:00:00') -@pytest.mark.parametrize( - ('duration', 'expected'), - ( +@pytest.fixture( + params=( # Simple duration strings - ('1Y', datetime.datetime.fromisoformat('2020-01-01T00:00:00')), - ('1y', datetime.datetime.fromisoformat('2020-01-01T00:00:00')), - ('1year', datetime.datetime.fromisoformat('2020-01-01T00:00:00')), - ('1years', datetime.datetime.fromisoformat('2020-01-01T00:00:00')), - ('1m', datetime.datetime.fromisoformat('2019-02-01T00:00:00')), - ('1month', datetime.datetime.fromisoformat('2019-02-01T00:00:00')), - ('1months', datetime.datetime.fromisoformat('2019-02-01T00:00:00')), - ('1w', datetime.datetime.fromisoformat('2019-01-08T00:00:00')), - ('1W', datetime.datetime.fromisoformat('2019-01-08T00:00:00')), - ('1week', datetime.datetime.fromisoformat('2019-01-08T00:00:00')), - ('1weeks', datetime.datetime.fromisoformat('2019-01-08T00:00:00')), - ('1d', datetime.datetime.fromisoformat('2019-01-02T00:00:00')), - ('1D', datetime.datetime.fromisoformat('2019-01-02T00:00:00')), - ('1day', datetime.datetime.fromisoformat('2019-01-02T00:00:00')), - ('1days', datetime.datetime.fromisoformat('2019-01-02T00:00:00')), - ('1h', datetime.datetime.fromisoformat('2019-01-01T01:00:00')), - ('1H', datetime.datetime.fromisoformat('2019-01-01T01:00:00')), - ('1hour', datetime.datetime.fromisoformat('2019-01-01T01:00:00')), - ('1hours', datetime.datetime.fromisoformat('2019-01-01T01:00:00')), - ('1M', datetime.datetime.fromisoformat('2019-01-01T00:01:00')), - ('1minute', datetime.datetime.fromisoformat('2019-01-01T00:01:00')), - ('1minutes', datetime.datetime.fromisoformat('2019-01-01T00:01:00')), - ('1s', datetime.datetime.fromisoformat('2019-01-01T00:00:01')), - ('1S', datetime.datetime.fromisoformat('2019-01-01T00:00:01')), - ('1second', datetime.datetime.fromisoformat('2019-01-01T00:00:01')), - ('1seconds', datetime.datetime.fromisoformat('2019-01-01T00:00:01')), + ('1Y', {"years": 1}), + ('1y', {"years": 1}), + ('1year', {"years": 1}), + ('1years', {"years": 1}), + ('1m', {"months": 1}), + ('1month', {"months": 1}), + ('1months', {"months": 1}), + ('1w', {"weeks": 1}), + ('1W', {"weeks": 1}), + ('1week', {"weeks": 1}), + ('1weeks', {"weeks": 1}), + ('1d', {"days": 1}), + ('1D', {"days": 1}), + ('1day', {"days": 1}), + ('1days', {"days": 1}), + ('1h', {"hours": 1}), + ('1H', {"hours": 1}), + ('1hour', {"hours": 1}), + ('1hours', {"hours": 1}), + ('1M', {"minutes": 1}), + ('1minute', {"minutes": 1}), + ('1minutes', {"minutes": 1}), + ('1s', {"seconds": 1}), + ('1S', {"seconds": 1}), + ('1second', {"seconds": 1}), + ('1seconds', {"seconds": 1}), # Complex duration strings - ('1y1m1w1d1H1M1S', datetime.datetime.fromisoformat('2020-02-09T01:01:01')), - ('5y100S', datetime.datetime.fromisoformat('2024-01-01T00:01:40')), - ('2w28H', datetime.datetime.fromisoformat('2019-01-16T04:00:00')), + ( + '1y1m1w1d1H1M1S', + { + "years": 1, + "months": 1, + "weeks": 1, + "days": 1, + "hours": 1, + "minutes": 1, + "seconds": 1 + } + ), + ('5y100S', {"years": 5, "seconds": 100}), + ('2w28H', {"weeks": 2, "hours": 28}), + + # Duration strings with spaces + ('1 year 2 months', {"years": 1, "months": 2}), + ('1d 2H', {"days": 1, "hours": 2}), + ('1 week2 days', {"weeks": 1, "days": 2}), ) ) -def test_duration_converter_for_valid(duration: str, expected: datetime): - converter = Duration() +def create_future_datetime(request): + """Yields duration string and target datetime.datetime object.""" + duration, duration_dict = request.param + future_datetime = FIXED_UTC_NOW + relativedelta(**duration_dict) + yield duration, future_datetime + +def test_duration_converter_for_valid(create_future_datetime: tuple): + converter = Duration() + duration, expected = create_future_datetime with patch('bot.converters.datetime') as mock_datetime: mock_datetime.utcnow.return_value = FIXED_UTC_NOW assert asyncio.run(converter.convert(None, duration)) == expected @@ -149,6 +159,10 @@ def test_duration_converter_for_valid(duration: str, expected: datetime): ('1d1w'), ('1s1y'), + # Duplicated units + ('1 year 2 years'), + ('1 M 10 minutes'), + # Unknown substrings ('1MVes'), ('1y3breads'), @@ -156,6 +170,11 @@ def test_duration_converter_for_valid(duration: str, expected: datetime): # Missing amount ('ym'), + # Incorrect whitespace + (" 1y"), + ("1S "), + ("1y 1m"), + # Garbage ('Guido van Rossum'), ('lemon lemon lemon lemon lemon lemon lemon'), -- cgit v1.2.3 From 3691657184304e2b1e5e06aea75cf9bbaf858bc6 Mon Sep 17 00:00:00 2001 From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com> Date: Mon, 23 Sep 2019 23:19:05 +0200 Subject: Restructure Duration converter docstring In this commit, I've restructured the Duration converter's docstring to a more readable format: a bullet point list. --- bot/converters.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index db3e2a426..339da7b60 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -195,10 +195,16 @@ class Duration(Converter): """ Converts a `duration` string to a datetime object that's `duration` in the future. - The converter supports years (symbols: `years`, `year, `Y`, `y`), months (`months`, `month`, - `m`), weeks (`weeks`, `week`, `W`, `w`), days (`days`, `day`, `D`, `d`), hours (`hours`, - `hour`, `H`, `h`), minutes (`minutes`, `minute`, `M`), and seconds (`seconds`, `second`, - `S`, `s`), The units must be provided in descending order of magnitude. + The converter supports the following symbols for each unit of time: + - years: `Y`, `y`, `year`, `years` + - months: `m`, `month`, `months` + - weeks: `w`, `W`, `week`, `weeks` + - days: `d`, `D`, `day`, `days` + - hours: `H`, `h`, `hour`, `hours` + - minutes: `M`, `minute`, `minutes` + - seconds: `S`, `s`, `second`, `seconds` + + The units need to be provided in descending order of magnitude. """ match = self.duration_parser.fullmatch(duration) if not match: -- cgit v1.2.3