Merged
Conversation
Contributor
|
utACK 9bb57c926665ecfeb61b185563757b3bd3452cbe |
Contributor
Author
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." 🙂 |
Member
|
utACK. I'm ok with leaving this as seperate commits, however ecfe41d fails on its own: |
Contributor
|
Yeah, separate commits make it easy to review IMO. |
jnewbery
force-pushed
the
improve_node_network_test
branch
from
9bb57c9 to
b9f8840
Compare
Contributor
Author
Thanks @laanwj . Now fixed. |
Member
|
Sorry, needs rebase. Didn't notice there was already a pull open to fixup this test. |
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.
jnewbery
force-pushed
the
improve_node_network_test
branch
from
b9f8840 to
ee5efad
Compare
Contributor
Author
|
rebased |
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. |
maflcko
pushed a commit
that referenced
this pull request
Dec 19, 2017
ee5efad [tests] refactor node_network_limited (John Newbery) b425131 [tests] remove redundant duplicate tests from node_network_limited (John Newbery) 2e02984 [tests] node_network_limited - remove race condition (John Newbery) dbfe294 [tests] define NODE_NETWORK_LIMITED in test framework (John Newbery) 1285312 [tests] fix flake8 warnings in node_network_limited.py (John Newbery) Pull request description: Fixes race condition in the node_network_limited test case introduced in #11740. Also tidies up the test and removes redundant duplicate tests. Tree-SHA512: a5240fe35509d81a47c3d3b141a956378675926093e658d24be43027b20d3b5f0ba7c6017c8208487a1849d4fdfb911a361911d571423db7c50711250aba3011
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Apr 5, 2020
ee5efad [tests] refactor node_network_limited (John Newbery) b425131 [tests] remove redundant duplicate tests from node_network_limited (John Newbery) 2e02984 [tests] node_network_limited - remove race condition (John Newbery) dbfe294 [tests] define NODE_NETWORK_LIMITED in test framework (John Newbery) 1285312 [tests] fix flake8 warnings in node_network_limited.py (John Newbery) Pull request description: Fixes race condition in the node_network_limited test case introduced in bitcoin#11740. Also tidies up the test and removes redundant duplicate tests. Tree-SHA512: a5240fe35509d81a47c3d3b141a956378675926093e658d24be43027b20d3b5f0ba7c6017c8208487a1849d4fdfb911a361911d571423db7c50711250aba3011
gades
pushed a commit
to cosanta/cosanta-core
that referenced
this pull request
Jun 30, 2021
ee5efad [tests] refactor node_network_limited (John Newbery) b425131 [tests] remove redundant duplicate tests from node_network_limited (John Newbery) 2e02984 [tests] node_network_limited - remove race condition (John Newbery) dbfe294 [tests] define NODE_NETWORK_LIMITED in test framework (John Newbery) 1285312 [tests] fix flake8 warnings in node_network_limited.py (John Newbery) Pull request description: Fixes race condition in the node_network_limited test case introduced in bitcoin#11740. Also tidies up the test and removes redundant duplicate tests. Tree-SHA512: a5240fe35509d81a47c3d3b141a956378675926093e658d24be43027b20d3b5f0ba7c6017c8208487a1849d4fdfb911a361911d571423db7c50711250aba3011
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes race condition in the node_network_limited test case introduced in #11740. Also tidies up the test and removes redundant duplicate tests.