Page MenuHomePhabricator

Added a block index test for received time
ClosedPublic

Authored by jasonbcox on Jul 3 2018, 18:17.

Details

Summary

Continues work on T355. Depends on D1534.

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
index3
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2834
Build 3772: Bitcoin ABC Buildbot (legacy)
Build 3771: arc lint + arc unit

Event Timeline

deadalnix requested changes to this revision.Jul 4 2018, 22:23
deadalnix added inline comments.
src/test/blockindex_tests.cpp
102 ↗(On Diff #4196)

Should the llabs be in there ? I see a potential pitfall using it because abs will simply truncate.

Corollary: this needs to test ranges that do not fit on 32 bits.

This revision now requires changes to proceed.Jul 4 2018, 22:23
src/test/blockindex_tests.cpp
102 ↗(On Diff #4196)

The llabs() call is in validation.cpp, wrapping the call to GetReceivedTimeDiff(). The intent of this was to allow it to be used to compare which block came first (if necessary). For that reason, it's not included in the test. Do you have an opinion on the diff always being a positive value?

Also, I've fixed the test to check greater-than-32-bit values.

Added coverage for 64-bit received time

deadalnix requested changes to this revision.Jul 6 2018, 11:50
deadalnix added inline comments.
src/test/blockindex_tests.cpp
102 ↗(On Diff #4216)

This is likely testing nothing because of overflow.

This revision now requires changes to proceed.Jul 6 2018, 11:50
jasonbcox added inline comments.
src/test/blockindex_tests.cpp
102 ↗(On Diff #4216)

Good catch.

jasonbcox marked an inline comment as done.

Fixed integer overflow.

Fixed typing/casting according to offline feedback

deadalnix added inline comments.
src/test/blockindex_tests.cpp
89
std::numeric_limits<uint32_t>::max()
This revision is now accepted and ready to land.Jul 10 2018, 16:09

constant -> numeric_limits

This revision was automatically updated to reflect the committed changes.