Skip to content

Conversation

@wchargin
Copy link
Contributor

Summary:
We mocked out the time zone to UTC for only one of two time-related
function calls. This was asymptomatic externally because Bazel’s test
runner sets the TZ environment variable to UTC
, but the internal
test runner has different semantics.

Test Plan:
Patching this into a sync CL fixes broken tests.

wchargin-branch: util-test-tz

Summary:
We mocked out the time zone to UTC for only one of two time-related
function calls. This was asymptomatic externally because [Bazel’s test
runner sets the `TZ` environment variable to `UTC`][1], but the internal
test runner has different semantics.

Test Plan:
Patching this into a sync CL fixes broken tests.

[1]: https://docs.bazel.build/versions/1.2.0/test-encyclopedia.html#initial-conditions

wchargin-branch: util-test-tz
@wchargin wchargin merged commit 4ca0ef4 into master Nov 27, 2019
@wchargin wchargin deleted the wchargin-util-test-tz branch November 27, 2019 19:36
wchargin added a commit that referenced this pull request Jan 27, 2020
Summary:
Follow-up to #2978, which added explicit configuration of `TZ=UTC`. In
cPython 3, the time zone used by `datetime.datetime.fromtimestamp` is
[cached when `time` is imported][1] unless explicitly reset, so mocking
the environment variable is no longer sufficient. We now call `tzset`
ourselves; we could also just set `TZ=UTC` before importing `datetime`,
but this seems cleaner.

[1]: https://github.com/python/cpython/blob/2528a6c3d0660c03ae43d796628462ccf8e58190/Modules/timemodule.c#L1752-L1763

Test Plan:
A test sync shows that this test passes in both Python 2 and Python 3,
whereas it previously only passed in Python 2.

wchargin-branch: util-test-tzset
wchargin added a commit that referenced this pull request Jan 28, 2020
Summary:
Follow-up to #2978, which added explicit configuration of `TZ=UTC`. In
cPython 3, the time zone used by `datetime.datetime.fromtimestamp` is
[cached when `time` is imported][1] unless explicitly reset, so mocking
the environment variable is no longer sufficient. We now call `tzset`
ourselves; we could also just set `TZ=UTC` before importing `datetime`,
but this seems cleaner.

[1]: https://github.com/python/cpython/blob/2528a6c3d0660c03ae43d796628462ccf8e58190/Modules/timemodule.c#L1752-L1763

Test Plan:
A test sync shows that this test passes in both Python 2 and Python 3,
whereas it previously only passed in Python 2.

wchargin-branch: util-test-tzset
bileschi pushed a commit to bileschi/tensorboard that referenced this pull request Mar 3, 2020
Summary:
Follow-up to tensorflow#2978, which added explicit configuration of `TZ=UTC`. In
cPython 3, the time zone used by `datetime.datetime.fromtimestamp` is
[cached when `time` is imported][1] unless explicitly reset, so mocking
the environment variable is no longer sufficient. We now call `tzset`
ourselves; we could also just set `TZ=UTC` before importing `datetime`,
but this seems cleaner.

[1]: https://github.com/python/cpython/blob/2528a6c3d0660c03ae43d796628462ccf8e58190/Modules/timemodule.c#L1752-L1763

Test Plan:
A test sync shows that this test passes in both Python 2 and Python 3,
whereas it previously only passed in Python 2.

wchargin-branch: util-test-tzset
nfelt pushed a commit that referenced this pull request Mar 4, 2020
Summary:
Follow-up to #2978, which added explicit configuration of `TZ=UTC`. In
cPython 3, the time zone used by `datetime.datetime.fromtimestamp` is
[cached when `time` is imported][1] unless explicitly reset, so mocking
the environment variable is no longer sufficient. We now call `tzset`
ourselves; we could also just set `TZ=UTC` before importing `datetime`,
but this seems cleaner.

[1]: https://github.com/python/cpython/blob/2528a6c3d0660c03ae43d796628462ccf8e58190/Modules/timemodule.c#L1752-L1763

Test Plan:
A test sync shows that this test passes in both Python 2 and Python 3,
whereas it previously only passed in Python 2.

wchargin-branch: util-test-tzset
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants