Peer time offset fast unit test instead of slow functional one.

[furszy]

Refactored the time offset check to be able to create an unit test to validate the same behavior without requiring to run the full, time consuming, p2p_time_offset.py functional test.

Testing time difference results:

Old functional test p2p_time_offset --> 267-280 seconds.

VS

New unit test timeOffset --> 1 millisecond.

[random-zebra]

NACK.
The functional test covers the whole flow (from the connection, down to getneworkinfo() output), trying to connect nodes with different times (set with setmocktime), even with offset too big (CheckOffsetDisconnectedPeers).

This unit test simply fills a vector of int64_t and verifies that the median is above and below the limit. Nothing else.

Further, being the runtime well below 1 sec, GetTime() is always equal to curTime, so you are just using offsets [0, 0, ..., 0] in the first case, of [-15, -15, ..., -15] in the second, and of [+15, +15, ..., +15] in the last one.
I think this is pretty useless, especially since we have util_MedianFilter (timedata_tests.cpp), which already tests (and with different samples) the CMedianFilter class.

[furszy]

I'm on the fence here. I did it while the functional test suite was running.. because it's a really long time consuming test that tests flows already tested elsewhere (I missed the CheckOffsetDisconnectedPeers method).

The peer offset test goal was to verify the correct time offset flow behavior, not the entire connections flow, nor any other flow. The time offset test should (and can) be only focused on the functionality that we are trying to test and nothing else.

I'm agree on expanding this to cover CheckOffsetDisconnectedPeers -- method which should be decoupled better to not mix flows with the networking code, or.. can go to the caller place and decouple it better as a complete addTimeData and CheckOffsetDisconnectedPeers flow. Abstracting it from the endless ProcessMessage method (possibly mocking the peers connection too). But well, this last flow will need a bigger refactoring.

So well, we need to do more in the architecture area to be able to test really specific flows in a set of unit tests and not spend testing time validating things that are already tested elsewhere (like peers connections).

[Fuzzbawls]

I'd rather keep this as a functional test rather than a unit test, as @random-zebra points out, the functional test covers the entire range from connection handshake.