test: fix intermittent failure in p2p_ibd_txrelay #19552

pull jonatack wants to merge 1 commits into bitcoin:master from jonatack:fix-intermittent-p2p_ibd_txrelay-failure changing 2 files +7 −5

jonatack commented at 6:49 AM on July 19, 2020: member

To fix these intermittent failures in Travis CI.

162/163 - p2p_ibd_txrelay.py failed, Duration: 2 s

stdout:
2020-07-19T05:44:17.213000Z TestFramework (INFO):
    Check that nodes set minfilter to MAX_MONEY while still in IBD
2020-07-19T05:44:17.216000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/Users/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-x86_64-apple-darwin16/test/functional/test_framework/test_framework.py", line 117, in main
    self.run_test()
  File "/Users/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-x86_64-apple-darwin16/test/functional/p2p_ibd_txrelay.py", line 30, in run_test
    assert_equal(conn_info['minfeefilter'], MAX_FEE_FILTER)
  File "/Users/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-x86_64-apple-darwin16/test/functional/test_framework/util.py", line 49, in assert_equal
    raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))

AssertionError: not(0E-8 == 0.09170997)
2020-07-19T05:44:17.293000Z TestFramework (INFO): Stopping nodes

At Marco's suggestion, cherry-picked part of #19134 to nicely simplify using wait_until.

fanquake added the label Tests on Jul 19, 2020
in test/functional/p2p_ibd_txrelay.py:27 in 41dfdac305 outdated
```
  23 | @@ -24,6 +24,7 @@ def set_test_params(self):
  24 |          ]
  25 |      def run_test(self):
  26 |          self.log.info("Check that nodes set minfilter to MAX_MONEY while still in IBD")
  27 | +        self.sync_all()
```
MarcoFalke commented at 7:37 AM on July 19, 2020:

There are no blocks or transactions to sync, so sync_all should be a no-op.

I presume the issue is that the send messages loop for the peer hasn't been called (yet). This could be fixed by forcing a ping (with the RPC ping) and then waiting for it to propagate, I think.

jonatack commented at 9:08 AM on July 19, 2020:

Good point, thanks! Trying a simple solution with 14bb388.
jonatack force-pushed on Jul 19, 2020
jonatack force-pushed on Jul 19, 2020
in test/functional/p2p_ibd_txrelay.py:35 in 14bb3881fc outdated
```
  30 |          self.log.info("Check that nodes set minfilter to MAX_MONEY while still in IBD")
  31 |          for node in self.nodes:
  32 |              assert node.getblockchaininfo()['initialblockdownload']
  33 | +            wait_until(lambda: (conn_info['minfeefilter'] != 0 for conn_info in node.getpeerinfo()))
  34 |              for conn_info in node.getpeerinfo():
  35 |                  assert_equal(conn_info['minfeefilter'], MAX_FEE_FILTER)
```
jonatack commented at 9:23 AM on July 19, 2020:

Could also replace the above 3 lines with the following, but it wouldn't be testing exactly the same thing.

wait_until(lambda: (conn_info['minfeefilter'] == MAX_FEE_FILTER for conn_info in node.getpeerinfo()))
MarcoFalke commented at 10:39 AM on July 19, 2020:

A tuple with more than one element always evaluates to true, so your exact suggestion wouldn't work, but Concept ACK replacing this with just wait_until

jonatack commented at 11:35 AM on July 19, 2020:

Thank you -- your comment made me realise that there is Python any(iterable) like the Ruby Enumerator#all? I was familiar with. Mimicked the way it is used in test/functional/test_framework/util.py:443-444 and applied to both assertions for symmetry and safety.

jonatack commented at 11:40 AM on July 19, 2020:

That also allowed no longer importing anything from test_framework.util :+1:
in test/functional/p2p_ibd_txrelay.py:13 in 14bb3881fc outdated
```
   7 | @@ -8,7 +8,10 @@
   8 |  
   9 |  from test_framework.messages import COIN
  10 |  from test_framework.test_framework import BitcoinTestFramework
  11 | -from test_framework.util import assert_equal
  12 | +from test_framework.util import (
  13 | +    assert_equal,
  14 | +    wait_until,
```
MarcoFalke commented at 10:37 AM on July 19, 2020:

unrelated style nit: If you want you can cherry-pick the change to the test framework from https://github.com/bitcoin/bitcoin/pull/19134/files#diff-64721c5ee64d44f7114d6d0d2226db4d and then call self.wait_until below instead of the import.

jonatack commented at 11:25 AM on July 19, 2020:

nice; done
jonatack force-pushed on Jul 19, 2020
test: fix intermittent p2p_ibd_txrelay race, add test_framework.py#wait_until 12410b1feb
jonatack force-pushed on Jul 19, 2020
vasild approved
vasild commented at 1:54 PM on July 21, 2020: member
ACK 12410b1fe

Sporadic test failures are one of the worst things that can happen to a software project.

I tried to reproduce the failure locally, but without success (there you go!). The following passes with and without the patch for me:
```
./test/functional/test_runner.py $(for i in {1..100} ; do echo p2p_ibd_txrelay.py ; done)
```
Anyway - the patch looks good.
MarcoFalke merged this on Jul 21, 2020
MarcoFalke closed this on Jul 21, 2020
jonatack deleted the branch on Jul 21, 2020
Fabcien referenced this in commit b5deb4270f on Sep 2, 2021
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#19134 test: Replace global wait_until with BitcoinTestFramework.wait_until and mininode.wait_until