net: Make addr relay mockable, add test #18454

As usual:

• Switch to std::chrono time to be type-safe and mockable
• Add basic test that relies on mocktime to add code coverage

Concept ACK. The code changes look good, will review the new test.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--2502f1a698b3751726fa55edcda76cd3-->

Coverage

^{Updated at: 2020-03-28T00:10:40.605800.}

Concept ACK

Thanks taking on this unglamourous (in the short-run) but important (in the long-run) work.

Code review ACK fa5bf23d527a450e72c2bf13d013e5393b664ca3, clean change, just read the new test.

Code looks good. I can't understand what the test is actually testing for?

It is a basic test to check that addr messages can be received (and are then relayed). For more information you can read the lines self.log.info(...)

Force pushed because I had to re-arrange the commits. Diff to previous review should be empty.

Yeah I tried reading that, just wanted to confirm. So, it actually doesn't test the new behavior (mockability)? Because setmocktime is called at the very end and nothing is really tested after it.

assert_debug_log is a context manager, so the checks in it are executed at the end.

You can check that the test fails by compiling current master, then checking out this branch, then running the functional tests without recompiling.

You can also check that this test fails by removing the call to setmocktime in the functional test.

utACK fad3f58fd5a9d35d5a6e3467b1b5bcf18081ca41, although I would prefer to comment the last lines of the test. I think it's counter-intuitive of what you are actually trying to demonstrate.

@naumenkogs Happy to do that, could you leave suggestions for in-line comments please, so that I can take them?

Removed last commit after chat with @naumenkogs to keep this pull focussed on one topic. The test in the last commit can be added later, if needed.

utACK fa1da3d

@promag Mind to re-ACK after I dropped one commit?

ACK fa1da3d4bfc0511a89f5b19d5a4d89e55ff7ccde (fabe56e44b6f683e24e37246a7a8851190947cb3 before #18454 (comment)), fa5bf23d527a450e72c2bf13d013e5393b664ca3 was dropped since last review.