Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_totals_with_time_delta_larger_than_24_hours fails after daylight saving time change #568

Open
sstock opened this issue Nov 6, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@sstock
Copy link

sstock commented Nov 6, 2023

Steps taken:

Expected all tests to pass, but one did not: test_totals_with_time_delta_larger_than_24_hours failed because the start time actual was one hour off the expected time.

$ make test
...
not ok 17 - test_totals.t: totals extension should print report for time delta larger than 24 hours
# FAIL: AssertionError on file /home/sstock/src-remote/timew-1.6.0/test/test_totals.t line 110 in test_totals_with_time_delta_larger_than_24_hours: 'self.assertEqual(':
#       Lists differ: ['', [26 chars]-04 07:53:03 - 2023-11-06 07:53:03', '', 'Tag [93 chars], ''] != ['', [26 chars]-04 08:53:03 - 2023-11-06 07:53:03', '', 'Tag [93 chars], '']
#       
#       First differing element 1:
#       'Total by Tag, for 2023-11-04 07:53:03 - 2023-11-06 07:53:03'
#       'Total by Tag, for 2023-11-04 08:53:03 - 2023-11-06 07:53:03'
#       
#         ['',
#       -  'Total by Tag, for 2023-11-04 07:53:03 - 2023-11-06 07:53:03',
#       ?                                 ^
#       
#       +  'Total by Tag, for 2023-11-04 08:53:03 - 2023-11-06 07:53:03',
#       ?                                 ^
#       
#          '',
#          'Tag        Total',
#          '----- ----------',
#          'foo     48:00:00',
#          '      ----------',
#          'Total   48:00:00',
#          '']
...
Passed:                          1457
Failed:                             1
Unexpected successes:               0
Skipped:                            7
Expected failures:                  0
Runtime:                         2.69 seconds
make[3]: *** [test/CMakeFiles/test.dir/build.make:82: test/CMakeFiles/test] Error 1
make[2]: *** [CMakeFiles/Makefile2:517: test/CMakeFiles/test.dir/all] Error 2
make[1]: *** [CMakeFiles/Makefile2:524: test/CMakeFiles/test.dir/rule] Error 2
make: *** [Makefile:306: test] Error 2

The significant aspect here is daylight saving time ended less than 48 hours ago. The test uses two times, now and 48 hours prior to now. The input times both use UTC. However the output (expected) values use the local time zone (in my case US/Eastern). I expect if I wait approximately 18 more hours the test will pass (update: 24 hours later and the test now passes when using my local time zone).

Forcing my local time zone to UTC, or any other that didn't recently change DST (e.g. Europe/Prague), allows the test to pass:

$ TZ=UTC make test
...
Passed:                          1458
Failed:                             0
Unexpected successes:               0
Skipped:                            7
Expected failures:                  0
Runtime:                         2.70 seconds
[100%] Built target test

Note: relates to #255.

Miscellaneous details:
OS: Ubuntu 22.04.3 LTS
Python: Python 3.10.12

@lauft
Copy link
Member

lauft commented Nov 7, 2023

Apart from not running the test in the proximity of DST change, a solution would be modify the test such as it either takes DST into account, or its assertions are not disturbed by its effects... 🤔

@sstock
Copy link
Author

sstock commented Nov 7, 2023

Call it bad timing, but this was the first time I had ever built timew and the test phase failed. I managed to hit the 1% of the year, 2 days now and 2 more approximately six months distant, with an issue :-).

Being unfamiliar with the tests and code I'm not currently in a good position to suggest a solution of which there are several:

  • Document that daylight saving time changes interfere with tests. This works for people (who find and read the appropriate documentation), but automatic builds during these times will generate spurious failures.
  • Document that tests should use UTC, either always or around daylight saving changes. This solves the issue for the aforementioned test, but could interfere with others. Specifically tests that explicitly focus on time zones may no longer accurately validate the functionality they are intended to examine.
  • Change the test_totals_with_time_delta_larger_than_24_hours to always use UTC. This test covers time periods and doesn't appear to have an interest in time zones so this would seem to be suitable. Having little experience with Python time zone handling I don't know how practical this would be to implement.
  • As mentioned, change the test to account for time zones. Not being familiar with the test or test framework I don't know the effort involved.
  • Change test running behavior so all tests use UTC. This seems too broad and runs the same risks as documenting that the tests use UTC.

@lauft lauft added the bug Something isn't working label Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants