From b2c0e354cd78a0566a18d823cfff4b8f9821af20 Mon Sep 17 00:00:00 2001 From: Paolo Lammens Date: Thu, 14 Feb 2019 20:17:42 +0100 Subject: [PATCH 1/3] feat: Add `last` kwarg Added a `last` kwarg to `run_repeating` that specifies when the job should last be run. The checking is done before executing the job, but with a small delay buffer, so that if `last` is an exact multiple of `interval`, the last job is still run, even though the actual time is slightly after the theoretical finish time. --- telegram/ext/jobqueue.py | 36 +++++++++++++++++++++++++++++++++--- tests/test_jobqueue.py | 7 +++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/telegram/ext/jobqueue.py b/telegram/ext/jobqueue.py index 5a04d4ea5d2..046d0bb33c6 100644 --- a/telegram/ext/jobqueue.py +++ b/telegram/ext/jobqueue.py @@ -89,7 +89,7 @@ def _put(self, job, time_spec=None, previous_t=None): Specification of the time for which the job should be scheduled. The precise semantics of this parameter depend on its type (see :func:`telegram.ext.JobQueue.run_repeating` for details). - Defaults to now + ``job.interval``. + Defaults to now + ``job.interval``. # TODO: look into this previous_t (optional): Time at which the job last ran (``None`` if it hasn't run yet). @@ -144,7 +144,7 @@ def run_once(self, callback, when, context=None, name=None): self._put(job, time_spec=when) return job - def run_repeating(self, callback, interval, first=None, context=None, name=None): + def run_repeating(self, callback, interval, first=None, last=None, context=None, name=None): """Creates a new ``Job`` that runs at specified intervals and adds it to the queue. Args: @@ -171,6 +171,11 @@ def run_repeating(self, callback, interval, first=None, context=None, name=None) tomorrow. Defaults to ``interval`` + last (:obj:`int` | :obj:`float` | :obj:`datetime.timedelta` | \ + :obj:`datetime.datetime` | :obj:`datetime.time`, optional): + Time after which the job should stop running. + This parameter will be interpreted depending on its type (same as for ``first``). + If ``None``, the job will run indefinitely. context (:obj:`object`, optional): Additional data needed for the callback function. Can be accessed through ``job.context`` in the callback. Defaults to ``None``. name (:obj:`str`, optional): The name of the new job. Defaults to @@ -189,6 +194,7 @@ def run_repeating(self, callback, interval, first=None, context=None, name=None) job = Job(callback, interval=interval, repeat=True, + finish_time=last, context=context, name=name, job_queue=self) @@ -268,6 +274,9 @@ def tick(self): self._set_next_peek(t) break + delay_buffer = 0.01 # tolerance for last + if job.finish_time is not None and now > job.finish_time + delay_buffer: + job.schedule_removal() # job shouldn't run anymore if job.removed: self.logger.debug('Removing job %s', job.name) continue @@ -288,7 +297,7 @@ def tick(self): if job.repeat and not job.removed: self._put(job, previous_t=t) else: - self.logger.debug('Dropping non-repeating or removed job %s', job.name) + self.logger.debug('Dropping non-repeating, removed, or finished job %s', job.name) def start(self): """Starts the job_queue thread.""" @@ -367,6 +376,10 @@ class Job(object): the job queue. repeat (:obj:`bool`, optional): If this job should be periodically execute its callback function (``True``) or only once (``False``). Defaults to ``True``. + finish_time (:obj:`int` | :obj:`float` | :obj:`datetime.timedelta` | \ + :obj:`datetime.datetime` | :obj:`datetime.time`, optional): + Time after which the job shouldn't run anymore. Only valid if :attr:`repeat` is + ``True``. Defaults to ``None``. context (:obj:`object`, optional): Additional data needed for the callback function. Can be accessed through ``job.context`` in the callback. Defaults to ``None``. name (:obj:`str`, optional): The name of the new job. Defaults to ``callback.__name__``. @@ -383,6 +396,7 @@ def __init__(self, callback, interval=None, repeat=True, + finish_time=None, context=None, days=Days.EVERY_DAY, name=None, @@ -395,8 +409,10 @@ def __init__(self, self._repeat = None self._interval = None + self._finish_time = None self.interval = interval self.repeat = repeat + self.finish_time = finish_time self._days = None self.days = days @@ -480,6 +496,20 @@ def repeat(self, repeat): raise ValueError("'repeat' can not be set to 'True' when no 'interval' is set") self._repeat = repeat + @property + def finish_time(self): + """:obj:`float`: Optional. Time at which the job should stop repeating.""" + return self._finish_time + + @finish_time.setter + def finish_time(self, time_spec): + if time_spec is not None: + if not self.repeat: + raise ValueError("'finish_time' cannot be set if job doesn't repeat") + self._finish_time = to_float_timestamp(time_spec) + else: + self._finish_time = None + @property def days(self): """Tuple[:obj:`int`]: Optional. Defines on which days of the week the job should run.""" diff --git a/tests/test_jobqueue.py b/tests/test_jobqueue.py index 4b5d8d10852..38ea1033f3d 100644 --- a/tests/test_jobqueue.py +++ b/tests/test_jobqueue.py @@ -126,6 +126,13 @@ def test_run_repeating_first_timezone(self, job_queue, timezone): sleep(0.001) assert self.result == 1 + def test_run_repeating_last(self, job_queue): + job_queue.run_repeating(self.job_run_once, 0.05, last=0.1) + sleep(0.12) + assert self.result == 2 + sleep(0.1) + assert self.result == 2 + def test_multiple(self, job_queue): job_queue.run_once(self.job_run_once, 0.01) job_queue.run_once(self.job_run_once, 0.02) From 01cd56e5e9a2ae6a76c3d06a2af18c79e54c6c7c Mon Sep 17 00:00:00 2001 From: Paolo Lammens Date: Sun, 17 Feb 2019 14:26:31 +0100 Subject: [PATCH 2/3] chore: Update AUTHORS.rst --- AUTHORS.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS.rst b/AUTHORS.rst index a72ca99f977..f5a3084419d 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -63,6 +63,7 @@ The following wonderful people contributed directly or indirectly to this projec - `Oleg Sushchenko `_ - `Or Bin `_ - `overquota `_ +- `Paolo Lammens `_ - `Patrick Hofmann `_ - `Paul Larsen `_ - `Pieter Schutz `_ From 76be57a2a6ef48b45fa9f3bbc4909a3cfc11f633 Mon Sep 17 00:00:00 2001 From: Paolo Lammens Date: Tue, 3 Sep 2019 16:47:18 +0200 Subject: [PATCH 3/3] test: enhance tests for `to_timestamp` converters Add parametrisations, extract constants and helpers. --- tests/test_helpers.py | 70 ++++++++++++++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 18 deletions(-) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 2c8290dd7f8..d20675bfd6c 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -1,4 +1,5 @@ #!/usr/bin/env python +# -*- coding: utf-8 -*- # # A library that provides a Python interface to the Telegram Bot API # Copyright (C) 2015-2018 @@ -16,8 +17,8 @@ # # You should have received a copy of the GNU Lesser Public License # along with this program. If not, see [http://www.gnu.org/licenses/]. -import time import datetime as dtm +import time import pytest @@ -29,6 +30,12 @@ from telegram.utils.helpers import _UtcOffsetTimezone, _datetime_to_float_timestamp +# TODO: move time test utils to tests.utils.time +# TODO: unify pytest.approx() calls when using now time + +# constants +DAY_TOTAL_SECONDS = dtm.timedelta(days=1).total_seconds() + # sample time specification values categorised into absolute / delta / time-of-day ABSOLUTE_TIME_SPECS = [dtm.datetime.now(tz=_UtcOffsetTimezone(dtm.timedelta(hours=-7))), dtm.datetime.utcnow()] @@ -38,6 +45,18 @@ RELATIVE_TIME_SPECS = DELTA_TIME_SPECS + TIME_OF_DAY_TIME_SPECS TIME_SPECS = ABSOLUTE_TIME_SPECS + RELATIVE_TIME_SPECS +# (naive UTC datetime, timestamp) input / expected pairs +NAIVE_DATETIME_FLOAT_TIMESTAMP_PAIRS = [ + (dtm.datetime(2019, 11, 11, 0, 26, 16, 10**5), 1573431976.1), + (dtm.datetime(1970, 1, 1), 0.0), + (dtm.datetime(1970, 1, 1, microsecond=1), 1e-6)] + + +def microsecond_precision(x): + """Utility to make equality assertions up to microsecond precision + (when floating point arithmetic means we don't have any guarantee beyond that)""" + return pytest.approx(x, abs=1e-6) + class TestHelpers(object): def test_escape_markdown(self): @@ -46,20 +65,24 @@ def test_escape_markdown(self): assert expected_str == helpers.escape_markdown(test_str) - def test_to_float_timestamp_absolute_naive(self): + @pytest.mark.parametrize(['datetime', 'expected_float_timestamp'], + NAIVE_DATETIME_FLOAT_TIMESTAMP_PAIRS, ids=str) + def test_to_float_timestamp_absolute_naive(self, datetime, expected_float_timestamp): """Conversion from timezone-naive datetime to timestamp. Naive datetimes should be assumed to be in UTC. """ - datetime = dtm.datetime(2019, 11, 11, 0, 26, 16, 10**5) - assert helpers.to_float_timestamp(datetime) == 1573431976.1 + assert helpers.to_float_timestamp(datetime) == expected_float_timestamp - def test_to_float_timestamp_absolute_aware(self, timezone): + @pytest.mark.parametrize('datetime_float_timestamp_pair', + NAIVE_DATETIME_FLOAT_TIMESTAMP_PAIRS, ids=str) + def test_to_float_timestamp_absolute_aware(self, datetime_float_timestamp_pair, timezone): """Conversion from timezone-aware datetime to timestamp""" # we're parametrizing this with two different UTC offsets to exclude the possibility # of an xpass when the test is run in a timezone with the same UTC offset - datetime = dtm.datetime(2019, 11, 11, 0, 26, 16, 10**5, tzinfo=timezone) - assert (helpers.to_float_timestamp(datetime) - == 1573431976.1 - timezone.utcoffset(None).total_seconds()) + datetime, float_timestamp = datetime_float_timestamp_pair + aware_datetime = datetime.replace(tzinfo=timezone) + expected = float_timestamp - timezone.utcoffset(None).total_seconds() + assert helpers.to_float_timestamp(aware_datetime) == microsecond_precision(expected) def test_to_float_timestamp_absolute_no_reference(self): """A reference timestamp is only relevant for relative time specifications""" @@ -71,17 +94,28 @@ def test_to_float_timestamp_delta(self, time_spec): """Conversion from a 'delta' time specification to timestamp""" reference_t = 0 delta = time_spec.total_seconds() if hasattr(time_spec, 'total_seconds') else time_spec - assert helpers.to_float_timestamp(time_spec, reference_t) == reference_t + delta + assert (helpers.to_float_timestamp(time_spec, reference_t) + == microsecond_precision(reference_t + delta)) - def test_to_float_timestamp_time_of_day(self): + @pytest.mark.parametrize('delta', [dtm.timedelta(hours=1), + dtm.timedelta(microseconds=1)], + ids=lambda d: 'delta={}'.format(d)) + def test_to_float_timestamp_time_of_day(self, delta): """Conversion from time-of-day specification to timestamp""" - hour, hour_delta = 12, 1 - ref_t = _datetime_to_float_timestamp(dtm.datetime(1970, 1, 1, hour=hour)) - - # test for a time of day that is still to come, and one in the past - time_future, time_past = dtm.time(hour + hour_delta), dtm.time(hour - hour_delta) - assert helpers.to_float_timestamp(time_future, ref_t) == ref_t + 60 * 60 * hour_delta - assert helpers.to_float_timestamp(time_past, ref_t) == ref_t + 60 * 60 * (24 - hour_delta) + hour = 12 + ref_datetime = dtm.datetime(1970, 1, 1, hour=hour) + ref_t = _datetime_to_float_timestamp(ref_datetime) + # make sure that ref_datetime ± delta falls within the same day + delta = min(delta, dtm.timedelta(hours=24 - hour), dtm.timedelta(hours=hour)) + delta_seconds = delta.total_seconds() + + # test for a time of day that is still to come, and one in the past (same day) + time_future, time_past = (ref_datetime + delta).time(), (ref_datetime - delta).time() + # pytest.approx(..., abs=1e-6): check up to microseconds (allow floating point arithmetic) + assert (helpers.to_float_timestamp(time_future, ref_t) + == microsecond_precision(ref_t + delta_seconds)) + assert (helpers.to_float_timestamp(time_past, ref_t) + == microsecond_precision(ref_t - delta_seconds + DAY_TOTAL_SECONDS)) def test_to_float_timestamp_time_of_day_timezone(self, timezone): """Conversion from timezone-aware time-of-day specification to timestamp""" @@ -95,7 +129,7 @@ def test_to_float_timestamp_time_of_day_timezone(self, timezone): assert helpers.to_float_timestamp(time_of_day, ref_t) == pytest.approx(ref_t) # test that by setting the timezone the timestamp changes accordingly: assert (helpers.to_float_timestamp(time_of_day.replace(tzinfo=timezone), ref_t) - == pytest.approx(ref_t + (-utc_offset.total_seconds() % (24 * 60 * 60)))) + == pytest.approx(ref_t + (-utc_offset.total_seconds() % DAY_TOTAL_SECONDS))) @pytest.mark.parametrize('time_spec', RELATIVE_TIME_SPECS, ids=str) def test_to_float_timestamp_default_reference(self, time_spec):