Merge #19730: ci: Set increased --timeout-factor by default

fa330ec2fe test: Remove confusing and broken use of wait_until global (MarcoFalke)
fa6583c30b ci: Set increased --timeout-factor by default (MarcoFalke)

Pull request description:

  Assuming that tests don't have a logic error or race, setting a high timeout should not cause any issues. The tests will still pass just as fast in the fastest case, but it allows for some buffer in case of slow disks or otherwise starved ci machines.

  Fixes #19729

ACKs for top commit:
  hebasto:
    ACK fa330ec2fe, I have reviewed the code, and it looks OK, I agree it can be merged.

Tree-SHA512: 3da80ee008c7b08bab5fdaf7804d57c79d6fed49a7d37b9c54fc89756659fcb9981fd10afc4d07bd90d93c1699fd410a0110a3cd34d016873759d114ce3cd82d

ci/test/00_setup_env.sh

@@ -35,6 +35,10 @@ export USE_BUSY_BOX=${USE_BUSY_BOX:-false}
 export RUN_UNIT_TESTS=${RUN_UNIT_TESTS:-true}
 export RUN_FUNCTIONAL_TESTS=${RUN_FUNCTIONAL_TESTS:-true}
 export RUN_SECURITY_TESTS=${RUN_SECURITY_TESTS:-false}
+# By how much to scale the test_runner timeouts (option --timeout-factor).
+# This is needed because some ci machines have slow CPU or disk, so sanitizers
+# might be slow or a reindex might be waiting on disk IO.
+export TEST_RUNNER_TIMEOUT_FACTOR=${TEST_RUNNER_TIMEOUT_FACTOR:-4}
 export TEST_RUNNER_ENV=${TEST_RUNNER_ENV:-}
 export RUN_FUZZ_TESTS=${RUN_FUZZ_TESTS:-false}
 export EXPECTED_TESTS_DURATION_IN_SECONDS=${EXPECTED_TESTS_DURATION_IN_SECONDS:-1000}

ci/test/00_setup_env_native_asan.sh

@@ -10,6 +10,5 @@ export CONTAINER_NAME=ci_native_asan
 export PACKAGES="clang llvm python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev"
 export DOCKER_NAME_TAG=ubuntu:20.04
 export NO_DEPENDS=1
-export TEST_RUNNER_EXTRA="--timeout-factor=4"  # Increase timeout because sanitizers slow down
 export GOAL="install"
 export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' --with-sanitizers=address,integer,undefined CC=clang CXX=clang++ --with-boost-process"

ci/test/00_setup_env_native_tsan.sh

@@ -10,6 +10,6 @@ export CONTAINER_NAME=ci_native_tsan
 export DOCKER_NAME_TAG=ubuntu:20.04
 export PACKAGES="clang llvm libc++abi-dev libc++-dev python3-zmq"
 export DEP_OPTS="CC=clang CXX='clang++ -stdlib=libc++'"
-export TEST_RUNNER_EXTRA="--exclude feature_block --timeout-factor=4"  # Increase timeout because sanitizers slow down. Low memory on Travis machines, exclude feature_block.
+export TEST_RUNNER_EXTRA="--exclude feature_block"  # Low memory on Travis machines, exclude feature_block.
 export GOAL="install"
 export BITCOIN_CONFIG="--enable-zmq --with-gui=no CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' CXXFLAGS='-g' --with-sanitizers=thread CC=clang CXX='clang++ -stdlib=libc++' --with-boost-process"

ci/test/00_setup_env_native_valgrind.sh

@@ -10,6 +10,6 @@ export CONTAINER_NAME=ci_native_valgrind
 export PACKAGES="valgrind clang llvm python3-zmq libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev"
 export USE_VALGRIND=1
 export NO_DEPENDS=1
-export TEST_RUNNER_EXTRA="--exclude rpc_bind --timeout-factor=4"  # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547
+export TEST_RUNNER_EXTRA="--exclude rpc_bind"  # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547
 export GOAL="install"
 export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=no CC=clang CXX=clang++"  # TODO enable GUI

ci/test/06_script_b.sh

@@ -35,7 +35,7 @@ fi
 
 if [ "$RUN_FUNCTIONAL_TESTS" = "true" ]; then
   BEGIN_FOLD functional-tests
-  DOCKER_EXEC LD_LIBRARY_PATH=$DEPENDS_DIR/$HOST/lib ${TEST_RUNNER_ENV} test/functional/test_runner.py --ci $MAKEJOBS --tmpdirprefix "${BASE_SCRATCH_DIR}/test_runner/" --ansi --combinedlogslen=4000 ${TEST_RUNNER_EXTRA} --quiet --failfast
+  DOCKER_EXEC LD_LIBRARY_PATH=$DEPENDS_DIR/$HOST/lib ${TEST_RUNNER_ENV} test/functional/test_runner.py --ci $MAKEJOBS --tmpdirprefix "${BASE_SCRATCH_DIR}/test_runner/" --ansi --combinedlogslen=4000 --timeout-factor=${TEST_RUNNER_TIMEOUT_FACTOR} ${TEST_RUNNER_EXTRA} --quiet --failfast
   END_FOLD
 fi
 

test/functional/wallet_basic.py

@@ -12,7 +12,6 @@ from test_framework.util import (
     assert_fee_amount,
     assert_raises_rpc_error,
     connect_nodes,
-    wait_until,
 )
 from test_framework.wallet_util import test_address
 
@@ -540,7 +539,7 @@ class WalletTest(BitcoinTestFramework):
             self.start_node(2, [m, "-limitancestorcount=" + str(chainlimit)])
             if m == '-reindex':
                 # reindex will leave rpc warm up "early"; Wait for it to finish
-                wait_until(lambda: [block_count] * 3 == [self.nodes[i].getblockcount() for i in range(3)])
+                self.wait_until(lambda: [block_count] * 3 == [self.nodes[i].getblockcount() for i in range(3)])
             assert_equal(balance_nodes, [self.nodes[i].getbalance() for i in range(3)])
 
         # Exercise listsinceblock with the last two blocks
@@ -589,7 +588,7 @@ class WalletTest(BitcoinTestFramework):
         self.start_node(0, extra_args=extra_args)
 
         # wait until the wallet has submitted all transactions to the mempool
-        wait_until(lambda: len(self.nodes[0].getrawmempool()) == chainlimit * 2)
+        self.wait_until(lambda: len(self.nodes[0].getrawmempool()) == chainlimit * 2)
 
         node0_balance = self.nodes[0].getbalance()
         # With walletrejectlongchains we will not create the tx and store it in our wallet.