Uses TZ = UTC in buildout

wesleybl commented 7 months ago

This ensures that tests using datetime will work locally on machines using TZ = GMT

It's the same thing that is in Plone 5.2 but was lost in Plone 6:

https://github.com/plone/buildout.coredev/blob/5.2/tests.cfg#L150-L158

wesleybl commented 7 months ago

@jenkins-plone-org please run jobs

davisagli commented 7 months ago

@wesleybl Which tests are failing due to the different timezone?

wesleybl commented 7 months ago

@davisagli @jensens see the original discussion about this: https://github.com/plone/buildout.coredev/pull/693

davisagli commented 7 months ago

@wesleybl When I run the buildout.coredev tests currently on my machine in UTC-8, I don't get any timezone-related errors. I think we improved the tests that were previously timezone-dependent, and don't currently need this.

davisagli commented 7 months ago

(Side note: on my M1 Mac, bin/test -j 4 completes in less than 6 minutes. Wow.)

mauritsvanrees commented 7 months ago

The pure Python test script that I wrote in this comment on 5.2 is still correct: setting TZ in the environment has no effect on Python 3.8 and higher; you need to call time.tzset() afterwards.

plone.restapi master does the same in base.cfg and in testing.py.

Since plone.restapi has this, I doubt this is still needed to do in coredev. But @wesleybl you have not yet said which tests fail for you.

I dug up some history from the beginning of 2021 to explain the difference between this part of tests.cfg in Plone 5.2 and 6:

Wesley created PR #693 to use TZ=UTC in 5.2, to fix test errors in plone.restapi, see this comment.
Jens merged this on February 1.
Then Wesley created PR #699 to do the same on 6.0.
Jens merged this on February 3.
On 6.0 Jens noticed that tests failed.
Jens reverted Wesley's change on 6.0 in PR #700.
On 5.2 I noticed that Python 3.8 tests failed in plone.app.discussion and plone.app.layout.
I created PR #701 for Plone 5.2 to call time.tzset() which fixed these errors.

This all happened within a few days, but no one put enough dots together to use the same solution on both branches. :-/ Oh well, that has been three years ago, so let's laugh at it. :-)

As said, my guess is that this is not needed anymore because plone.restapi handles this for its tests. Jenkins is happy either way. So the question is still what test failure would be solved by this.

wesleybl commented 7 months ago

@davisagli @mauritsvanrees when I run the ./bin/test -m plone.restapi command, I still have some errors. For example:

