[qa] Ensure bitcoind processes are cleaned up when tests end #12904

1. 
   sdaftuar commented at 3:09 pm on April 6, 2018: member

   When tests fail (such as due to a bug in the test, race condition, etc), it’s possible that we could follow code paths that bypass our normal node shutdown that occurs in TestNode.stop_node. Add a destructor to TestNode that cleans this up.
2. 
   sdaftuar commented at 3:13 pm on April 6, 2018: member

   I have been able to reproduce the issue of bitcoind processes living on after tests exit in a couple ways:

   • #12375: running wallet_encryption.py on macos fails and leaves a bitcoind running
   • the feature_dbcrash.py failure that should be fixed in #12902 does the same.

   This PR causes all nodes to exit in both those situations.

3. 
   MarcoFalke commented at 3:30 pm on April 6, 2018: member

   utACK 9caecce6c7b00d3cc43eb220e91e9d1ac42251e8, makes sense
4. 
   laanwj commented at 4:59 pm on April 6, 2018: member

   Should this heed –noshutdown?
5. in test/functional/test_framework/test_node.py:91 in 9caecce6c7 outdated

   83@@ -84,6 +84,13 @@ def __init__(self, i, datadir, rpchost, timewait, binary, stderr, mocktime, cove
   84 
   85         self.p2ps = []
   86 
   87+    def __del__(self):
   88+        # Ensure that we don't leave any bitcoind processes lying around after
   89+        # the test ends
   90+        if self.process:
   91+            self.log.info("Cleaning up leftover process")
   

   ---

   ---

   

   MarcoFalke commented at 5:04 pm on April 6, 2018:

   Looks like the log is flushed and closed when the tests are done, so wouldn’t work at this point.

   ---

   

   sdaftuar commented at 6:30 pm on April 6, 2018:

   Odd, I was able to see this logging work when I tested locally. Will remove.

   ---

   

   jnewbery commented at 8:08 pm on April 6, 2018:

   Yes, it appears that logging can work after calling logging.shutdown(), but the documentation states that no calls to logging should be made after shutdown: https://docs.python.org/3/library/logging.html#logging.shutdown
6. 
   sdaftuar commented at 6:31 pm on April 6, 2018: member

   Should this heed –noshutdown? @laanwj Oops, yes of course it should, will fix.

7. sdaftuar force-pushed on Apr 6, 2018
8. 
   jnewbery commented at 8:09 pm on April 6, 2018: member

   Seems reasonable. I don’t think this can do any harm.

   utACK ccc9031c69e67c336362b197c7decf14af98a877

9. fanquake added the label Tests on Apr 6, 2018
10. 
    MarcoFalke commented at 2:40 pm on April 7, 2018: member

    Might have to set self.process to None in feature_help.py?
11. 
    promag commented at 1:58 pm on April 8, 2018: member

    Tested ACK f1d91ef.

    Using the instructions in #12902 (comment) with this change kills all test processes.

    Code looks good, please squash.

12. 
    jonasschnelli commented at 2:00 pm on April 8, 2018: contributor

    utACK f1d91ef5a2e91c07060b39f0285a733ecf6831b3
13. [qa] Ensure bitcoind processes are cleaned up when tests end e36a0c0852
14. sdaftuar force-pushed on Apr 8, 2018
15. 
    sdaftuar commented at 2:48 pm on April 8, 2018: member

    Squashed f1d91ef -> e36a0c08529bccc695ec71a7ec1df89367cc1628
16. 
    promag commented at 3:24 pm on April 8, 2018: member

    Tested e36a0c0.
17. 
    laanwj commented at 3:40 pm on April 8, 2018: member

    utACK e36a0c08529bccc695ec71a7ec1df89367cc1628
18. laanwj merged this on Apr 8, 2018
19. laanwj closed this on Apr 8, 2018

    ---

20. laanwj referenced this in commit 15c3bb4268 on Apr 8, 2018
21. 
    MarcoFalke commented at 5:23 pm on April 8, 2018: member

    Post-merge tested ACK on Windows e36a0c0
22. MarcoFalke referenced this in commit 0752146e4d on Apr 20, 2018
23. MarcoFalke referenced this in commit 6c26df06ad on Apr 20, 2018
24. HashUnlimited referenced this in commit 9e629dc009 on May 13, 2018
25. laanwj referenced this in commit 531a0337ca on Jun 11, 2018
26. ccebrecos referenced this in commit a272cb997b on Sep 14, 2018
27. schancel referenced this in commit f5873a89b6 on Jun 9, 2019
28. jtoomim referenced this in commit 618005af73 on Jun 29, 2019
29. jonspock referenced this in commit 1ee762d9a4 on Jul 4, 2019
30. jonspock referenced this in commit 2c1056d027 on Jul 4, 2019
31. proteanx referenced this in commit 4d52f0671d on Jul 5, 2019
32. jonspock referenced this in commit 0b4a16fa7a on Jul 9, 2019
33. PastaPastaPasta referenced this in commit 003235b7cc on Mar 14, 2020
34. PastaPastaPasta referenced this in commit 6653068a03 on Mar 19, 2020
35. PastaPastaPasta referenced this in commit 187b2e6f8d on Mar 21, 2020
36. PastaPastaPasta referenced this in commit 7afc672d80 on Mar 24, 2020
37. ckti referenced this in commit 29f9c85d75 on Mar 28, 2021
38. UdjinM6 referenced this in commit 8d22516df1 on Jun 19, 2021
39. UdjinM6 referenced this in commit ad11ad8ec5 on Jun 24, 2021
40. UdjinM6 referenced this in commit 4fb22e4c31 on Jun 26, 2021
41. UdjinM6 referenced this in commit b02642d53f on Jun 26, 2021
42. UdjinM6 referenced this in commit 69fd5dee3d on Jun 26, 2021
43. UdjinM6 referenced this in commit 05dd5c7079 on Jun 28, 2021
44. gades referenced this in commit 82dab91071 on Jun 30, 2021
45. DrahtBot locked this on Sep 8, 2021

Contributors
sdaftuar MarcoFalke laanwj jnewbery promag jonasschnelli

Labels
Tests