test: bump test timeouts so that functional tests run in valgrind #17770

 547 | @@ -548,7 +548,7 @@ def run_test(self):
 548 |          tx.rehash()
 549 |          new_txs.append(tx)
 550 |          self.update_block(40, new_txs)
 551 | -        self.send_blocks([b40], success=False, reject_reason='bad-blk-sigops', reconnect=True)
 552 | +        self.send_blocks([b40], success=False, reject_reason='bad-blk-sigops', reconnect=True, timeout=200)

MarcoFalke commented at 3:14 PM on December 18, 2019:

You might just bump the default in def send_blocks(....

However, feature_block.py might be the hardest to get right, as it includes a 1080 block reorg. Not sure how long that will take, but you might want to export TEST_RUNNER_EXTRA="--exclude feature_block for now.

 284 | @@ -285,9 +285,9 @@ def run_test(self):
 285 |  
 286 |          # Make 3 tx and mine them on node 1
 287 |          txs = []
 288 | -        txs.append(self.nodes[0].sendtoaddress(not_related_address, 11))
 289 | -        txs.append(self.nodes[0].sendtoaddress(not_related_address, 11))
 290 | -        txs.append(self.nodes[0].sendtoaddress(not_related_address, 11))
 291 | +        txs.append(self.nodes[0].sendtoaddress(not_related_address, 11, timeout=120))
 292 | +        txs.append(self.nodes[0].sendtoaddress(not_related_address, 11, timeout=120))
 293 | +        txs.append(self.nodes[0].sendtoaddress(not_related_address, 11, timeout=120))

MarcoFalke commented at 5:03 PM on December 19, 2019:

rpc calls don't have a timeout argument.

Also, this test properly failed with an uninitialized read:

 node0 stderr ==27834== Thread 7 b-httpworker.2:

==27834== Conditional jump or move depends on uninitialised value(s)

==27834==    at 0x204563: rest_blockhash_by_height(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (rest.cpp:611)

==27834==    by 0x20B4C8: std::_Function_handler<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&), bool (*)(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>::_M_invoke(std::_Any_data const&, HTTPRequest*&&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (std_function.h:301)

==27834==    by 0x3B90B4: operator() (std_function.h:706)

==27834==    by 0x3B90B4: HTTPWorkItem::operator()() (httpserver.cpp:55)

==27834==    by 0x3B988D: WorkQueue<HTTPClosure>::Run() (httpserver.cpp:114)

==27834==    by 0x3B5968: HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int) (httpserver.cpp:342)

==27834==    by 0x3BA1CA: __invoke_impl<void, void (*)(WorkQueue<HTTPClosure> *, int), WorkQueue<HTTPClosure> *, int> (invoke.h:60)

==27834==    by 0x3BA1CA: __invoke<void (*)(WorkQueue<HTTPClosure> *, int), WorkQueue<HTTPClosure> *, int> (invoke.h:95)

==27834==    by 0x3BA1CA: _M_invoke<0, 1, 2> (thread:234)

==27834==    by 0x3BA1CA: operator() (thread:243)

==27834==    by 0x3BA1CA: std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int> > >::_M_run() (thread:186)

==27834==    by 0x662A66E: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)

==27834==    by 0x54876DA: start_thread (pthread_create.c:463)

==27834==    by 0x6FCD88E: clone (clone.S:95)

==27834== 

{
   Temporary suppression of uninitialized read in rest code
   Memcheck:Cond
   fun:_ZL24rest_blockhash_by_heightP11HTTPRequestRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
} 

Not sure how that could have happened, so you might want to add the suppression for now to the valgrind.supp file

practicalswift commented at 9:31 PM on December 19, 2019:

Hrmm, that is super weird. Valgrind false positives are very rare, but I'm unable to reproduce. How the heck was this triggered? :)

   4 | @@ -5,11 +5,10 @@
   5 |  # file COPYING or http://www.opensource.org/licenses/mit-license.php.
   6 |  
   7 |  export LC_ALL=C.UTF-8
   8 | -
   9 | +export TEST_RUNNER_EXTRA="--exclude feature_block"

MarcoFalke commented at 6:27 PM on December 19, 2019:

temporarily, for faster ci runs, you can also export RUN_UNIT_TESTS=false

 163 | @@ -164,7 +164,7 @@ def run_test(self):
 164 |          # found with or without /checkmempool.
 165 |  
 166 |          # do a tx and don't sync
 167 | -        txid = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 0.1)
 168 | +        txid = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 0.1, timeout=120)

