qa: Use node.datadir instead of tmpdir in test framework #12076

1. 
   MarcoFalke commented at 1:05 pm on January 2, 2018: member

   Commit c53c9831eedaf3b311bb942945268830f9ba3abc introduced the utility function get_datadir_path, however not all places in the code use this util function. Using the util function everywhere makes it easier to review pull requests related to the datadir.

   This commit replaces datadir path creation with the datadir member of TestNode, which itself uses get_datadir_path.

2. MarcoFalke added the label Refactoring on Jan 2, 2018
3. MarcoFalke added the label Tests on Jan 2, 2018
4. MarcoFalke force-pushed on Jan 2, 2018
5. MarcoFalke force-pushed on Jan 2, 2018
6. MarcoFalke force-pushed on Jan 2, 2018
7. in test/functional/keypool-topup.py:35 in fa72adaaac outdated

   36         self.log.info("Make backup of wallet")
   37-
   38         self.stop_node(1)
   39-
   40-        shutil.copyfile(self.tmpdir + "/node1/regtest/wallets/wallet.dat", self.tmpdir + "/wallet.bak")
   41+        shutil.copyfile(datadir_regtest_1 + "/wallets/wallet.dat", datadir_regtest_1 + "/wallet.bak")
   

   ---

   ---

   

   promag commented at 3:08 pm on January 2, 2018:

   Should also use os.path.join?
8. 
   promag commented at 3:15 pm on January 2, 2018: member

   utACK fa72ada.

   BTW, maybe this should be simpler:

   0foo = self.node_path(1, 'regtest', 'wallets', 'wallet.dat')
   

9. fanquake deleted a comment on Jan 3, 2018
10. MarcoFalke force-pushed on Jan 3, 2018
11. MarcoFalke force-pushed on Jan 3, 2018
12. MarcoFalke force-pushed on Jan 3, 2018
13. MarcoFalke renamed this:
    qa: Use get_datadir_path
    qa: Use self.node_path
    on Jan 3, 2018
14. MarcoFalke renamed this:
    qa: Use self.node_path
    qa: Use node_path wrapper in test framwork
    on Jan 3, 2018
15. MarcoFalke force-pushed on Jan 3, 2018
16. MarcoFalke force-pushed on Jan 3, 2018
17. MarcoFalke force-pushed on Jan 3, 2018
18. MarcoFalke force-pushed on Jan 3, 2018
19. 
    promag commented at 4:57 pm on January 3, 2018: member

    utACK fa3dda1, will check for missing changes.
20. in test/functional/test_framework/test_framework.py:220 in fa3dda1297 outdated

    216@@ -213,7 +217,7 @@ def add_nodes(self, num_nodes, extra_args=None, rpchost=None, timewait=None, bin
    217         assert_equal(len(extra_args), num_nodes)
    218         assert_equal(len(binary), num_nodes)
    219         for i in range(num_nodes):
    220-            self.nodes.append(TestNode(i, self.options.tmpdir, extra_args[i], rpchost, timewait=timewait, binary=binary[i], stderr=None, mocktime=self.mocktime, coverage_dir=self.options.coveragedir))
    221+            self.nodes.append(TestNode(i, self.node_path(i), extra_args[i], rpchost, timewait=timewait, binary=binary[i], stderr=None, mocktime=self.mocktime, coverage_dir=self.options.coveragedir))
    

    ---

    ---

    

    promag commented at 11:08 pm on January 3, 2018:

    Use get_datadir_path(self.options.tmpdir, i) instead.
21. in test/functional/test_framework/test_framework.py:208 in fa3dda1297 outdated

    203@@ -203,6 +204,9 @@ def run_test(self):
    204 
    205     # Public helper methods. These can be accessed by the subclass test scripts.
    206 
    207+    def node_path(self, i, *paths):
    208+        return os.path.join(get_datadir_path(self.options.tmpdir, i), *paths)
    

    ---

    ---

    

    promag commented at 11:09 pm on January 3, 2018:

    Use os.path.join(node[i].datadir, *paths) instead.
22. 
    promag commented at 11:12 pm on January 3, 2018: member

    With these suggestions, node_path honours runtime changes to datadir, not to mention get_datadir_path is called once only.
23. 
    promag commented at 11:13 pm on January 3, 2018: member

    Nit, there are a couple of these (node[i].datadir) that could be changed:

    0-        assert os.path.isfile(os.path.join(self.nodes[0].datadir, "regtest", "debug.log"))
    1+        assert os.path.isfile(self.node_path(0, 'regtest', 'debug.log'))
    

24. laanwj renamed this:
    qa: Use node_path wrapper in test framwork
    qa: Use node_path wrapper in test framework
    on Jan 5, 2018
25. 
    jnewbery commented at 7:00 pm on January 8, 2018: member

    concept ACK.

    What are the benefits of using self.node_path(i) over self.nodes[i].datadir?

26. 
    promag commented at 7:19 pm on January 8, 2018: member

    Implicit path join.
27. 
    jnewbery commented at 7:42 pm on January 8, 2018: member

    ok, let me rephrase: what are the benefits of implementing this as a method on BitcoinTestFramework rather than on TestNode?
28. 
    promag commented at 8:43 pm on January 8, 2018: member

    @jnewbery I guess either are ok. You prefer node[i].path() or something like that? My suggestion was to avoid os.path.join() or datadir + '/' foo all over the tests.
29. MarcoFalke force-pushed on Jan 13, 2018
30. MarcoFalke renamed this:
    qa: Use node_path wrapper in test framework
    qa: Use node.datadir instead of tmpdir in test framework
    on Jan 13, 2018
31. MarcoFalke force-pushed on Jan 13, 2018
32. MarcoFalke force-pushed on Jan 13, 2018
33. MarcoFalke force-pushed on Mar 6, 2018
34. MarcoFalke force-pushed on Mar 7, 2018
35. MarcoFalke force-pushed on Mar 8, 2018
36. MarcoFalke force-pushed on Mar 17, 2018
37. qa: Use node.datadir instead of tmpdir in test framework c8330d4216
38. MarcoFalke force-pushed on Mar 19, 2018
39. laanwj merged this on Mar 22, 2018
40. laanwj closed this on Mar 22, 2018

    ---

41. laanwj referenced this in commit 6d36f599f8 on Mar 22, 2018
42. MarcoFalke deleted the branch on Mar 22, 2018
43. PastaPastaPasta referenced this in commit 6d46698979 on Dec 16, 2020
44. PastaPastaPasta referenced this in commit e0739852d7 on Dec 18, 2020
45. DrahtBot locked this on Sep 8, 2021

Contributors
MarcoFalke promag jnewbery

Labels
Refactoring Tests