test: move sync_* implementations to framework member functions #18932

  26 | @@ -27,9 +27,7 @@
  27 |  
  28 |  from test_framework.util import (
  29 |      adjust_bitcoin_conf_for_pre_17,
  30 | -    assert_equal,
  31 | -    sync_blocks,
  32 | -    sync_mempools,
  33 | +    assert_equal

MarcoFalke commented at 2:51 PM on May 10, 2020:

nit: Please keep the trailing comma to minimize this diff and future diffs, as well as guiding editors with formatting.

  29 | @@ -30,9 +30,7 @@
  30 |      connect_nodes,
  31 |      disconnect_nodes,
  32 |      get_datadir_path,
  33 | -    initialize_datadir,
  34 | -    sync_blocks,
  35 | -    sync_mempools,
  36 | +    initialize_datadir

MarcoFalke commented at 2:51 PM on May 10, 2020:

same

 528 | -        sync_mempools(nodes or self.nodes, **kwargs)
 529 | +        sync_blocks needs to be called with an rpc_connections set that has least
 530 | +        one node already synced to the latest, stable tip, otherwise there's a
 531 | +        chance it might return before all nodes are stably synced.
 532 | +        """
 533 | +        stop_time = time.time() + timeout

MarcoFalke commented at 5:14 PM on May 10, 2020:

The timeout should be scaled by self.factor, no? If yes, please add a commit for that.

brakmic commented at 5:43 PM on May 10, 2020:

Wasn't aware of it as the methods weren't adapted in the factor-PR. Have used the original self.options.factor property to adjust timeouts for both methods.