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

fa07b8b 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 bitcoin/bitcoin#15845 (review).

  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 fa07b8b
  practicalswift:
    ACK fa07b8b
  mzumsande:
    ACK fa07b8b, I also tested that this fixes the issue in #15845.

Tree-SHA512: 1e30b06f0d2829144a61cc1bc9bdd6a694cbd911afff83dd3ad2a3f15b577fd30acdf9f1469f8cb724d0642ad5d297364fd5a8a2a9c8619a7a71fa9ae2837cdc

Author: MarcoFalke

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");
 }