[tests] functional tests should call BitcoinTestFramework start/stop node methods #10359

pull jnewbery wants to merge 3 commits into bitcoin:master from jnewbery:test_framework_start_stop_nodes changing 30 files +125 −128
  1. jnewbery commented at 4:38 pm on May 8, 2017: member

    Second step towards #10082

    This PR changes the individual test cases to call the BitcoinTestFramework.start_node and .stop_node methods rather than the util.py functions. This is in preparation for moving the start/stop node functionality into BitcoinTestFramework (currently the BitcoinTestFramework start/stop node methods are just wrappers for the util.py functions).

    The second commit changes the names of the functions in util.py to mark them as private. This is so no test cases are added/modified to call the functions directly before the next step of #10082 is merged.

  2. MarcoFalke commented at 6:17 pm on May 8, 2017: member
    Concept ACK. Needs rebase.
  3. MarcoFalke added the label Tests on May 8, 2017
  4. jnewbery force-pushed on May 8, 2017
  5. jnewbery force-pushed on May 9, 2017
  6. jnewbery force-pushed on May 15, 2017
  7. jnewbery commented at 8:00 pm on May 15, 2017: member
    rebased
  8. [tests] Functional tests call self.start_node(s) and self.stop_node(s) 
    This commit changes the individual test scripts to call the
start_node(s) and stop_node(s) methods in BitcoinTestFramework.
    d8c218f9c2
  9. [tests] Update start/stop node functions to be private module functions 
    This commit marks the start/stop functions in util.py as private module
functions. A future PR will remove these entirely and move the
functionality directly into the BitcoinTestFramework class, but setting them as
private in this PR will prevent anyone from accidentally calling them
before that future PR is merged.
    a433d8a15e
  10. jnewbery force-pushed on May 31, 2017
  11. jnewbery commented at 9:50 pm on May 31, 2017: member

    rebased. It’d be really helpful to get this merged so we can make some progress on #10082 . This is the only disruptive changeset in that larger PR - the rest of the changes are restricted to the test_framework directory.

    This PR touches a lot of files, but it should be a trivial review. It’s simply changing calls from stop_node() to calls to self.stop_node() etc wrapper functions in the first commit. The second commit renames the inner functions so no-one calls them explicitly from the test cases (the inner functions will be removed entirely in a future PR). @MarcoFalke , @TheBlueMatt , @ryanofsky , @sdaftuar, @jimmysong, @kallewoof - if I could convince any of you to review, I’d be very grateful!

  12. in test/functional/abandonconflict.py:77 in d8c218f9c2 outdated 
    72@@ -73,8 +73,8 @@ def run_test(self):
73 
74         # Restart the node with a higher min relay fee so the parent tx is no longer in mempool
75         # TODO: redo with eviction
76-        stop_node(self.nodes[0],0)
77-        self.nodes[0]=start_node(0, self.options.tmpdir, ["-minrelaytxfee=0.0001"])
78+        self.stop_node(0)
79+        self.nodes[0] = self.start_node(0, self.options.tmpdir, ["-minrelaytxfee=0.0001"])
    jimmysong commented at 10:27 pm on May 31, 2017:
    The plan is to change start_node so assignment here would be unnecessary in the future?
    jnewbery commented at 2:05 pm on June 1, 2017:
    yes - that’s certainly something we can do in a future PR once the BitcoinTestFramework class is responsible for tracking and stop/starting the nodes.
  13. in test/functional/blockchain.py:27 in d8c218f9c2 outdated 
    23@@ -24,6 +24,7 @@
24     assert_raises_jsonrpc,
25     assert_is_hex_string,
26     assert_is_hash_string,
27+    connect_nodes_bi,
    jimmysong commented at 10:29 pm on May 31, 2017:
    Not sure why this was added?
    ryanofsky commented at 10:29 pm on May 31, 2017:
    Change seems unrelated, maybe revert.
    jnewbery commented at 2:04 pm on June 1, 2017:
    yes. not sure how that snuck in. Reverted.
    jnewbery commented at 2:06 pm on June 1, 2017:
    me neither :flushed:. Removed
  14. in test/functional/fundrawtransaction.py:457 in d8c218f9c2 outdated 
    456-        stop_node(self.nodes[0], 0)