MarcoFalke commented at 2:09 PM on December 20, 2019:

As mentioned previously, RPCs don't have timeout argument

   4 | @@ -5,11 +5,11 @@
   5 |  # file COPYING or http://www.opensource.org/licenses/mit-license.php.
   6 |  
   7 |  export LC_ALL=C.UTF-8
   8 | -
   9 | +export TEST_RUNNER_EXTRA="--exclude feature_block"

MarcoFalke commented at 12:19 PM on December 25, 2019:

If the feature_abortnode test is causing issues that are non-trivial to fix, you might exclude it for now:

export TEST_RUNNER_EXTRA="--exclude feature_block,feature_abortnode"

   4 | @@ -5,11 +5,11 @@
   5 |  # file COPYING or http://www.opensource.org/licenses/mit-license.php.
   6 |  
   7 |  export LC_ALL=C.UTF-8
   8 | -
   9 | +export TEST_RUNNER_EXTRA="--exclude feature_block,feature_abortnode,wallet_createwallet"

MarcoFalke commented at 11:46 AM on January 3, 2020:

export TEST_RUNNER_EXTRA="--exclude feature_block,rpc_bind.py --ipv4,feature_abortnode,wallet_createwallet"

Due to failure here: https://travis-ci.org/bitcoin/bitcoin/jobs/632228754#L2879

  39 | @@ -40,9 +40,9 @@ def run_test(self):
  40 |  
  41 |              # Check that node0 aborted
  42 |              self.log.info("Waiting for crash")
  43 | -            wait_until(lambda: self.nodes[0].is_node_stopped(), timeout=60)
  44 | +            wait_until(lambda: self.nodes[0].is_node_stopped(), timeout=200)
  45 |          self.log.info("Node crashed - now verifying restart fails")
  46 | -        self.nodes[0].assert_start_raises_init_error()
  47 | +        self.nodes[0].assert_start_raises_init_error(timeout=200)

MarcoFalke commented at 11:47 AM on January 3, 2020:

This does not have a timeout argument. Please reset the change.

   4 | @@ -5,11 +5,11 @@
   5 |  # file COPYING or http://www.opensource.org/licenses/mit-license.php.
   6 |  
   7 |  export LC_ALL=C.UTF-8
   8 | -
   9 | +export TEST_RUNNER_EXTRA="--exclude feature_block,rpc_bind,feature_abortnode,wallet_createwallet"

MarcoFalke commented at 7:41 PM on January 14, 2020:

export TEST_RUNNER_EXTRA="--exclude feature_block,rpc_bind,feature_abortnode,wallet_createwallet,wallet_multiwallet"

Maybe exclude this for now due to the failure https://travis-ci.org/bitcoin/bitcoin/jobs/636719655#L4607

   4 | @@ -5,11 +5,10 @@
   5 |  # file COPYING or http://www.opensource.org/licenses/mit-license.php.
   6 |  
   7 |  export LC_ALL=C.UTF-8
   8 | -
   9 | +export TEST_RUNNER_EXTRA="--exclude feature_block,rpc_bind,feature_assumevalid,feature_cltv,feature_abortnode,wallet_createwallet,wallet_multiwallet"

MarcoFalke commented at 9:50 PM on January 23, 2020:

ok, then maybe revert the changes to this file altogether, but I'd still like to get this merged. The timeout changes help me running the valgrind tests on my own ci infrastructure. We can look into how to enable them on travis later on.

   4 | @@ -5,11 +5,9 @@
   5 |  # file COPYING or http://www.opensource.org/licenses/mit-license.php.
   6 |  
   7 |  export LC_ALL=C.UTF-8
   8 | -
   9 |  export PACKAGES="valgrind clang llvm python3-zmq libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev"
  10 |  export USE_VALGRIND=1
  11 |  export NO_DEPENDS=1
  12 | -export TEST_RUNNER_EXTRA="p2p_segwit.py"  # Only run one test for now. TODO enable all and bump timeouts

MarcoFalke commented at 2:05 PM on January 24, 2020:

please reset all changes in this file to restore the inital state of the travis config