test: Avoid intermittent test failure in feature_csv_activation.py #22907
Conversation
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
Member
Author
|
Larger excerpt: |
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
Contributor
|
Concept ACK |
Member
|
It would make sense to separate the styling changes from the behavior change into separate commits. Concept ACK |
Member
Author
|
Makes sense. Done. |
maflcko
force-pushed
the
2109-testFix
branch
from
fad5cb0 to
fa676db
Compare
Contributor
There was a problem hiding this comment.
tAck fa676db
peertimeout will fix the test failure for now as it overrides the default timeout of 60 seconds
Tested on Ubuntu 21.02
nina1234
approved these changes
Sep 9, 2021
fanquake
approved these changes
Sep 10, 2021
| self.num_nodes = 1 | ||
| self.setup_clean_chain = True | ||
| self.extra_args = [[ | ||
| '-peertimeout=999999', # bump because mocktime might cause a disconnect otherwise |
Member
There was a problem hiding this comment.
I take it this needs to be an even larger timeout than we currently use in a few other places, i.e: "-peertimeout=9999", # bump because mocktime might cause a disconnect otherwise? Otherwise would be good to settle on the same LARGE number everywhere.
Member
Author
There was a problem hiding this comment.
It only needs to be 1 second longer than the whole duration of the test. If the test takes 100 seconds, it needs to be 101. So 9999 seems enough for our purposes here.
Member
Author
There was a problem hiding this comment.
Maybe peertimeout should be set to ~infinity by default in our functional tests, unless opted out?
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Sep 11, 2021
…_csv_activation.py fa676db test: pep-8 whitespace (MarcoFalke) faed284 test: Avoid intermittent test failure in feature_csv_activation.py (MarcoFalke) Pull request description: Otherwise there will be disconnects if the test runs longer than the default peertimeout (60s): ``` node0 2021-09-05T20:28:30.973116Z (mocktime: 2021-09-01T07:17:29Z) [net] [net.cpp:1323] [InactivityCheck] socket receive timeout: 393061s peer=0 ``` Fix that by skipping `InactivityCheck` via a large `-peertimeout`. ACKs for top commit: fanquake: ACK fa676db Tree-SHA512: 061c0585a805aa2f8e55c4beedd4b8498a2951f33d60aa3632dda0a284db3a627d14a23dbd57e8a66c69a1612f39418e3a755c8ca97f6ae1105c0d70f0d1a801
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 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.
Otherwise there will be disconnects if the test runs longer than the default peertimeout (60s):
Fix that by skipping
InactivityCheckvia a large-peertimeout.