metomi/isodatetime

Our tests run only on UTC, and fail on other TZ's like NZ

kinow opened this issue ยท 10 comments

kinow commented

Hi,

Looks like our tests were written for UTC only. The following tests fail when running the tests in NZ time zone.

isodatetime/tests/test_00.py::TestSuite::test_timepoint_strftime_strptime FAILED
isodatetime/tests/test_datetimeoper.py::TestDateTimeOperator::test_process_time_point_str_now_0 FAILED
isodatetime/tests/test_datetimeoper.py::TestDateTimeOperator::test_process_time_point_str_ref_0 FAILED
isodatetime/tests/test_datetimeoper.py::TestDateTimeOperator::test_process_time_point_str_x FAILED

Left the test_timepoint running for a few minutes and got no error.

We should handle testing in any time zone. If a test is handling time zones, then these must be parameters to the tests.

Even better, we can have tests running on multiple time zones. That way we would reduce the chances of having issues like #89.

I'm unable to reproduce the failures of

test_datetimeoper.py::TestDateTimeOperator::test_process_time_point_str_now_0
test_datetimeoper.py::TestDateTimeOperator::test_process_time_point_str_ref_0

as both pass even when I set export TZ=UTC-12

kinow commented
test results
(venv) kinow@ranma:~/Development/python/workspace/isodatetime$ python setup.py test
running pytest
WARNING: Testing via this command is deprecated and will be removed in a future version. Users looking for a generic test entry point independent of test runner are encouraged to use tox.
running egg_info
writing metomi_isodatetime.egg-info/PKG-INFO
writing dependency_links to metomi_isodatetime.egg-info/dependency_links.txt
writing entry points to metomi_isodatetime.egg-info/entry_points.txt
writing top-level names to metomi_isodatetime.egg-info/top_level.txt
reading manifest file 'metomi_isodatetime.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'metomi_isodatetime.egg-info/SOURCES.txt'
running build_ext
================================================================ test session starts ================================================================
platform linux -- Python 3.7.6, pytest-5.4.2, py-1.8.1, pluggy-0.13.1 -- /home/kinow/Development/python/workspace/isodatetime/venv/bin/python
cachedir: .pytest_cache
rootdir: /home/kinow/Development/python/workspace/isodatetime, inifile: pytest.ini
plugins: cov-2.9.0, env-0.6.2
collected 55 items                                                                                                                                  

