test refactor: feature_config_args.py - Stop nodes at the end of tests, not at the beginning

This ensures we don't needlessly start the node, and reduces implicit dependencies between test functions.

test_seed_peers() - Move assert calling RPC to verify correct chain after our own function actually started the node.

Author: hodlinator

test/functional/feature_config_args.py

@@ -27,9 +27,20 @@ def set_test_params(self):
         self.wallet_names = []
         self.disable_autoconnect = False
 
+    # Overridden to avoid attempt to sync not yet started nodes.
+    def setup_network(self):
+        self.setup_nodes()
+
+    # Overriden to not start nodes automatically - doing so is the
+    # responsibility of each test function.
+    def setup_nodes(self):
+        self.add_nodes(self.num_nodes, self.extra_args)
+        # Ensure a log file exists as TestNode.assert_debug_log() expects it.
+        self.nodes[0].debug_log_path.parent.mkdir()
+        self.nodes[0].debug_log_path.touch()
+
     def test_config_file_parser(self):
         self.log.info('Test config file parser')
-        self.stop_node(0)
 
         # Check that startup fails if conf= is set in bitcoin.conf or in an included conf file
         bad_conf_file_path = self.nodes[0].datadir_path / "bitcoin_bad.conf"
@@ -162,12 +173,11 @@ def test_invalid_command_line_options(self):
                 )
 
     def test_log_buffer(self):
-        self.stop_node(0)
         with self.nodes[0].assert_debug_log(expected_msgs=['Warning: parsed potentially confusing double-negative -connect=0\n']):
             self.start_node(0, extra_args=['-noconnect=0'])
+        self.stop_node(0)
 
     def test_args_log(self):
-        self.stop_node(0)
         self.log.info('Test config args logging')
         with self.nodes[0].assert_debug_log(
                 expected_msgs=[
@@ -196,10 +206,10 @@ def test_args_log(self):
                 '-rpcuser=secret-rpcuser',
                 '-torpassword=secret-torpassword',
             ])
+        self.stop_node(0)
 
     def test_networkactive(self):
         self.log.info('Test -networkactive option')
-        self.stop_node(0)
         with self.nodes[0].assert_debug_log(expected_msgs=['SetNetworkActive: true\n']):
             self.start_node(0)
 
@@ -222,16 +232,12 @@ def test_networkactive(self):
         self.stop_node(0)
         with self.nodes[0].assert_debug_log(expected_msgs=['SetNetworkActive: false\n']):
             self.start_node(0, extra_args=['-nonetworkactive=1'])
+        self.stop_node(0)
 
     def test_seed_peers(self):
         self.log.info('Test seed peers')
         default_data_dir = self.nodes[0].datadir_path
         peer_dat = default_data_dir / 'peers.dat'
-        # Only regtest has no fixed seeds. To avoid connections to random
-        # nodes, regtest is the only network where it is safe to enable
-        # -fixedseeds in tests
-        util.assert_equal(self.nodes[0].getblockchaininfo()['chain'],'regtest')
-        self.stop_node(0)
 
         # No peers.dat exists and -dnsseed=1
         # We expect the node will use DNS Seeds, but Regtest mode does not have
@@ -248,6 +254,12 @@ def test_seed_peers(self):
                 timeout=10,
         ):
             self.start_node(0, extra_args=['-dnsseed=1', '-fixedseeds=1', f'-mocktime={start}'])
+
+        # Only regtest has no fixed seeds. To avoid connections to random
+        # nodes, regtest is the only network where it is safe to enable
+        # -fixedseeds in tests
+        util.assert_equal(self.nodes[0].getblockchaininfo()['chain'],'regtest')
+
         with self.nodes[0].assert_debug_log(expected_msgs=[
                 "Adding fixed seeds as 60 seconds have passed and addrman is empty",
         ]):
@@ -294,13 +306,13 @@ def test_seed_peers(self):
                 "Adding fixed seeds as 60 seconds have passed and addrman is empty",
         ]):
             self.nodes[0].setmocktime(start + 65)
+        self.stop_node(0)
 
     def test_connect_with_seednode(self):
         self.log.info('Test -connect with -seednode')
         seednode_ignored = ['-seednode is ignored when -connect is used\n']
         dnsseed_ignored = ['-dnsseed is ignored when -connect is used and -proxy is specified\n']
         addcon_thread_started = ['addcon thread start\n']
-        self.stop_node(0)
 
         # When -connect is supplied, expanding addrman via getaddr calls to ADDR_FETCH(-seednode)
         # nodes is irrelevant and -seednode is ignored.
@@ -325,6 +337,7 @@ def test_connect_with_seednode(self):
             with self.nodes[0].assert_debug_log(expected_msgs=addcon_thread_started,
                     unexpected_msgs=seednode_ignored):
                 self.restart_node(0, extra_args=[connect_arg, '-seednode=fakeaddress2'])
+        self.stop_node(0)
 
     def test_ignored_conf(self):
         self.log.info('Test error is triggered when the datadir in use contains a bitcoin.conf file that would be ignored '