[Tests] Fix fundrawtransaction feerate test #8200

pull jonasschnelli wants to merge 1 commits into bitcoin:master from jonasschnelli:2016/06/fix_frt_test changing 1 files +6 −2

jonasschnelli commented at 1:56 PM on June 13, 2016: contributor

Different fundrawtransaction calls can result in selecting different inputs which can break/fail the test. This commit reduces the test strictness.

Alternative solution for #8199.
jonasschnelli added the label Tests on Jun 13, 2016

sipa commented at 1:57 PM on June 13, 2016: member

This doesn't fix anything. The failure I see is

$ ./fundrawtransaction.py 
Initializing test directory /tmp/test7pcerde1/4231
Mining blocks...
JSONRPC error: Insufficient funds
  File "/home/pw/git/bitcoin/qa/rpc-tests/test_framework/test_framework.py", line 144, in main
    self.run_test()
  File "./fundrawtransaction.py", line 684, in run_test
    result = self.nodes[3].fundrawtransaction(rawtx) # uses min_relay_tx_fee (set by settxfee)
  File "/home/pw/git/bitcoin/qa/rpc-tests/test_framework/coverage.py", line 49, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/home/pw/git/bitcoin/qa/rpc-tests/test_framework/authproxy.py", line 144, in __call__
    raise JSONRPCException(response['error'])
Stopping nodes
Not cleaning up dir /tmp/test7pcerde1/4231
Failed

[Tests] Fix fundrawtransaction feerate test

Different fundrawtransaction calls can result in selecting different inputs which can break/fail the test.

This commit reduces the test strictness.

140cfa10e0

jonasschnelli force-pushed on Jun 13, 2016
jonasschnelli commented at 2:03 PM on June 13, 2016: contributor
Updated the PR. Could be a timing issues (locked inputs / synced mempools, etc.). Added sync/generate before the new tests.
```
        self.sync_all()
        self.nodes[0].generate(1)
        self.sync_all()
```
sipa commented at 2:09 PM on June 13, 2016: member

Tested ACK
in qa/rpc-tests/fundrawtransaction.py:None in 140cfa10e0
```
 677 | @@ -678,14 +678,18 @@ def run_test(self):
 678 |          assert(signedtx["complete"])
 679 |          self.nodes[0].sendrawtransaction(signedtx["hex"])
 680 |  
 681 | +        self.sync_all()
```
MarcoFalke commented at 2:10 PM on June 13, 2016:

Nit: You don't need the sync when you mine on the node where you broadcast the transaction.

MarcoFalke commented at 2:11 PM on June 13, 2016:

I think we should get rid of such sync_all() as they pause the test suite quite a bit.

jonasschnelli commented at 2:32 PM on June 13, 2016:

I guess there are many places where we do this. What about fixing all together in an upcoming PR?

MarcoFalke commented at 3:10 PM on June 13, 2016:

Sure, but let's not make it worse if there is no reason to do so.
jonasschnelli commented at 6:55 AM on June 14, 2016: contributor

Closing in favor or #8201
jonasschnelli closed this on Jun 14, 2016
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#8199 Avoid test bug introduced in #7967 #8201 [qa] fundrawtransaction: Fix race, assert amounts