Add unit test for the outbound connection logic
ClosedPublic
Actions

Authored by Fabien on Aug 4 2022, 20:26.

Details

Reviewers

sdulfari

Group Reviewers

Restricted Project

Commits

rABC9b5e2671d190: Add unit test for the outbound connection logic

Summary

This diff adds a CConnmanTest implementation that allows for testing the outbound connection logic that is used in the network thread. For now only a test for the network group limitation is added so we can later add and test avalanche specifics for this feature.

Depends on D11827.

Test Plan

ninja check

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.Aug 4 2022, 20:26

Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 4 2022, 20:26

Fabien requested review of this revision.Aug 4 2022, 20:26

Fabien added a child revision: D11829: [avalanche] Remove the network bypass flag, make it default for avalanche outbounds.Aug 4 2022, 20:39

Harbormaster completed remote builds in B19774: Diff 34588.Aug 4 2022, 20:42

sdulfari added a subscriber: sdulfari.Aug 5 2022, 06:32

sdulfari added inline comments.

src/net.cpp
2288–2289 ↗	(On Diff #34588)	There's a bad code smell here where mockOpenConnection is checked for this interruptNet.sleep_for but not the others. After digging around this patch, it becomes clear the other code paths aren't affected, but will this be true in the future?
src/test/net_tests.cpp
1241–1247 ↗	(On Diff #34588)	Nit: Formatting like this collapses down a lot of white and vertical space. This can be applied everywhere, but is most acute in this part since each item is the same except for the group number. Expected outbound counts can stay on their own lines as is.

Fabien added inline comments.Aug 5 2022, 08:43

src/net.cpp
2288–2289 ↗	(On Diff #34588)	There are 2 kinds of sleeps: The ones for waiting before retrying the connections: this is the one that is avoided here. The above 2 are also of this kind but this code is only relevant to the `-connect` option which is a bit out of scope for this diff and can be changed if needed in the future. The ones that are by design (see the feeler time "noise" below) which we want to keep and eventually test.
src/test/net_tests.cpp
1241–1247 ↗	(On Diff #34588)	Good idea