From 926ce58892cb456aae73c7a0483f4a99fea6bcd3 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 16 May 2024 17:38:01 +0100 Subject: [PATCH 01/11] :bug: Ensure that next_occurrence only adds one day when necessary, to avoid it choosing a datetime more than 1 day in the future --- data_safe_haven/functions/strings.py | 6 +++++- tests/functions/test_strings.py | 5 +++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/data_safe_haven/functions/strings.py b/data_safe_haven/functions/strings.py index 96fc76a54a..e26fe456f3 100644 --- a/data_safe_haven/functions/strings.py +++ b/data_safe_haven/functions/strings.py @@ -46,8 +46,12 @@ def next_occurrence( minute=minute, second=0, microsecond=0, - ) + datetime.timedelta(days=1) + ) utc_dt = local_dt.astimezone(pytz.utc) + # Add one day until this datetime is in the future + utc_now = datetime.datetime.now(pytz.utc) + datetime.timedelta(minutes=1) + while utc_dt < utc_now: + utc_dt += datetime.timedelta(days=1) if time_format == "iso": return utc_dt.isoformat() elif time_format == "iso_minute": diff --git a/tests/functions/test_strings.py b/tests/functions/test_strings.py index dcca11c1ba..8fa40788dc 100644 --- a/tests/functions/test_strings.py +++ b/tests/functions/test_strings.py @@ -9,11 +9,12 @@ def test_next_occurrence_is_within_next_day(): + dt_utc_now = datetime.datetime.now(datetime.UTC) next_time = next_occurrence(5, 13, "Australia/Perth") + dt_utc_tomorrow = dt_utc_now + datetime.timedelta(days=1, minutes=5) dt_next_time = datetime.datetime.fromisoformat(next_time) - dt_utc_now = datetime.datetime.now(datetime.UTC) assert dt_next_time > dt_utc_now - assert dt_next_time < dt_utc_now + datetime.timedelta(days=1) + assert dt_next_time < dt_utc_tomorrow def test_next_occurrence_has_correct_time(): From 936341f68cd69f02bef40a324590e9428ce197ea Mon Sep 17 00:00:00 2001 From: James Robinson Date: Fri, 17 May 2024 12:59:14 +0100 Subject: [PATCH 02/11] :bulb: Added explanatory comment for next_occurrence logic --- data_safe_haven/functions/strings.py | 8 +++++--- tests/functions/test_strings.py | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/data_safe_haven/functions/strings.py b/data_safe_haven/functions/strings.py index e26fe456f3..d6589c32e0 100644 --- a/data_safe_haven/functions/strings.py +++ b/data_safe_haven/functions/strings.py @@ -48,9 +48,11 @@ def next_occurrence( microsecond=0, ) utc_dt = local_dt.astimezone(pytz.utc) - # Add one day until this datetime is in the future - utc_now = datetime.datetime.now(pytz.utc) + datetime.timedelta(minutes=1) - while utc_dt < utc_now: + # Add one day until this datetime is at least 1 hour in the future. + # This ensures that any Azure functions which depend on this datetime being in + # the future should treat it as valid. + utc_near_future = datetime.datetime.now(pytz.utc) + datetime.timedelta(hours=1) + while utc_dt < utc_near_future: utc_dt += datetime.timedelta(days=1) if time_format == "iso": return utc_dt.isoformat() diff --git a/tests/functions/test_strings.py b/tests/functions/test_strings.py index 8fa40788dc..98585b39fe 100644 --- a/tests/functions/test_strings.py +++ b/tests/functions/test_strings.py @@ -8,10 +8,10 @@ from data_safe_haven.functions import next_occurrence, sanitise_sre_name -def test_next_occurrence_is_within_next_day(): +def test_next_occurrence_will_occur_soon(): dt_utc_now = datetime.datetime.now(datetime.UTC) next_time = next_occurrence(5, 13, "Australia/Perth") - dt_utc_tomorrow = dt_utc_now + datetime.timedelta(days=1, minutes=5) + dt_utc_tomorrow = dt_utc_now + datetime.timedelta(days=1, hours=2) dt_next_time = datetime.datetime.fromisoformat(next_time) assert dt_next_time > dt_utc_now assert dt_next_time < dt_utc_tomorrow From 1eb9293aa037db0fa9f7cc5158d6b0a2906e30f1 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Fri, 17 May 2024 13:06:44 +0100 Subject: [PATCH 03/11] :white_check_mark: Add test that will always trigger the +1day logic --- tests/functions/test_strings.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/functions/test_strings.py b/tests/functions/test_strings.py index 98585b39fe..4e39249617 100644 --- a/tests/functions/test_strings.py +++ b/tests/functions/test_strings.py @@ -17,6 +17,18 @@ def test_next_occurrence_will_occur_soon(): assert dt_next_time < dt_utc_tomorrow +def test_next_occurrence_day_shift(): + dt_utc_now = datetime.datetime.now(datetime.UTC) + dt_perth_now = dt_utc_now.astimezone(pytz.timezone("Australia/Perth")) + next_time = next_occurrence( + dt_perth_now.hour, dt_perth_now.minute, "Australia/Perth" + ) + dt_utc_tomorrow = dt_utc_now + datetime.timedelta(days=1, hours=2) + dt_next_time = datetime.datetime.fromisoformat(next_time) + assert dt_next_time > dt_utc_now + assert dt_next_time < dt_utc_tomorrow + + def test_next_occurrence_has_correct_time(): next_time = next_occurrence(5, 13, "Australia/Perth") dt_as_utc = datetime.datetime.fromisoformat(next_time) From d227f3d267d4b4dd1bb20df9f411b0d53caa90d8 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 21 May 2024 14:51:15 +0100 Subject: [PATCH 04/11] :art: Better variable name --- tests/functions/test_strings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functions/test_strings.py b/tests/functions/test_strings.py index 4e39249617..800ba38eda 100644 --- a/tests/functions/test_strings.py +++ b/tests/functions/test_strings.py @@ -11,10 +11,10 @@ def test_next_occurrence_will_occur_soon(): dt_utc_now = datetime.datetime.now(datetime.UTC) next_time = next_occurrence(5, 13, "Australia/Perth") - dt_utc_tomorrow = dt_utc_now + datetime.timedelta(days=1, hours=2) + dt_utc_26_hours_time = dt_utc_now + datetime.timedelta(hours=26) dt_next_time = datetime.datetime.fromisoformat(next_time) assert dt_next_time > dt_utc_now - assert dt_next_time < dt_utc_tomorrow + assert dt_next_time < dt_utc_26_hours_time def test_next_occurrence_day_shift(): From 18c45c07d8f1db07ad62b3ba3466bdcdb6cefeb6 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 21 May 2024 16:30:24 +0100 Subject: [PATCH 05/11] :arrow_up: Add freezegun as a dependency for tests --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index d4f74a7801..6c05f8ed26 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -114,6 +114,7 @@ all = [ [tool.hatch.envs.test] dependencies = [ "coverage>=7.5.1", + "freezegun>=1.5", "pytest>=8.1", "pytest-mock>=3.14", ] From 09af7930ce20ae1aaecb254875b65aab967067e5 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 21 May 2024 16:30:47 +0100 Subject: [PATCH 06/11] :white_check_mark: Use frozen times to simplify next_occurrence tests --- tests/functions/test_strings.py | 39 ++++++++++----------------------- 1 file changed, 11 insertions(+), 28 deletions(-) diff --git a/tests/functions/test_strings.py b/tests/functions/test_strings.py index 800ba38eda..cdea96d63d 100644 --- a/tests/functions/test_strings.py +++ b/tests/functions/test_strings.py @@ -1,45 +1,28 @@ import datetime -import re import pytest -import pytz +from freezegun import freeze_time from data_safe_haven.exceptions import DataSafeHavenInputError from data_safe_haven.functions import next_occurrence, sanitise_sre_name -def test_next_occurrence_will_occur_soon(): - dt_utc_now = datetime.datetime.now(datetime.UTC) +@freeze_time(datetime.datetime(2024, 1, 2, 1, 0, tzinfo=datetime.timezone.utc)) +def test_next_occurrence(): next_time = next_occurrence(5, 13, "Australia/Perth") - dt_utc_26_hours_time = dt_utc_now + datetime.timedelta(hours=26) - dt_next_time = datetime.datetime.fromisoformat(next_time) - assert dt_next_time > dt_utc_now - assert dt_next_time < dt_utc_26_hours_time + assert next_time == "2024-01-02T21:13:00+00:00" -def test_next_occurrence_day_shift(): - dt_utc_now = datetime.datetime.now(datetime.UTC) - dt_perth_now = dt_utc_now.astimezone(pytz.timezone("Australia/Perth")) - next_time = next_occurrence( - dt_perth_now.hour, dt_perth_now.minute, "Australia/Perth" - ) - dt_utc_tomorrow = dt_utc_now + datetime.timedelta(days=1, hours=2) - dt_next_time = datetime.datetime.fromisoformat(next_time) - assert dt_next_time > dt_utc_now - assert dt_next_time < dt_utc_tomorrow +@freeze_time(datetime.datetime(2024, 1, 2, 1, 0, tzinfo=datetime.timezone.utc)) +def test_next_occurrence_timeformat(): + next_time = next_occurrence(5, 13, "Australia/Perth", time_format="iso_minute") + assert next_time == "2024-01-02 21:13" -def test_next_occurrence_has_correct_time(): +@freeze_time(datetime.datetime(2024, 1, 2, 23, 0, tzinfo=datetime.timezone.utc)) +def test_next_occurrence_is_tomorrow(): next_time = next_occurrence(5, 13, "Australia/Perth") - dt_as_utc = datetime.datetime.fromisoformat(next_time) - dt_as_local = dt_as_utc.astimezone(pytz.timezone("Australia/Perth")) - assert dt_as_local.hour == 5 - assert dt_as_local.minute == 13 - - -def test_next_occurrence_timeformat(): - next_time = next_occurrence(5, 13, "Australia/Perth", time_format="iso_minute") - assert re.match(r"\d\d\d\d-\d\d-\d\d \d\d:13", next_time) + assert next_time == "2024-01-03T21:13:00+00:00" def test_next_occurrence_invalid_hour(): From 8aea0374abd9f06d4c124ab6c8ae4bd08923f01f Mon Sep 17 00:00:00 2001 From: James Robinson Date: Tue, 21 May 2024 17:36:54 +0100 Subject: [PATCH 07/11] :rotating_light: Switch to more compact datetime.UTC --- tests/functions/test_strings.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/functions/test_strings.py b/tests/functions/test_strings.py index cdea96d63d..31f3b8bf3a 100644 --- a/tests/functions/test_strings.py +++ b/tests/functions/test_strings.py @@ -7,19 +7,19 @@ from data_safe_haven.functions import next_occurrence, sanitise_sre_name -@freeze_time(datetime.datetime(2024, 1, 2, 1, 0, tzinfo=datetime.timezone.utc)) +@freeze_time(datetime.datetime(2024, 1, 2, 1, 0, tzinfo=datetime.UTC)) def test_next_occurrence(): next_time = next_occurrence(5, 13, "Australia/Perth") assert next_time == "2024-01-02T21:13:00+00:00" -@freeze_time(datetime.datetime(2024, 1, 2, 1, 0, tzinfo=datetime.timezone.utc)) +@freeze_time(datetime.datetime(2024, 1, 2, 1, 0, tzinfo=datetime.UTC)) def test_next_occurrence_timeformat(): next_time = next_occurrence(5, 13, "Australia/Perth", time_format="iso_minute") assert next_time == "2024-01-02 21:13" -@freeze_time(datetime.datetime(2024, 1, 2, 23, 0, tzinfo=datetime.timezone.utc)) +@freeze_time(datetime.datetime(2024, 1, 2, 23, 0, tzinfo=datetime.UTC)) def test_next_occurrence_is_tomorrow(): next_time = next_occurrence(5, 13, "Australia/Perth") assert next_time == "2024-01-03T21:13:00+00:00" From 3a03e33bbcdce6f37428d52d088beeba5c52483c Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Wed, 22 May 2024 12:15:24 +0100 Subject: [PATCH 08/11] Parametrise next occurrence tests and add dst test --- tests/functions/test_strings.py | 86 ++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 40 deletions(-) diff --git a/tests/functions/test_strings.py b/tests/functions/test_strings.py index 31f3b8bf3a..b050a1b5cd 100644 --- a/tests/functions/test_strings.py +++ b/tests/functions/test_strings.py @@ -7,46 +7,52 @@ from data_safe_haven.functions import next_occurrence, sanitise_sre_name -@freeze_time(datetime.datetime(2024, 1, 2, 1, 0, tzinfo=datetime.UTC)) -def test_next_occurrence(): - next_time = next_occurrence(5, 13, "Australia/Perth") - assert next_time == "2024-01-02T21:13:00+00:00" - - -@freeze_time(datetime.datetime(2024, 1, 2, 1, 0, tzinfo=datetime.UTC)) -def test_next_occurrence_timeformat(): - next_time = next_occurrence(5, 13, "Australia/Perth", time_format="iso_minute") - assert next_time == "2024-01-02 21:13" - - -@freeze_time(datetime.datetime(2024, 1, 2, 23, 0, tzinfo=datetime.UTC)) -def test_next_occurrence_is_tomorrow(): - next_time = next_occurrence(5, 13, "Australia/Perth") - assert next_time == "2024-01-03T21:13:00+00:00" - - -def test_next_occurrence_invalid_hour(): - with pytest.raises(DataSafeHavenInputError) as exc_info: - next_occurrence(99, 13, "Europe/London") - assert exc_info.match(r"Time '99:13' was not recognised.") - - -def test_next_occurrence_invalid_minute(): - with pytest.raises(DataSafeHavenInputError) as exc_info: - next_occurrence(5, 99, "Europe/London") - assert exc_info.match(r"Time '5:99' was not recognised.") - - -def test_next_occurrence_invalid_timezone(): - with pytest.raises(DataSafeHavenInputError) as exc_info: - next_occurrence(5, 13, "Mars/OlympusMons") - assert exc_info.match(r"Timezone 'Mars/OlympusMons' was not recognised.") - - -def test_next_occurrence_invalid_timeformat(): - with pytest.raises(DataSafeHavenInputError) as exc_info: - next_occurrence(5, 13, "Australia/Perth", time_format="invalid") - assert exc_info.match(r"Time format 'invalid' was not recognised.") +class TestNextOccurance: + @pytest.mark.parametrize("hour,minute,timezone,expected", [ + (5, 13, "Australia/Perth", "2024-01-02T21:13:00+00:00"), + (0, 13, "Australia/Perth", "2024-01-02T16:13:00+00:00"), + (20, 13, "Australia/Perth", "2024-01-02T12:13:00+00:00"), + (20, 13, "Europe/London", "2024-01-02T20:13:00+00:00"), + ]) + @freeze_time(datetime.datetime(2024, 1, 2, 1, 0, tzinfo=datetime.UTC)) + def test_next_occurrence(self, hour, minute, timezone, expected): + next_time = next_occurrence(hour, minute, timezone) + assert next_time == expected + + @freeze_time(datetime.datetime(2024, 7, 2, 1, 0, tzinfo=datetime.UTC)) + def test_dst(self): + next_time = next_occurrence(13, 5, "Europe/London") + assert next_time == "2024-07-02T12:05:00+00:00" + + @freeze_time(datetime.datetime(2024, 1, 2, 1, 0, tzinfo=datetime.UTC)) + def test_timeformat(self): + next_time = next_occurrence(5, 13, "Australia/Perth", time_format="iso_minute") + assert next_time == "2024-01-02 21:13" + + @freeze_time(datetime.datetime(2024, 1, 2, 23, 0, tzinfo=datetime.UTC)) + def test_is_tomorrow(self): + next_time = next_occurrence(5, 13, "Australia/Perth") + assert next_time == "2024-01-03T21:13:00+00:00" + + def test_invalid_hour(self): + with pytest.raises(DataSafeHavenInputError) as exc_info: + next_occurrence(99, 13, "Europe/London") + assert exc_info.match(r"Time '99:13' was not recognised.") + + def test_invalid_minute(self): + with pytest.raises(DataSafeHavenInputError) as exc_info: + next_occurrence(5, 99, "Europe/London") + assert exc_info.match(r"Time '5:99' was not recognised.") + + def test_invalid_timezone(self): + with pytest.raises(DataSafeHavenInputError) as exc_info: + next_occurrence(5, 13, "Mars/OlympusMons") + assert exc_info.match(r"Timezone 'Mars/OlympusMons' was not recognised.") + + def test_invalid_timeformat(self): + with pytest.raises(DataSafeHavenInputError) as exc_info: + next_occurrence(5, 13, "Australia/Perth", time_format="invalid") + assert exc_info.match(r"Time format 'invalid' was not recognised.") @pytest.mark.parametrize( From 75ae6d5dfff376e46a4c9b2a6321d010a4d9e971 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Wed, 22 May 2024 12:20:16 +0100 Subject: [PATCH 09/11] Fix linting --- tests/functions/test_strings.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/functions/test_strings.py b/tests/functions/test_strings.py index b050a1b5cd..86bfc176fb 100644 --- a/tests/functions/test_strings.py +++ b/tests/functions/test_strings.py @@ -8,12 +8,15 @@ class TestNextOccurance: - @pytest.mark.parametrize("hour,minute,timezone,expected", [ - (5, 13, "Australia/Perth", "2024-01-02T21:13:00+00:00"), - (0, 13, "Australia/Perth", "2024-01-02T16:13:00+00:00"), - (20, 13, "Australia/Perth", "2024-01-02T12:13:00+00:00"), - (20, 13, "Europe/London", "2024-01-02T20:13:00+00:00"), - ]) + @pytest.mark.parametrize( + "hour,minute,timezone,expected", + [ + (5, 13, "Australia/Perth", "2024-01-02T21:13:00+00:00"), + (0, 13, "Australia/Perth", "2024-01-02T16:13:00+00:00"), + (20, 13, "Australia/Perth", "2024-01-02T12:13:00+00:00"), + (20, 13, "Europe/London", "2024-01-02T20:13:00+00:00"), + ], + ) @freeze_time(datetime.datetime(2024, 1, 2, 1, 0, tzinfo=datetime.UTC)) def test_next_occurrence(self, hour, minute, timezone, expected): next_time = next_occurrence(hour, minute, timezone) From ecbd33f822c8d5ab7ccf5242a5857d2ff11fa62c Mon Sep 17 00:00:00 2001 From: James Robinson Date: Wed, 22 May 2024 13:24:58 +0100 Subject: [PATCH 10/11] :wrench: Use human-readable strings for freeze_time --- tests/functions/test_strings.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/functions/test_strings.py b/tests/functions/test_strings.py index 86bfc176fb..762c3d6718 100644 --- a/tests/functions/test_strings.py +++ b/tests/functions/test_strings.py @@ -17,22 +17,22 @@ class TestNextOccurance: (20, 13, "Europe/London", "2024-01-02T20:13:00+00:00"), ], ) - @freeze_time(datetime.datetime(2024, 1, 2, 1, 0, tzinfo=datetime.UTC)) + @freeze_time("1am on Jan 2nd, 2024") def test_next_occurrence(self, hour, minute, timezone, expected): next_time = next_occurrence(hour, minute, timezone) assert next_time == expected - @freeze_time(datetime.datetime(2024, 7, 2, 1, 0, tzinfo=datetime.UTC)) + @freeze_time("1am on July 2nd, 2024") def test_dst(self): next_time = next_occurrence(13, 5, "Europe/London") assert next_time == "2024-07-02T12:05:00+00:00" - @freeze_time(datetime.datetime(2024, 1, 2, 1, 0, tzinfo=datetime.UTC)) + @freeze_time("1am on Jan 2nd, 2024") def test_timeformat(self): next_time = next_occurrence(5, 13, "Australia/Perth", time_format="iso_minute") assert next_time == "2024-01-02 21:13" - @freeze_time(datetime.datetime(2024, 1, 2, 23, 0, tzinfo=datetime.UTC)) + @freeze_time("9pm on Jan 2nd, 2024") def test_is_tomorrow(self): next_time = next_occurrence(5, 13, "Australia/Perth") assert next_time == "2024-01-03T21:13:00+00:00" From 2424422aa33c5e77ddfe5c7cffa9b63854411c66 Mon Sep 17 00:00:00 2001 From: Jim Madge Date: Wed, 22 May 2024 14:11:41 +0100 Subject: [PATCH 11/11] Remove unused import --- tests/functions/test_strings.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/functions/test_strings.py b/tests/functions/test_strings.py index 762c3d6718..051faa3984 100644 --- a/tests/functions/test_strings.py +++ b/tests/functions/test_strings.py @@ -1,5 +1,3 @@ -import datetime - import pytest from freezegun import freeze_time