README.md::README.md PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_get_local_time_zone_format PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_get_local_time_zone_no_dst PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_get_local_time_zone_with_dst PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_largest_truncated_property_name PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_smallest_missing_property_name PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_timeduration_dumper PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_timeduration_parser PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_timepoint_dump_format PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_timepoint_dumper PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_timepoint_dumper_after_copy PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_timepoint_dumper_bounds_error_message PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_timepoint_dumper_get_time_zone PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_timepoint_parser PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_timepoint_strftime_strptime FAILED
metomi/isodatetime/tests/test_00.py::TestSuite::test_timepoint_time_zone PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_timerecurrence PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_timerecurrence_alt_calendars PASSED
metomi/isodatetime/tests/test_00.py::TestSuite::test_timerecurrence_parser PASSED
metomi/isodatetime/tests/test_01.py::TestDataModel::test_days_in_year_range SKIPPED
metomi/isodatetime/tests/test_01.py::TestDataModel::test_duration_floordiv PASSED
metomi/isodatetime/tests/test_01.py::TestDataModel::test_duration_in_weeks_floordiv PASSED
metomi/isodatetime/tests/test_01.py::TestDataModel::test_duration_subtract PASSED
metomi/isodatetime/tests/test_01.py::TestDataModel::test_duration_to_weeks PASSED
metomi/isodatetime/tests/test_01.py::TestDataModel::test_timeduration PASSED
metomi/isodatetime/tests/test_01.py::TestDataModel::test_timeduration_add_week PASSED
metomi/isodatetime/tests/test_01.py::TestDataModel::test_timepoint_add_duration PASSED
metomi/isodatetime/tests/test_01.py::TestDataModel::test_timepoint_add_duration_without_minute PASSED
metomi/isodatetime/tests/test_01.py::TestDataModel::test_timepoint_bounds PASSED
metomi/isodatetime/tests/test_01.py::TestDataModel::test_timepoint_conflicting_inputs PASSED
metomi/isodatetime/tests/test_01.py::TestDataModel::test_timepoint_plus_float_time_duration_day_of_month_type PASSED
metomi/isodatetime/tests/test_01.py::TestDataModel::test_timepoint_subtract PASSED
metomi/isodatetime/tests/test_datetimeoper.py::TestDateTimeOperator::test_diff_time_point_strs PASSED
metomi/isodatetime/tests/test_datetimeoper.py::TestDateTimeOperator::test_format_duration_str_x PASSED
metomi/isodatetime/tests/test_datetimeoper.py::TestDateTimeOperator::test_process_time_point_str_calendar PASSED
metomi/isodatetime/tests/test_datetimeoper.py::TestDateTimeOperator::test_process_time_point_str_format PASSED
metomi/isodatetime/tests/test_datetimeoper.py::TestDateTimeOperator::test_process_time_point_str_now_0 PASSED
metomi/isodatetime/tests/test_datetimeoper.py::TestDateTimeOperator::test_process_time_point_str_ref_0 PASSED
metomi/isodatetime/tests/test_datetimeoper.py::TestDateTimeOperator::test_process_time_point_str_ref_1 PASSED
metomi/isodatetime/tests/test_datetimeoper.py::TestDateTimeOperator::test_process_time_point_str_ref_2 PASSED
metomi/isodatetime/tests/test_datetimeoper.py::TestDateTimeOperator::test_process_time_point_str_x FAILED
metomi/isodatetime/tests/test_exceptions.py::test_isodatetime_error PASSED
metomi/isodatetime/tests/test_main.py::TestMain::test_1_bad PASSED
metomi/isodatetime/tests/test_main.py::TestMain::test_1_good PASSED
metomi/isodatetime/tests/test_main.py::TestMain::test_1_null PASSED
metomi/isodatetime/tests/test_main.py::TestMain::test_1_version PASSED
metomi/isodatetime/tests/test_main.py::TestMain::test_2_bad PASSED
metomi/isodatetime/tests/test_main.py::TestMain::test_2_good PASSED
metomi/isodatetime/tests/test_main.py::TestMain::test_3_bad PASSED
metomi/isodatetime/tests/test_main.py::TestMain::test_3_good PASSED
metomi/isodatetime/tests/test_main.py::TestMain::test_4_bad PASSED
metomi/isodatetime/tests/test_main.py::TestMain::test_4_good PASSED
metomi/isodatetime/tests/test_main.py::test_pipe[2000-args0-2000] PASSED
metomi/isodatetime/tests/test_main.py::test_pipe[2000\n2001-args1-8784.0] PASSED
metomi/isodatetime/tests/test_time_point.py::TestTimePointCompat::test_timepoint SKIPPED

===================================================================== FAILURES ======================================================================
____________________________________________________ TestSuite.test_timepoint_strftime_strptime _____________________________________________________

self = <isodatetime.tests.test_00.TestSuite testMethod=test_timepoint_strftime_strptime>

    def test_timepoint_strftime_strptime(self):
        """Test the strftime/strptime for date/time expressions."""
        parser = parsers.TimePointParser()
        parse_tokens = list(parser_spec.STRFTIME_TRANSLATE_INFO.keys())
        parse_tokens.remove("%z")  # Don't test datetime's tz handling.
        parse_tokens.remove("%j")  # Can't have day_of_year as well as month
        format_string = ""
        for i, token in enumerate(parse_tokens):
            format_string += token
            if i % 2 == 0:
                format_string += " "
            if i % 3 == 0:
                format_string += ":"
            if i % 5 == 0:
                format_string += "?foobar"
            if i % 7 == 0:
                format_string += "++("
        strftime_string = format_string
        strptime_strings = [format_string]
        for key in parser_spec.STRPTIME_EXCLUSIVE_GROUP_INFO.keys():
            strptime_strings[-1] = strptime_strings[-1].replace(key, "")
        strptime_strings.append(format_string)
        for values in parser_spec.STRPTIME_EXCLUSIVE_GROUP_INFO.values():
            for value in values:
                strptime_strings[-1] = strptime_strings[-1].replace(value, "")
        ctrl_date = datetime.datetime(2002, 3, 1, 12, 30, 2)
    
        # Test %z dumping.
        for sign in [1, -1]:
            for hour in range(0, 24):
                for minute in range(0, 59):
                    if hour == 0 and minute == 0 and sign == -1:
                        # -0000, same as +0000, but invalid.
                        continue
                    test_date = data.TimePoint(
                        year=ctrl_date.year,
                        month_of_year=ctrl_date.month,
                        day_of_month=ctrl_date.day,
                        hour_of_day=ctrl_date.hour,
                        minute_of_hour=ctrl_date.minute,
                        second_of_minute=ctrl_date.second,
                        time_zone_hour=sign * hour,
                        time_zone_minute=sign * minute
                    )
                    ctrl_string = "-" if sign == -1 else "+"
                    ctrl_string += "%02d%02d" % (hour, minute)
                    self.assertEqual(test_date.strftime("%z"),
                                     ctrl_string,
                                     "%z for " + str(test_date))
    
        test_date = data.TimePoint(
            year=ctrl_date.year,
            month_of_year=ctrl_date.month,
            day_of_month=ctrl_date.day,
            hour_of_day=ctrl_date.hour,
            minute_of_hour=ctrl_date.minute,
            second_of_minute=ctrl_date.second
        )
        test_date.set_time_zone_to_utc()
        for test_date in [test_date, test_date.copy().to_week_date(),
                          test_date.copy().to_ordinal_date()]:
            ctrl_data = ctrl_date.strftime(strftime_string)
            test_data = test_date.strftime(strftime_string)
