test: Fix intermittent wallet_multiwallet issue with got_loading_error #20569
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
fa08f12 to
fab48da
Compare
ryanofsky
approved these changes
Dec 16, 2020
There was a problem hiding this comment.
Code review ACK fab48da. This seems like a good workaround. I think more ideally think load and unload RPCs would not have racy status reporting (suggested previously #19300 (review) and
#19232 (comment)) so the race condition that this test tries to trigger just wouldn't exist.
Also for reference there was discussion about fragility of this test when it was added #19300 (comment)
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Dec 17, 2020
…with got_loading_error fab48da test: Fix intermittent wallet_multiwallet issue with got_loading_error (MarcoFalke) fa8e15f test: pep8 wallet_multiwallet.py (MarcoFalke) Pull request description: Failing the test after 10 iterations without a loading error is problematic because it may take 11 iterations to get a loading error. Fix that by running until a loading error occurs, which should happen in almost all runs within the first 10 iterations. ACKs for top commit: ryanofsky: Code review ACK fab48da. This seems like a good workaround. I think more ideally think load and unload RPCs would not have racy status reporting (suggested previously bitcoin#19300 (review) and Tree-SHA512: 6b80b26d916276efe2a01af93bca7dbf71a3e67db9d3deb15175070719bf7d1325a1410d93e74c0316942e388faa2ba185dc9d3759c82d1c73c3c509b9997f05
|
Doesn't that introduce a risk that one of the 3 threads never detects a race and keeps running forever alone without interference from the other two? |
|
nvm, I missed the fact that |
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Dec 20, 2021
Summary: Failing the test after 10 iterations without a loading error is problematic because it may take 11 iterations to get a loading error. Fix that by running until a loading error occurs, which should happen in almost all runs within the first 10 iterations. This is a backport of [[bitcoin/bitcoin#20569 | core#20569]] Test Plan: `ninja check-functional` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D10684
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Failing the test after 10 iterations without a loading error is problematic because it may take 11 iterations to get a loading error.
Fix that by running until a loading error occurs, which should happen in almost all runs within the first 10 iterations.