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

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

Tree-SHA512: 1e30b06f0d2829144a61cc1bc9bdd6a694cbd911afff83dd3ad2a3f15b577fd30acdf9f1469f8cb724d0642ad5d297364fd5a8a2a9c8619a7a71fa9ae2837cdc
This commit is contained in:
MarcoFalke 2019-11-04 14:20:50 -05:00
commit 33b155f287
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25

View file

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