>           self.assertEqual(test_data, ctrl_data, strftime_string)
E           AssertionError: '01 :?foobar++(2002-03-0112 03:30 1014985802?foobar02 :12:30:02++(2002 ' != '01 :?foobar++(2002-03-0112 03:30 1014939002?foobar02 :12:30:02++(2002 '
E           - 01 :?foobar++(2002-03-0112 03:30 1014985802?foobar02 :12:30:02++(2002 
E           ?                                       ^^^
E           + 01 :?foobar++(2002-03-0112 03:30 1014939002?foobar02 :12:30:02++(2002 
E           ?                                       ^^^
E            : %d :?foobar++(%F%H %m:%M %s?foobar%S :%X++(%Y

metomi/isodatetime/tests/test_00.py:1259: AssertionError
________________________________________________ TestDateTimeOperator.test_process_time_point_str_x _________________________________________________

self = <isodatetime.tests.test_datetimeoper.TestDateTimeOperator testMethod=test_process_time_point_str_x>

    def test_process_time_point_str_x(self):
        """DateTimeOperator.process_time_point_str(...)
    
        Basic parse and dump of a time point string.
        """
        # 2009-02-13T23:31:30Z
        point_str = str(seconds2point(1234567890))
        datetimeoper = idt_dtoper.DateTimeOperator()
        # Unix time
        self.assertEqual(
            '2019-01-11T10:40:15Z',
            datetimeoper.process_time_point_str(
                'Fri 11 Jan 10:40:15 UTC 2019',
>               print_format=datetimeoper.CURRENT_TIME_DUMP_FORMAT_Z))
E       AssertionError: '2019-01-11T10:40:15Z' != '2019-01-10T22:40:15Z'
E       - 2019-01-11T10:40:15Z
E       ?          ^ ^^
E       + 2019-01-10T22:40:15Z
E       ?          ^ ^^

metomi/isodatetime/tests/test_datetimeoper.py:111: AssertionError
============================================================== short test summary info ==============================================================
SKIPPED [1] metomi/isodatetime/tests/test_01.py:277: need --runslow option to run
SKIPPED [1] metomi/isodatetime/tests/test_time_point.py: need --runslow option to run
FAILED metomi/isodatetime/tests/test_00.py::TestSuite::test_timepoint_strftime_strptime - AssertionError: '01 :?foobar++(2002-03-0112 03:30 101498...
FAILED metomi/isodatetime/tests/test_datetimeoper.py::TestDateTimeOperator::test_process_time_point_str_x - AssertionError: '2019-01-11T10:40:15Z'...
====================================================== 2 failed, 51 passed, 2 skipped in 4.40s ======================================================

Running TZ=UTC python setup.py test, everything works.

The tests you mentioned appear to work on my environment too. Only TestDateTimeOperator.test_process_time_point_str_x and TestSuite.test_timepoint_strftime_strptime appear to have problems with TZ now.

TZ="America/Sao_Paulo" python setup.py test (`AssertionError: '2019-01-11T10:40:15Z' != '2019-01-11T13:40:15Z'`)
TZ="Asia/Colombo" python setup.py test (`AssertionError: '2019-01-11T10:40:15Z' != '2019-01-11T05:10:15Z'`)
TZ="Asia/Tokyo" python setup.py test (`AssertionError: '2019-01-11T10:40:15Z' != '2019-01-11T01:40:15Z'`)
TZ="Pacific/Auckland" python setup.py test (`AssertionError: '2019-01-11T10:40:15Z' != '2019-01-10T22:40:15Z'`)
TZ="NZST-12NZDT,M9.5.0,M4.1.0/3" python setup.py test (`AssertionError: '2019-01-11T10:40:15Z' != '2019-01-10T22:40:15Z'`)
TZ=UTC-12 python setup.py test (`AssertionError: '2019-01-11T10:40:15Z' != '2019-01-10T22:40:15Z'`)

(the assertion error is for TestDateTimeOperator.test_process_time_point_str_x)

Maybe these tests need to have the env TZ enforced to UTC (assuming it was intentional). That could make the tests pass, regardless of the system time zone.

Sor far I have come up with a fix for the strftime / strptime test by explicitly setting the time zones of the test and control dates in it to UTC instead of having unknown time zones which seem to be prone to becoming the local time zone in various method calls.

The problem with TestDateTimeOperator.test_process_time_point_str_x lies here:

def get_datetime_strptime(self, time_point_str, parse_format):
"""Use the datetime library's strptime as a fallback."""
point = seconds2point(
time.mktime(time.strptime(time_point_str, parse_format)))
# FIXME: Neither time.strptime nor datetime.datetime.strptime
# returns the value of the %Z (time zone) field, so we ignore that for
# now except explicit UTC/GMT in the string.
if any(s in time_point_str for s in ('UTC', 'GMT')):
point.set_time_zone_to_utc()
return point

Forgetting the problem for a moment, can I draw attention to the FIXME comment. It seems bad practice to ignore any non GMT or UTC time zone string. Actually in fact the time.strptime() method will raise

ValueError: time data 'Fri 11 Jan 10:40:15 ET 2019' does not
    match format '%a %d %b %H:%M:%S %Z %Y'

for any time zone string that isn't UTC or GMT anyway.

So I propose we raise an error that is more descriptive, e.g. "only UTC or GMT are allowed"

Ah wait, unfortunately there is no way to differentiate between an unrecognised zone (i.e. anything that isn't UTC/GMT) and a bad time string (e.g. Fri 11 Poo 10:40:15 UTC 2019)

For running tests on a different time zone, I take it we would have to add a strategy option to the Travis test matrix

isodatetime/.travis.yml

Lines 8 to 22 in 5a380a1

matrix:
fast_finish: true
include:
- python: 3.5
env: RUN_COVERAGE=false
- python: 3.6
env: RUN_COVERAGE=false
- python: 3.7
env: RUN_COVERAGE=true
- python: 3.7
env: RUN_COVERAGE=false TZ=UTC
- python: 3.8-dev
env: RUN_COVERAGE=false
- os: osx
env: RUN_COVERAGE=false

Possibly just change the TZ=UTC to TZ=UTC-5 or something on line 18? It seems the others are running on UTC anyway?

kinow commented

Ah wait, unfortunately there is no way to differentiate between an unrecognised zone (i.e. anything that isn't UTC/GMT) and a bad time string (e.g. Fri 11 Poo 10:40:15 UTC 2019)

Maybe that could be done after Py3.9 if PEP-615 provides the list of valid timezones - https://www.python.org/dev/peps/pep-0615/

For running tests on a different time zone, I take it we would have to add a strategy option to the Travis test matrix
Possibly just change the TZ=UTC to TZ=UTC-5 or something on line 18? It seems the others are running on UTC anyway?

+1 looks good to me

If we want to cover really all the cases for time zones, I think we would need:

  • UTC
  • Positive timezone offset without minutes (e.g. Pacific/Auckland, UTC+12)
  • Positive timezone offset with minutes (e.g. Asia/Colombo, UTC+5:30)
  • Negative timezone offset without minutes (e.g. America/Buenos_Aires, UTC-3)
  • Negative timezone offset with minutes (e.g. America/St_Johns, UTC-3:30)

That would prevent issues for users running somewhere like Newfoundland Canada with that America/St_Johns timezone, e.g. #93

(or we can start with one random TZ, and add more later)

If we want to cover really all the cases for time zones...

In my opinion that's excessive; the tests already take > 1 hour to run on Travis. Probably just having one time zone with minutes offset (e.g. export TZ=XXX-05:30) as the base case is enough. Or one ahead & one behind UTC, both with minutes offset, at most.

(Note for some reason in the TZ env variable the sign is backwards, i.e. -05:30 is ahead of UTC and +05:30 is behind. Also the (3 or more) letters at the beginning are irrelevant, that's just the name you're giving to the time zone)

https://www.gnu.org/software/libc/manual/html_node/TZ-Variable.html

Note for some reason in the TZ env variable the sign is backwards, i.e. -05:30 is ahead of UTC and +05:30 is behind.

From the tz database:

# We use POSIX-style signs in the Zone names and the output abbreviations,
# even though this is the opposite of what many people expect.
# POSIX has positive signs west of Greenwich, but many people expect
# positive signs east of Greenwich.  For example, TZ='Etc/GMT+4' uses
# the abbreviation "GMT+4" and corresponds to 4 hours behind UT
# (i.e. west of Greenwich) even though many people would expect it to
# mean 4 hours ahead of UT (i.e. east of Greenwich).