Improve node network test #11867

pull jnewbery wants to merge 5 commits into bitcoin:master from jnewbery:improve_node_network_test changing 2 files +34 −56

jnewbery commented at 6:43 PM on December 11, 2017: member

Fixes race condition in the node_network_limited test case introduced in #11740. Also tidies up the test and removes redundant duplicate tests.
jonasschnelli commented at 6:44 PM on December 11, 2017: contributor

utACK 9bb57c926665ecfeb61b185563757b3bd3452cbe Would prefer a squash.
jonasschnelli added the label Tests on Dec 11, 2017
jnewbery commented at 6:45 PM on December 11, 2017: member

Would prefer a squash.

Was just updating the original comment to say "Split into small commits for ease of review, but can be squashed into a single commit for merge." :slightly_smiling_face:

laanwj commented at 10:41 AM on December 17, 2017: member

utACK.

I'm ok with leaving this as seperate commits, however ecfe41d fails on its own:

2017-12-17 10:40:04.636000 TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
  File "/store/orion/projects/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 118, in main
    self.run_test()
  File "test/functional/node_network_limited.py", line 52, in run_test
    assert_equal(self.getSignaledServiceFlags(), 1036)  # 1036 == 0x40C == 0100 0000 1100
AttributeError: 'NodeNetworkLimitedTest' object has no attribute 'getSignaledServiceFlags'

promag commented at 11:17 AM on December 17, 2017: member

Yeah, separate commits make it easy to review IMO.
jnewbery force-pushed on Dec 18, 2017
jnewbery commented at 1:22 AM on December 18, 2017: member

however ecfe41d fails on its own

Thanks @laanwj . Now fixed.
MarcoFalke commented at 1:19 PM on December 19, 2017: member

Sorry, needs rebase. Didn't notice there was already a pull open to fixup this test.
[tests] fix flake8 warnings in node_network_limited.py 1285312048
[tests] define NODE_NETWORK_LIMITED in test framework dbfe294805

[tests] node_network_limited - remove race condition

node_network_limited had a race condition, since wait_for_block()
doesn't do what you might expect. It only checks the most recent block
received over the P2P interface (perhaps we should rename the method
wait_for_most_recent_block() to avoid future confusion). The test can
fail if the node sends us invs for other blocks, we respond with a
getdata, and the node sends us one of those blocks in the 0.05 second
wait_until loop window.

Fix this by not responding to inv messages with getdata messages.

2e02984591

[tests] remove redundant duplicate tests from node_network_limited b425131f5a
[tests] refactor node_network_limited ee5efad6cf
jnewbery force-pushed on Dec 19, 2017
jnewbery commented at 4:25 PM on December 19, 2017: member

rebased

MarcoFalke commented at 8:16 PM on December 19, 2017: member

Thanks for the quick rebase. Checked that each commit does what it advertises and the tests pass. The refactoring will make the test twice as fast.

MarcoFalke merged this on Dec 19, 2017
MarcoFalke closed this on Dec 19, 2017
MarcoFalke referenced this in commit 18a1bbad98 on Dec 19, 2017
jnewbery deleted the branch on Dec 19, 2017
PastaPastaPasta referenced this in commit ef5919f666 on Apr 4, 2020
PastaPastaPasta referenced this in commit 561451810e on Apr 5, 2020
ckti referenced this in commit 3dbf94324e on Mar 28, 2021
gades referenced this in commit bb8fc00969 on Jun 30, 2021
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#11740 Implement BIP159 NODE_NETWORK_LIMITED (pruned peers) *signaling only*