Failure in test test_transition_with_effective_date (plone.restapi.tests.test_workflow.TestWorkflowTransition.test_transition_with_effective_date)
Traceback (most recent call last):
  File "/usr/lib/python3.11/unittest/case.py", line 57, in testPartExecutor
    yield
  File "/usr/lib/python3.11/unittest/case.py", line 623, in run
    self._callTestMethod(testMethod)
  File "/usr/lib/python3.11/unittest/case.py", line 579, in _callTestMethod
    if method() is not None:
  File "/home/user/git/buildout.coredev/src/plone.restapi/src/plone/restapi/tests/test_workflow.py", line 197, in test_transition_with_effective_date
    self.assertEqual(
  File "/usr/lib/python3.11/unittest/case.py", line 873, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/lib/python3.11/unittest/case.py", line 1253, in assertMultiLineEqual
    self.fail(self._formatMessage(msg, standardMsg))
  File "/usr/lib/python3.11/unittest/case.py", line 703, in fail
    raise self.failureException(msg)
AssertionError: '2018-06-24T09:17:00+00:00' != '2018-06-24T09:17:00-03:00'
- 2018-06-24T09:17:00+00:00
?                    ^ ^
+ 2018-06-24T09:17:00-03:00
?                    ^ ^

Failure in test test_statictime_full_history (plone.restapi.tests.test_statictime.TestStaticTime.test_statictime_full_history)
Traceback (most recent call last):
  File "/usr/lib/python3.11/unittest/case.py", line 57, in testPartExecutor
    yield
  File "/usr/lib/python3.11/unittest/case.py", line 623, in run
    self._callTestMethod(testMethod)
  File "/usr/lib/python3.11/unittest/case.py", line 579, in _callTestMethod
    if method() is not None:
  File "/home/user/git/buildout.coredev/src/plone.restapi/src/plone/restapi/tests/test_statictime.py", line 242, in test_statictime_full_history
    self.assertEqual(
  File "/usr/lib/python3.11/unittest/case.py", line 873, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/lib/python3.11/unittest/case.py", line 1079, in assertListEqual
    self.assertSequenceEqual(list1, list2, msg, seq_type=list)
  File "/usr/lib/python3.11/unittest/case.py", line 1061, in assertSequenceEqual
    self.fail(msg)
  File "/usr/lib/python3.11/unittest/case.py", line 703, in fail
    raise self.failureException(msg)
AssertionError: Lists differ: [Date[21 chars]0:00 GMT-3'), -612844200.0, DateTime('1950/07/[15 chars]-3')] != [Date[21 chars]0:00 UTC'), -612855000.0, DateTime('1950/07/31 19:30:00 UTC')]

First differing element 0:
DateTime('1950/07/31 17:30:00 GMT-3')
DateTime('1950/07/31 17:30:00 UTC')

- [DateTime('1950/07/31 17:30:00 GMT-3'),
?                                ^^ ^^

+ [DateTime('1950/07/31 17:30:00 UTC'),
?                                ^ ^

-  -612844200.0,
?       ^^^

+  -612855000.0,
?       ^^^

-  DateTime('1950/07/31 19:30:00 GMT-3')]
?                                ^^ ^^

+  DateTime('1950/07/31 19:30:00 UTC')]
?              

Failure in test test_statictime_lockinfo (plone.restapi.tests.test_statictime.TestStaticTime.test_statictime_lockinfo)
Traceback (most recent call last):
  File "/usr/lib/python3.11/unittest/case.py", line 57, in testPartExecutor
    yield
  File "/usr/lib/python3.11/unittest/case.py", line 623, in run
    self._callTestMethod(testMethod)
  File "/usr/lib/python3.11/unittest/case.py", line 579, in _callTestMethod
    if method() is not None:
  File "/home/user/git/buildout.coredev/src/plone.restapi/src/plone/restapi/tests/test_statictime.py", line 278, in test_statictime_lockinfo
    self.assertEqual(-612858600.0, lock_infos[0]["time"])
  File "/usr/lib/python3.11/unittest/case.py", line 873, in assertEqual
    assertion_func(first, second, msg=msg)
  File "/usr/lib/python3.11/unittest/case.py", line 866, in _baseAssertEqual
    raise self.failureException(msg)
AssertionError: -612858600.0 != -612847800.0

Note that the use of the TZ environment variable in testing.py predates my first PR here. I commented out testing.py's setUp and tearDown methods and only got one more error:

Failure in test test_brain_summary_includes_all_metadata_fields (plone.restapi.tests.test_serializer_summary.TestSummarySerializers.test_brain_summary_includes_all_metadata_fields)
Traceback (most recent call last):
  File "/usr/lib/python3.11/unittest/case.py", line 57, in testPartExecutor
    yield
  File "/usr/lib/python3.11/unittest/case.py", line 623, in run
    self._callTestMethod(testMethod)
  File "/usr/lib/python3.11/unittest/case.py", line 579, in _callTestMethod
    if method() is not None:
  File "/home/user/git/buildout.coredev/src/plone.restapi/src/plone/restapi/tests/test_serializer_summary.py", line 144, in test_brain_summary_includes_all_metadata_fields
    self.assertLessEqual(
  File "/usr/lib/python3.11/unittest/case.py", line 1265, in assertLessEqual
    self.fail(self._formatMessage(msg, standardMsg))
  File "/usr/lib/python3.11/unittest/case.py", line 703, in fail
    raise self.failureException(msg)
AssertionError: dict_items([('@id', 'http://nohost/plone/doc1'), ('@type', 'DXTestDocument'), ('CreationDate', '2016-01-21T01:14:48+00:00'), ('Creator', 'test_user_1_'), ('Date', '2017-01-21T01:14:48+00:00'), ('Description', 'Description'), ('EffectiveDate', 'None'), ('ExpirationDate', 'None'), ('ModificationDate', '2017-01-21T01:14:48+00:00'), ('Subject', []), ('Title', 'Lorem Ipsum'), ('Type', 'DX Test Document'), ('UID', 'c6dcbd55ab2746e199cd4ed458000001'), ('author_name', None), ('cmf_uid', None), ('commentators', []), ('created', '2016-01-21T01:14:48+00:00'), ('description', 'Description'), ('effective', '1969-12-31T00:00:00+00:00'), ('end', None), ('exclude_from_nav', False), ('expires', '2499-12-31T00:00:00+00:00'), ('getIcon', None), ('getId', 'doc1'), ('getObjSize', '0 KB'), ('getPath', '/plone/doc1'), ('getRemoteUrl', None), ('getURL', 'http://nohost/plone/doc1'), ('id', 'doc1'), ('in_response_to', None), ('is_folderish', False), ('last_comment_date', None), ('listCreators', ['test_user_1_']), ('location', None), ('mime_type', 'text/plain'), ('modified', '2017-01-21T01:14:48+00:00'), ('portal_type', 'DXTestDocument'), ('review_state', 'private'), ('start', None), ('sync_uid', None), ('title', 'Lorem Ipsum'), ('total_comments', 0)]) not less than or equal to dict_items([('mime_type', 'text/plain'), ('@id', 'http://nohost/plone/doc1'), ('getId', 'doc1'), ('EffectiveDate', 'None'), ('description', 'Description'), ('portal_type', 'DXTestDocument'), ('exclude_from_nav', False), ('@type', 'DXTestDocument'), ('is_folderish', False), ('sync_uid', None), ('created', '2016-01-21T01:14:48+00:00'), ('commentators', []), ('in_response_to', None), ('title', 'Lorem Ipsum'), ('Creator', 'test_user_1_'), ('expires', '2499-12-31T03:00:00+00:00'), ('author_name', None), ('last_comment_date', None), ('total_comments', 0), ('getIcon', None), ('UID', 'c6dcbd55ab2746e199cd4ed458000001'), ('effective', '1969-12-31T03:00:00+00:00'), ('cmf_uid', None), ('Title', 'Lorem Ipsum'), ('getPath', '/plone/doc1'), ('listCreators', ['test_user_1_']), ('getObjSize', '0 KB'), ('start', None), ('Date', '2017-01-20T22:14:48-03:00'), ('Type', 'DX Test Document'), ('ModificationDate', '2017-01-20T22:14:48-03:00'), ('id', 'doc1'), ('type_title', 'DX Test Document'), ('end', None), ('getRemoteUrl', None), ('image_scales', None), ('getURL', 'http://nohost/plone/doc1'), ('Description', 'Description'), ('ExpirationDate', 'None'), ('location', None), ('nav_title', None), ('CreationDate', '2016-01-20T22:14:48-03:00'), ('modified', '2017-01-21T01:14:48+00:00'), ('Subject', []), ('review_state', 'private')])

So it seems like he's not solving all the problems.

Anyway, I think using TZ in buildout is more practical, because it will solve the problem in any package.

davisagli commented 7 months ago

@wesleybl Interesting, I can reproduce your errors if I set my system timezone to America/Sao_Paulo, but not when it is set to America/Los_Angeles.

I think we should fix this in plone.restapi's tests. The goal is for them to be consistent regardless of what TZ environment variable is set. I guess there is some bug in https://github.com/plone/plone.restapi/blob/main/src/plone/restapi/tests/statictime.py which tries to handle this already.

jensens commented 7 months ago

I guess there is some bug in https://github.com/plone/plone.restapi/blob/main/src/plone/restapi/tests/statictime.py which tries to handle this already.

Looking at statictime.py's complexity: Why don't we use freezegun here?

wesleybl commented 7 months ago

I guess there is some bug in https://github.com/plone/plone.restapi/blob/main/src/plone/restapi/tests/statictime.py which tries to handle this already.

@davisagli I don't quite understand what StaticTime is but it seems to be very specific for documentation tests. I don't know how to use it for other tests.

Looking at statictime.py's complexity: Why don't we use freezegun here?

@jensens It appears that the freezegun was used in the past, but problems occurred. See:

https://github.com/plone/plone.restapi/blob/24f25d7632ed23ffd0087ba810657250242ddccb/src/plone/restapi/tests/statictime.py#L31C5-L35C44

I still think that using the environment variable is much simpler and would solve the problem in all packages, not just restapi.

plone / buildout.coredev

Uses TZ = UTC in buildout #909