[tests] Make p2p-leaktests.py more robust#11078
Merged
maflcko merged 1 commit intobitcoin:masterfrom Sep 13, 2017
Merged
Conversation
Contributor
Author
|
Happened again on Travis. This seems to have become more common: |
laanwj
reviewed
Aug 17, 2017
test/functional/p2p-leaktests.py
Outdated
Member
There was a problem hiding this comment.
Making things more robust by adding sleeps w/out checks is kind of meh (as things can be terribly laggy). Can it check this precondition somehow and sleep longer if necessary?
jnewbery
force-pushed
the
p2p_leaktests_robust
branch
from
fc70754 to
a53b395
Compare
Contributor
Author
yes, you're right - it's meh, and in this case it was a red herring. I've managed to work out what was going wrong. When the [conn.disconnect_node() for conn in connections]in a reasonably fast environment, the test will continue and add the new allowed_service_bit5_node.add_connection(connections[5])before the old In a slower environment (Travis), the So it's kind of the opposite of what I thought. The solution is to restart the thread with I've also added a |
jnewbery
force-pushed
the
p2p_leaktests_robust
branch
from
a53b395 to
fb56824
Compare
TheBlueMatt
reviewed
Aug 21, 2017
test/functional/p2p-leaktests.py
Outdated
Contributor
There was a problem hiding this comment.
I've seen this test fail locally with high -j on a system with slow I/O at this assert, I assume this patch will help, though I was unable to reproduce reliably to test :(.
jnewbery
force-pushed
the
p2p_leaktests_robust
branch
from
fb56824 to
0063d2c
Compare
Contributor
Author
|
This is still causing occasional failures on Travis. @MarcoFalke - any chance of review/merge? |
Member
|
@jnewbery Thanks for the ping. I ignored the pull because I assumed it still had the ugly |
Member
|
utACK 0063d2c |
maflcko
pushed a commit
that referenced
this pull request
Sep 13, 2017
0063d2c [tests] Make p2p-leaktests.py more robust (John Newbery) Pull request description: There has been an example of p2p-leaktests.py failing on travis in the new service bits test (introduced in #11001 . It appeared to me that the previous p2p connections had not been fully disconnected before attempting to add new p2p connections. I've added a sleep and restarted the NetworkThread, but I don't know whether this will fix the problem, since I'm unable to reproduce the failure locally. @MarcoFalke - not sure what you want to do here? I don't think this change could make things any worse. Tree-SHA512: f5427c26267185a903c9b75bb3925bf153b8afce70c8e493bf8f585f57d809d20643b4ee69081300b211d22e960242aecc3d719f4ddd230aa08fdc5484b55055
maflcko
pushed a commit
to maflcko/bitcoin-core
that referenced
this pull request
Oct 3, 2017
Github-Pull: bitcoin#11078 Rebased-From: 0063d2c
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 10, 2020
0063d2c [tests] Make p2p-leaktests.py more robust (John Newbery) Pull request description: There has been an example of p2p-leaktests.py failing on travis in the new service bits test (introduced in bitcoin#11001 . It appeared to me that the previous p2p connections had not been fully disconnected before attempting to add new p2p connections. I've added a sleep and restarted the NetworkThread, but I don't know whether this will fix the problem, since I'm unable to reproduce the failure locally. @MarcoFalke - not sure what you want to do here? I don't think this change could make things any worse. Tree-SHA512: f5427c26267185a903c9b75bb3925bf153b8afce70c8e493bf8f585f57d809d20643b4ee69081300b211d22e960242aecc3d719f4ddd230aa08fdc5484b55055
gades
pushed a commit
to cosanta/cosanta-core
that referenced
this pull request
Jun 30, 2021
0063d2c [tests] Make p2p-leaktests.py more robust (John Newbery) Pull request description: There has been an example of p2p-leaktests.py failing on travis in the new service bits test (introduced in bitcoin#11001 . It appeared to me that the previous p2p connections had not been fully disconnected before attempting to add new p2p connections. I've added a sleep and restarted the NetworkThread, but I don't know whether this will fix the problem, since I'm unable to reproduce the failure locally. @MarcoFalke - not sure what you want to do here? I don't think this change could make things any worse. Tree-SHA512: f5427c26267185a903c9b75bb3925bf153b8afce70c8e493bf8f585f57d809d20643b4ee69081300b211d22e960242aecc3d719f4ddd230aa08fdc5484b55055
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.
4 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.
There has been an example of p2p-leaktests.py failing on travis in the new service bits test (introduced in #11001 . It appeared to me that the previous p2p connections had not been fully disconnected before attempting to add new p2p connections.
I've added a sleep and restarted the NetworkThread, but I don't know whether this will fix the problem, since I'm unable to reproduce the failure locally.
@MarcoFalke - not sure what you want to do here? I don't think this change could make things any worse.