457-        stop_node(self.nodes[1], 2)
458-        stop_node(self.nodes[2], 3)
459 
460-        self.nodes = start_nodes(self.num_nodes, self.options.tmpdir)
461+        self.nodes = self.start_nodes(self.num_nodes, self.options.tmpdir)
    jimmysong commented at 10:30 pm on May 31, 2017:
    I assume that self.start_nodes would also do the assignment in the future?
    kallewoof commented at 0:04 am on June 1, 2017:
    Unless it should be possible to append nodes.
    jnewbery commented at 2:07 pm on June 1, 2017:

    certainly something we can do in a future PR.

    BitcoinTestFramework could include two functions - start_nodes() to start the nodes in self.nodes, and add_nodes() if you need to append nodes to that list.

  15. in test/functional/net.py:17 in d8c218f9c2 outdated 
    13@@ -14,8 +14,7 @@
14     assert_equal,
15     assert_raises_jsonrpc,
16     connect_nodes_bi,
17-    p2p_port,
18-)
19+    p2p_port)
    jimmysong commented at 10:31 pm on May 31, 2017:
    Nit: I personally like comma endings so adding new imports has cleaner diffs.
    kallewoof commented at 0:04 am on June 1, 2017:
    Agreed
    jnewbery commented at 2:08 pm on June 1, 2017:
    Sure. Reverted.
  16. in test/functional/fundrawtransaction.py:452 in d8c218f9c2 outdated 
    448@@ -449,12 +449,12 @@ def run_test(self):
449         ############################################################
450         # locked wallet test
451         self.nodes[1].encryptwallet("test")
452+        self.stop_node(0)
    ryanofsky commented at 10:33 pm on May 31, 2017:
    Maybe move these up one line so nodes[1].encrypt and pop(1) can be next to each other.
  17. ryanofsky commented at 10:48 pm on May 31, 2017: member
    Slightly tested ACK a433d8a15e165cfb42e44319d608365603f64e0c
  18. jimmysong approved
  19. jimmysong commented at 10:53 pm on May 31, 2017: contributor
    ACK. testing now.
  20. jimmysong commented at 11:35 pm on May 31, 2017: contributor

    Tested, ACK a433d8a15e165cfb42e44319d608365603f64e0c

    A couple of really small nits. Excited to keep this moving!

  21. kallewoof approved
  22. kallewoof commented at 0:06 am on June 1, 2017: member
    utACK a433d8a15e165cfb42e44319d608365603f64e0c
  23. fixup: fix nits 53f6775fe1
  24. jnewbery commented at 2:08 pm on June 1, 2017: member
    Thanks @ryanofsky @jimmysong @kallewoof . Nits fixed in fixup commit. Will squash when this is ready for merge.
  25. MarcoFalke commented at 10:12 am on June 2, 2017: member
    utACK 53f6775
  26. MarcoFalke merged this on Jun 2, 2017
  27. MarcoFalke closed this on Jun 2, 2017
  28. MarcoFalke referenced this in commit 329fc1dce7 on Jun 2, 2017
  29. jnewbery deleted the branch on Jun 2, 2017
  30. PastaPastaPasta referenced this in commit 5ef558b79b on Jun 20, 2019
  31. PastaPastaPasta referenced this in commit 9eb1d29471 on Jun 20, 2019
  32. PastaPastaPasta referenced this in commit 3e12ca8539 on Jun 22, 2019
  33. PastaPastaPasta referenced this in commit 7827881304 on Jun 22, 2019
  34. PastaPastaPasta referenced this in commit b07fccfe91 on Jun 22, 2019
  35. PastaPastaPasta referenced this in commit 0fa4ef95c7 on Jun 22, 2019
  36. PastaPastaPasta referenced this in commit 92aa8f1ae9 on Jun 22, 2019
  37. PastaPastaPasta referenced this in commit 004800f921 on Jun 22, 2019
  38. PastaPastaPasta referenced this in commit 96f83f79d4 on Jun 22, 2019
  39. PastaPastaPasta referenced this in commit dec9be3b3b on Jun 22, 2019
  40. PastaPastaPasta referenced this in commit 1e2c4ea6b1 on Jun 22, 2019
  41. PastaPastaPasta referenced this in commit e9cc02f777 on Jun 22, 2019
  42. PastaPastaPasta referenced this in commit e353d271f0 on Jun 26, 2019
  43. barrystyle referenced this in commit 1dac01ceb9 on Jan 22, 2020
  44. DrahtBot locked this on Sep 8, 2021


jnewbery MarcoFalke jimmysong ryanofsky kallewoof

Labels
Tests

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 18:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me