Page MenuHomePhabricator

[tests] Remove ctime() call which may be unreliable on some systems
ClosedPublic

Authored by jasonbcox on May 10 2020, 21:50.

Details

Summary

time.ctime() documentation is not clear on the maximum value supported, but we
get a hint from documentation of related functions such as:
https://docs.python.org/3/library/datetime.html#datetime.date.fromtimestamp

On my machine, I see failures somewhere around time.ctime(pow(2, 35)), although it varies
for some people: https://stackoverflow.com/questions/46133223/maximum-value-of-timestamp

Until the 2038 Problem is reliably fixed everywhere, we should not rely on it when logging
timestamps from tests, as these values will tend to prod boundary conditions (ie. very large
and very small timestamps).

Test Plan

ninja check-functional

Diff Detail

Repository
rABC Bitcoin ABC
Branch
fix-repr-ctime
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 10693
Build 19183: Default Diff Build & Tests
Build 19182: arc lint + arc unit

Event Timeline

Fabien requested changes to this revision.May 11 2020, 07:47
Fabien added a subscriber: Fabien.
Fabien added inline comments.
test/functional/test_framework/messages.py
511 ↗(On Diff #19877)

It would be easier to just display the timestamp as a number. If a test is checking the representation it would fail depending on the time.

This revision now requires changes to proceed.May 11 2020, 07:47
test/functional/test_framework/messages.py
511 ↗(On Diff #19877)

No test currently does this, but that's a good point.

Just remove the pretty printing entirely

jasonbcox retitled this revision from [tests] Limit timestamps passed to ctime() which may be unreliable on some systems to [tests] Remove ctime() call which may be unreliable on some systems.May 11 2020, 16:59
jasonbcox edited the summary of this revision. (Show Details)
This revision is now accepted and ready to land.May 11 2020, 21:50