test: Add unit test for AddTimeData#16563
Conversation
|
Welcome as a contributor @mzumsande! Would you mind adding a comment describing the bug-turned-feature to give some context to this change? :-) |
|
Maybe this:
|
|
Yes, I meant #4521 - there is also a comment about this in the source code of timedata.cpp which I referenced in the test. Basically, the method of getting offsets from peers will become ineffective after it has reached 200 samples, because it only attempts to change |
|
@mzumsande Could you add a comment to |
mzumsande
force-pushed
the
201908_test_timedata
branch
2 times, most recently
from
dbd01a9 to
cf02a00
Compare
|
@practicalswift I added a comment about this and also adjusted the other comments to make it more clear what is tested. |
ariard
left a comment
ariard
left a comment
There was a problem hiding this comment.
Otherwise, tested ACK cf02a00,
Thanks to give an opportunity to read about AddTimeData!
mzumsande
force-pushed
the
201908_test_timedata
branch
from
cf02a00 to
50a9f58
Compare
|
Thanks @ariard for the review, I adressed your comments. |
|
ACK 50a9f58. |
src/test/timedata_tests.cpp
Outdated
src/test/timedata_tests.cpp
Outdated
There was a problem hiding this comment.
nit, should #include <string> - iwyu.
src/test/timedata_tests.cpp
Outdated
There was a problem hiding this comment.
nit, could avoid warning var by doing GetWarnings("gui").find("...").
mzumsande
force-pushed
the
201908_test_timedata
branch
from
50a9f58 to
7cd069d
Compare
|
Addressed nits by @promag. |
|
ACK 7cd069d, thanks for adding a test |
7cd069d Add test for AddTimeData (Martin Zumsande) Pull request description: `AddTimeData()` has poor test coverage but interesting logic (including a bug turned into a feature). This PR adds a unit test for it. ACKs for top commit: laanwj: ACK 7cd069d, thanks for adding a test Tree-SHA512: 8228f9027e52ed534411d595c7e45cf4edeee9757f26f5141fbcfae3fc6f598a8cea7f734bb8f55238857a37ad2f2d518e859e1fe8c106c0712da976792ac132
Summary: This is a backport of Core [[bitcoin/bitcoin#16563 | PR16563]] Test Plan: make check Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D5844
Summary: This is a backport of Core [[bitcoin/bitcoin#16563 | PR16563]] Test Plan: make check Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D5844
7cd069d Add test for AddTimeData (Martin Zumsande) Pull request description: `AddTimeData()` has poor test coverage but interesting logic (including a bug turned into a feature). This PR adds a unit test for it. ACKs for top commit: laanwj: ACK 7cd069d, thanks for adding a test Tree-SHA512: 8228f9027e52ed534411d595c7e45cf4edeee9757f26f5141fbcfae3fc6f598a8cea7f734bb8f55238857a37ad2f2d518e859e1fe8c106c0712da976792ac132
7cd069d Add test for AddTimeData (Martin Zumsande) Pull request description: `AddTimeData()` has poor test coverage but interesting logic (including a bug turned into a feature). This PR adds a unit test for it. ACKs for top commit: laanwj: ACK 7cd069d, thanks for adding a test Tree-SHA512: 8228f9027e52ed534411d595c7e45cf4edeee9757f26f5141fbcfae3fc6f598a8cea7f734bb8f55238857a37ad2f2d518e859e1fe8c106c0712da976792ac132
7 participants
AddTimeData()has poor test coverage but interesting logic (including a bug turned into a feature). This PR adds a unit test for it.