Skip to content

Commit

Permalink
Fix negative-DST bug
Browse files Browse the repository at this point in the history
  • Loading branch information
Zac-HD committed Nov 16, 2021
1 parent e3480a3 commit 922973b
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 34 deletions.
7 changes: 7 additions & 0 deletions hypothesis-python/RELEASE.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
RELEASE_TYPE: patch

This patch fixes a rare internal error in the :func:`hypothesis.strategies.datetimes`
strategy, where the implementation of ``allow_imaginary=False`` crashed when checking
a time during the skipped hour of a DST transition *if* the DST offset is negative -
only true of ``Europe/Dublin``, who we presume have their reasons - and the ``tzinfo``
object is a :pypi:`pytz` timezone (which predates :pep:`495`).
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,13 @@ def datetime_does_not_exist(value):
# meaningless before ~1900 and subject to a lot of change by
# 9999, so it should be a very small fraction of possible values.
return True

if value.tzinfo.utcoffset != roundtrip.tzinfo.utcoffset:
# This only ever occurs during imaginary (i.e. nonexistent) datetimes,
# and only for pytz timezones which do not follow PEP-495 semantics.
# (may exclude a few other edge cases, but you should use zoneinfo anyway)
return True

assert value.tzinfo is roundtrip.tzinfo, "so only the naive portions are compared"
return value != roundtrip

Expand Down Expand Up @@ -196,9 +203,6 @@ def datetimes(
which did not (or will not) occur due to daylight savings, leap seconds,
timezone and calendar adjustments, etc. Imaginary datetimes are allowed
by default, because malformed timestamps are a common source of bugs.
Note that because :pypi:`pytz` predates :pep:`495`, this does not work
correctly with timezones that use a negative DST offset (such as
``"Europe/Dublin"``).
Examples from this strategy shrink towards midnight on January 1st 2000,
local time.
Expand Down
64 changes: 33 additions & 31 deletions hypothesis-python/tests/datetime/test_pytz_timezones.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from hypothesis.errors import InvalidArgument
from hypothesis.extra.pytz import timezones
from hypothesis.strategies import data, datetimes, just, sampled_from, times
from hypothesis.strategies._internal.datetime import datetime_does_not_exist

from tests.common.debug import (
assert_all_examples,
Expand Down Expand Up @@ -124,36 +125,37 @@ def test_datetimes_stay_within_naive_bounds(data, lo, hi):
assert lo <= out.replace(tzinfo=None) <= hi


@pytest.mark.xfail(reason="is_dst not equivalent to fold when DST offset is negative")
def test_datetimes_can_exclude_imaginary():
# The day of a spring-forward transition; 2am is imaginary
australia = {
"min_value": dt.datetime(2020, 10, 4),
"max_value": dt.datetime(2020, 10, 5),
"timezones": just(pytz.timezone("Australia/Sydney")),
}
# Ireland uses *negative* offset DST, which means that our sloppy interpretation
# of "is_dst=not fold" bypasses the filter for imaginary times. This is basically
# unfixable without redesigning pytz per PEP-495, and it's much more likely to be
# replaced by dateutil or PEP-615 zoneinfo in the standard library instead.
# (we use both so an optimistic `is_dst=bool(fold)` also fails the test)
ireland = {
"min_value": dt.datetime(2019, 3, 31),
"max_value": dt.datetime(2019, 4, 1),
"timezones": just(pytz.timezone("Europe/Dublin")),
}
@pytest.mark.parametrize(
"kw",
[
# Ireland uses *negative* offset DST, which means that our sloppy interpretation
# of "is_dst=not fold" bypasses the filter for imaginary times. This is basically
# unfixable without redesigning pytz per PEP-495, and it's much more likely to be
# replaced by dateutil or PEP-615 zoneinfo in the standard library instead.
{
"min_value": dt.datetime(2019, 3, 31),
"max_value": dt.datetime(2019, 4, 1),
"timezones": just(pytz.timezone("Europe/Dublin")),
},
# The day of a spring-forward transition in Australia; 2am is imaginary
# (the common case so an optimistic `is_dst=bool(fold)` also fails the test)
{
"min_value": dt.datetime(2020, 10, 4),
"max_value": dt.datetime(2020, 10, 5),
"timezones": just(pytz.timezone("Australia/Sydney")),
},
],
)
def test_datetimes_can_exclude_imaginary(kw):
# Sanity check: fail unless those days contain an imaginary hour to filter out
find_any(
datetimes(**australia, allow_imaginary=True),
lambda x: not datetime_exists(x),
)
find_any(
datetimes(**ireland, allow_imaginary=True),
lambda x: not datetime_exists(x),
)
find_any(datetimes(**kw, allow_imaginary=True), lambda x: not datetime_exists(x))

# Assert that with allow_imaginary=False we only generate existing datetimes.
assert_all_examples(
datetimes(**australia, allow_imaginary=False)
| datetimes(**ireland, allow_imaginary=False),
datetime_exists,
)
assert_all_examples(datetimes(**kw, allow_imaginary=False), datetime_exists)


def test_really_weird_tzinfo_case():
x = dt.datetime(2019, 3, 31, 2, 30, tzinfo=pytz.timezone("Europe/Dublin"))
assert x.tzinfo is not x.astimezone(dt.timezone.utc).astimezone(x.tzinfo)
# And that weird case exercises the rare branch in our helper:
assert datetime_does_not_exist(x)

0 comments on commit 922973b

Please sign in to comment.