[tests] fix timeout issues from TestNode #11077
Merged
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
|
utACK, needs rebase |
Fixes a couple of bugs from the introduction of TestNode: - test scripts were no longer able to specify a custom timeout for starting a node. Therefore tests with nodes that take a long time to start up (eg pruning.py) would fail. - the test for whether a node has failed on start up was broken by changing 'assert x is None' to 'assert not x'. Since subprocess.poll() can return None (indicating the node is still running) or 0 (indicating the node exited with return code 0), this was a regression.
jnewbery
force-pushed
the
test_node_fixes
branch
from
cf2afb2 to
2b4ea52
Compare
Contributor
Author
|
Thanks @laanwj . Rebased |
Contributor
|
Looks good, also ran tests locally |
laanwj
added a commit
that referenced
this pull request
Aug 23, 2017
2b4ea52 [tests] fix timeout issues from TestNode (John Newbery) Pull request description: Fixes a couple of bugs from the introduction of TestNode: - test scripts were no longer able to specify a custom timeout for starting a node. Therefore tests with nodes that take a long time to start up (eg pruning.py) would fail. - the test for whether a node has failed on start up was broken by changing 'assert x is None' to 'assert not x'. Since subprocess.poll() can return None (indicating the node is still running) or 0 (indicating the node exited with return code 0), this was a regression. Tree-SHA512: 42a62a5459eea2e5d83b44dae2a5ccc7b15eb7fef8f8745ff04884dbba8f79d66ffdd65c67d37f6865b36da3f522bcdd0d6ea99861d7ce86dd8a56dc29cd643f
Merged
Member
|
post merge utACK 2b4ea52 |
maflcko
reviewed
Aug 30, 2017
| self.rpc_timeout = timewait | ||
| else: | ||
| # Wait for up to 60 seconds for the RPC server to respond | ||
| self.rpc_timeout = 60 |
Member
There was a problem hiding this comment.
Previously we used a default of HTTP_TIMEOUT = 30 (authproxy)
Why was this changed?
Contributor
Author
There was a problem hiding this comment.
Longer is better (I think?). Travis is so unpredictable that one minute seemed appropriate.
We can tune it back down if you think that's better.
maflcko
pushed a commit
that referenced
this pull request
Sep 1, 2017
7148b74 [tests] Functional tests must explicitly set num_nodes (John Newbery) 5448a14 [tests] don't override __init__() in individual tests (John Newbery) 6cf094a [tests] Avoid passing around member variables in test_framework (John Newbery) 36b6268 [tests] TestNode: separate add_node from start_node (John Newbery) be2a2ab [tests] fix - use rpc_timeout as rpc timeout (John Newbery) Pull request description: Some additional tidyups after the introduction of TestNode: - commit 1 makes TestNode use the correct rpc timeout. This should have been included in #11077 - commit 2 separates `add_node()` from `start_node()` as originally discussed here: #10556 (comment) with @kallewoof . The test writer no longer needs to assign to `self.nodes` when starting/stopping nodes. - commit 3 adds a `set_test_params()` method, so individual tests don't need to override `__init__()` and call `super().__init__()` Tree-SHA512: 0adb030623b96675b5c29e2890ce99ccd837ed05f721d0c91b35378c5ac01b6658174aac12f1f77402e1d38b61f39b3c43b4df85c96952565dde1cda05b0db84
maflcko
pushed a commit
to maflcko/bitcoin-core
that referenced
this pull request
Oct 3, 2017
Fixes a couple of bugs from the introduction of TestNode: - test scripts were no longer able to specify a custom timeout for starting a node. Therefore tests with nodes that take a long time to start up (eg pruning.py) would fail. - the test for whether a node has failed on start up was broken by changing 'assert x is None' to 'assert not x'. Since subprocess.poll() can return None (indicating the node is still running) or 0 (indicating the node exited with return code 0), this was a regression. Github-Pull: bitcoin#11077 Rebased-From: 2b4ea52
codablock
pushed a commit
to codablock/dash
that referenced
this pull request
Sep 24, 2019
2b4ea52 [tests] fix timeout issues from TestNode (John Newbery) Pull request description: Fixes a couple of bugs from the introduction of TestNode: - test scripts were no longer able to specify a custom timeout for starting a node. Therefore tests with nodes that take a long time to start up (eg pruning.py) would fail. - the test for whether a node has failed on start up was broken by changing 'assert x is None' to 'assert not x'. Since subprocess.poll() can return None (indicating the node is still running) or 0 (indicating the node exited with return code 0), this was a regression. Tree-SHA512: 42a62a5459eea2e5d83b44dae2a5ccc7b15eb7fef8f8745ff04884dbba8f79d66ffdd65c67d37f6865b36da3f522bcdd0d6ea99861d7ce86dd8a56dc29cd643f
codablock
pushed a commit
to codablock/dash
that referenced
this pull request
Sep 24, 2019
7148b74 [tests] Functional tests must explicitly set num_nodes (John Newbery) 5448a14 [tests] don't override __init__() in individual tests (John Newbery) 6cf094a [tests] Avoid passing around member variables in test_framework (John Newbery) 36b6268 [tests] TestNode: separate add_node from start_node (John Newbery) be2a2ab [tests] fix - use rpc_timeout as rpc timeout (John Newbery) Pull request description: Some additional tidyups after the introduction of TestNode: - commit 1 makes TestNode use the correct rpc timeout. This should have been included in bitcoin#11077 - commit 2 separates `add_node()` from `start_node()` as originally discussed here: bitcoin#10556 (comment) with @kallewoof . The test writer no longer needs to assign to `self.nodes` when starting/stopping nodes. - commit 3 adds a `set_test_params()` method, so individual tests don't need to override `__init__()` and call `super().__init__()` Tree-SHA512: 0adb030623b96675b5c29e2890ce99ccd837ed05f721d0c91b35378c5ac01b6658174aac12f1f77402e1d38b61f39b3c43b4df85c96952565dde1cda05b0db84
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.
Fixes a couple of bugs from the introduction of TestNode:
starting a node. Therefore tests with nodes that take a long time to
start up (eg pruning.py) would fail.
by changing 'assert x is None' to 'assert not x'. Since
subprocess.poll() can return None (indicating the node is still running)
or 0 (indicating the node exited with return code 0), this was a
regression.