Merge #17366: test: Reset global args between test suites

fa07b8beb598642655b1207afd275b801ff8cec2 test: Reset global args between test suites (MarcoFalke)

Pull request description:

  Ideally there wouldn't be any globals in Bitcoin Core. However, as we still have globals, they need to be reset between runs of test cases. One way to do this is to run each suite in a different process. `make check` does that. However, `./src/test/test_bitcoin` when run manually or on appveyor is a single process, where all globals are preserved between test cases.

  This leads to hard to debug issues such as https://github.com/bitcoin/bitcoin/pull/15845#pullrequestreview-310852164.

  Fix that by resetting the global arg for each test suite. Note that this wont reset the arg between test cases, as the constructor/destructor is not called for them.

  Addendum: This is not a general fix, only for `-segwitheight`. I don't know if clearing all args can be done with today's argsmanager.  Nor do I know if it makes sense. Maybe we want datadir set to a temp path to not risk accidentally corrupting the default data dir?

ACKs for top commit:
  laanwj:
    ACK fa07b8beb598642655b1207afd275b801ff8cec2
  practicalswift:
    ACK fa07b8beb598642655b1207afd275b801ff8cec2
  mzumsande:
    ACK fa07b8beb598642655b1207afd275b801ff8cec2, I also tested that this fixes the issue in #15845.

Tree-SHA512: 1e30b06f0d2829144a61cc1bc9bdd6a694cbd911afff83dd3ad2a3f15b577fd30acdf9f1469f8cb724d0642ad5d297364fd5a8a2a9c8619a7a71fa9ae2837cdc

1 files changed, 3 insertions, 4 deletions

diff --git a/src/test/setup_common.cpp b/src/test/setup_common.cpp
index 73517f3f9b..797b72ff59 100644
--- a/src/test/setup_common.cpp
+++ b/src/test/setup_common.cpp
@@ -129,6 +129,7 @@ TestChain100Setup::TestChain100Setup()
     // CreateAndProcessBlock() does not support building SegWit blocks, so don't activate in these tests.
     // TODO: fix the code to support SegWit blocks.
     gArgs.ForceSetArg("-segwitheight", "432");
+    // Need to recreate chainparams
     SelectParams(CBaseChainParams::REGTEST);
 
     // Generate a 100-block chain:
@@ -142,12 +143,9 @@ TestChain100Setup::TestChain100Setup()
     }
 }
 
-//
 // Create a new block with just given transactions, coinbase paying to
 // scriptPubKey, and try to add it to the current chain.
-//
-CBlock
-TestChain100Setup::CreateAndProcessBlock(const std::vector<CMutableTransaction>& txns, const CScript& scriptPubKey)
+CBlock TestChain100Setup::CreateAndProcessBlock(const std::vector<CMutableTransaction>& txns, const CScript& scriptPubKey)
 {
     const CChainParams& chainparams = Params();
     std::unique_ptr<CBlockTemplate> pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey);
@@ -175,6 +173,7 @@ TestChain100Setup::CreateAndProcessBlock(const std::vector<CMutableTransaction>&
 
 TestChain100Setup::~TestChain100Setup()
 {
+    gArgs.ForceSetArg("-segwitheight", "0");
 }