From f659d98cc62cf759ca911a22f8c1b36f6331200e Mon Sep 17 00:00:00 2001
From: "Wladimir J. van der Laan" <laanwj@gmail.com>
Date: Wed, 3 Jul 2019 13:49:59 +0200
Subject: [PATCH] Merge #16294: qt: test: Create at most one testing setup

faa1e0fb1712b1f94334e42794163f79988270fd qt: test: Create at most one testing setup (MarcoFalke)

Pull request description:

  It is assumed that ideally only one BasicTestingSetup exists at any point in time for each process (due to use of globals).

  This assumption is violated in the GUI tests, as a testing setup is created as the first step of the `main` function and then (sometimes) another one for the following test cases.

  So, the gui tests create two testing setups:
  * `BasicTestingSetup` in `main` (added in fa4a04a5a942d582c62773d815c7e1e9897975d0)
  * a testing setup for individual test cases

  Avoid that by destructing the testing setup in main after creation and then move the explicit `ECC_Stop` to the only places where it is needed (before and after `apptests`).

ACKs for top commit:
  laanwj:
    code review ACK faa1e0fb1712b1f94334e42794163f79988270fd

Tree-SHA512: b8edceb7e2a8749e1de3ea80bc20b6fb7d4390bf366bb9817206ada3dc8669a91416f4803c22a0e6c636c514e0c858dcfe04523221f8851b10deaf472f107d82
---
 src/qt/test/apptests.cpp                |  5 +++--
 src/qt/test/paymentservertests.cpp      |  4 ++--
 src/qt/test/rpcnestedtests.cpp          |  3 ---
 src/qt/test/test_main.cpp               | 12 +++++++++---
 test/lint/lint-circular-dependencies.sh |  1 +
 5 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp
index a14cc41ac3..938dc983fe 100644
--- a/src/qt/test/apptests.cpp
+++ b/src/qt/test/apptests.cpp
@@ -12,6 +12,8 @@
 #include <qt/networkstyle.h>
 #include <qt/rpcconsole.h>
 #include <shutdown.h>
+#include <test/setup_common.h>
+#include <univalue.h>
 #include <validation.h>
 
 #if defined(HAVE_CONFIG_H)
@@ -31,8 +33,6 @@
 #include <QtGlobal>
 #include <QtTest/QtTestWidgets>
 #include <QtTest/QtTestGui>
-#include <string>
-#include <univalue.h>
 
 namespace {
 //! Call getblockchaininfo RPC and check first field of JSON output.
@@ -67,6 +67,7 @@ void AppTests::appTests()
     }
 #endif
 
+    BasicTestingSetup test{CBaseChainParams::REGTEST}; // Create a temp data directory to backup the gui settings to
     ECC_Stop(); // Already started by the common test setup, so stop it to avoid interference
     LogInstance().DisconnectTestLogger();
 
diff --git a/src/qt/test/paymentservertests.cpp b/src/qt/test/paymentservertests.cpp
index a195ad5aa3..67721bb76f 100644
--- a/src/qt/test/paymentservertests.cpp
+++ b/src/qt/test/paymentservertests.cpp
@@ -13,7 +13,7 @@
 #include <random.h>
 #include <script/script.h>
 #include <script/standard.h>
-#include <util/system.h>
+#include <test/setup_common.h>
 #include <util/strencodings.h>
 
 #include <openssl/x509.h>
@@ -66,7 +66,7 @@ static SendCoinsRecipient handleRequest(PaymentServer* server, std::vector<unsig
 
 void PaymentServerTests::paymentServerTests()
 {
-    SelectParams(CBaseChainParams::MAIN);
+    BasicTestingSetup testing_setup(CBaseChainParams::MAIN);
     auto node = interfaces::MakeNode();
     OptionsModel optionsModel(*node);
     PaymentServer* server = new PaymentServer(nullptr, false);
diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp
index e4fe30ec2e..65f71cb3f5 100644
--- a/src/qt/test/rpcnestedtests.cpp
+++ b/src/qt/test/rpcnestedtests.cpp
@@ -40,9 +40,6 @@ void RPCNestedTests::rpcNestedTests()
     tableRPC.appendCommand("rpcNestedTest", &vRPCCommands[0]);
     //mempool.setSanityCheck(1.0);
 
-    ECC_Stop(); // Already started by the common test setup, so stop it to avoid interference
-    LogInstance().DisconnectTestLogger();
-
     TestingSetup test;
 
     if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished();
diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp
index c0482d6758..c0259de234 100644
--- a/src/qt/test/test_main.cpp
+++ b/src/qt/test/test_main.cpp
@@ -44,12 +44,18 @@ Q_IMPORT_PLUGIN(QCocoaIntegrationPlugin);
 #endif
 #endif
 
-extern void noui_connect();
-
 // This is all you need to run all the tests
 int main(int argc, char *argv[])
 {
-    BasicTestingSetup test{CBaseChainParams::REGTEST};
+    // Initialize persistent globals with the testing setup state for sanity.
+    // E.g. -datadir in gArgs is set to a temp directory dummy value (instead
+    // of defaulting to the default datadir), or globalChainParams is set to
+    // regtest params.
+    //
+    // All tests must use their own testing setup (if needed).
+    {
+        BasicTestingSetup dummy{CBaseChainParams::REGTEST};
+    }
 
     auto node = interfaces::MakeNode();
 
diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh
index 02feca84be..f65d153539 100755
--- a/test/lint/lint-circular-dependencies.sh
+++ b/test/lint/lint-circular-dependencies.sh
@@ -67,6 +67,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
     "llmq/signing -> net_processing -> llmq/signing"
     "llmq/signing_shares -> net_processing -> llmq/signing_shares"
     "logging -> util/system -> logging"
+    "logging -> util/system -> random -> logging"
     "masternode/payments -> validation -> masternode/payments"
     "net -> netmessagemaker -> net"
     "net_processing -> spork -> net_processing"