Tests: Add logging in loops in p2p_sendhears.py #12849
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
maflcko
reviewed
Mar 31, 2018
test/functional/p2p_sendheaders.py
Outdated
| @@ -380,6 +380,8 @@ def test_nonnull_locators(self, test_node, inv_node): | |||
| # PART 3. Headers announcements can stop after large reorg, and resume after | |||
| # getheaders or inv from peer. | |||
| for j in range(2): | |||
| self.log.info("Part 3: iteration level: {}".format(j)) | |||
|
|
|||
There was a problem hiding this comment.
I'd slightly prefer if the comments in the loops down there were transformed into debug statements. (Where we check for i == ... and j == ...)
maflcko
reviewed
Mar 31, 2018
test/functional/p2p_sendheaders.py
Outdated
| @@ -406,6 +408,8 @@ def test_nonnull_locators(self, test_node, inv_node): | |||
| test_node.wait_for_block(new_block_hashes[-1]) | |||
|
|
|||
| for i in range(3): | |||
| self.log.debug("Part 3: i == {}".format(i)) | |||
|
|
|||
| # Mine another block, still should get only an inv | |||
There was a problem hiding this comment.
What I meant was to transfer those comments from # ... to self.log.debug(...
There was a problem hiding this comment.
Ahh sorry, I think I get what you mean now
jnewbery
reviewed
Apr 2, 2018
test/functional/p2p_sendheaders.py
Outdated
| @@ -381,7 +381,7 @@ def test_nonnull_locators(self, test_node, inv_node): | |||
| # getheaders or inv from peer. | |||
| for j in range(2): | |||
| # First try mining a reorg that can propagate with header announcement | |||
| new_block_hashes = self.mine_reorg(length=7) | |||
| new_block_hashes = self.mine_reorg(length=7) | |||
There was a problem hiding this comment.
nit: you've added trailing whitespace here.
test/functional/p2p_sendheaders.py
Outdated
| @@ -412,11 +412,13 @@ def test_nonnull_locators(self, test_node, inv_node): | |||
| test_node.check_last_announcement(inv=[tip], headers=[]) | |||
| if i == 0: | |||
| # Just get the data -- shouldn't cause headers announcements to resume | |||
There was a problem hiding this comment.
You can remove this comment now that you have a log saying the same thing!
Same for comments lower down.
There was a problem hiding this comment.
Thanks @jnewbery, I appreciate the patience for a newbie, have pushed a new commit removing the comments above the debug prints.
maflcko
reviewed
Apr 2, 2018
There was a problem hiding this comment.
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
src/test/.vscode/settings.json
Outdated
| "files.associations": { | ||
| "*.ipp": "cpp" | ||
| } | ||
| } |
| @@ -411,21 +411,18 @@ def test_nonnull_locators(self, test_node, inv_node): | |||
| inv_node.check_last_announcement(inv=[tip], headers=[]) | |||
| test_node.check_last_announcement(inv=[tip], headers=[]) | |||
| if i == 0: | |||
| # Just get the data -- shouldn't cause headers announcements to resume | |||
Changing logs to debug level and format of statment to j == ... Updating debug statements to include comments when checking for iteration level of i and j Removing comments above log prints in Part 3 Removing unwanted file Added log prints in loops in p2p_sendheaders
1709160 to
8dd547d
Compare
|
As it stands, this doesn't seem to be that useful. It'll print out the same log many times over as it iterates over |
|
utACK 8dd547d. Seems like a tiny improvement over what it was before. |
maflcko
pushed a commit
that referenced
this pull request
Apr 7, 2018
8dd547d Adding logging for loop iteration level in p2p_sendheaders.py (ccdle12) Pull request description: PR for #12453 New contributor looking at mainly the 'good first issues'. From my understanding of the issue: * Track the iteration level of loop when test fails in Travis CI Further clarification on the issue would be greatly appreciated to narrow down what can be fixed or updated. Tree-SHA512: 228524aad54c09979d990a0fc6818e13a11e1ab5e78b606b892e897bba7b1e09bf25447feb631049e45ac017eeb61fbf1c1f11e8bd5103648f976a05099ba9f9
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Dec 16, 2020
8dd547d Adding logging for loop iteration level in p2p_sendheaders.py (ccdle12) Pull request description: PR for bitcoin#12453 New contributor looking at mainly the 'good first issues'. From my understanding of the issue: * Track the iteration level of loop when test fails in Travis CI Further clarification on the issue would be greatly appreciated to narrow down what can be fixed or updated. Tree-SHA512: 228524aad54c09979d990a0fc6818e13a11e1ab5e78b606b892e897bba7b1e09bf25447feb631049e45ac017eeb61fbf1c1f11e8bd5103648f976a05099ba9f9
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Dec 18, 2020
8dd547d Adding logging for loop iteration level in p2p_sendheaders.py (ccdle12) Pull request description: PR for bitcoin#12453 New contributor looking at mainly the 'good first issues'. From my understanding of the issue: * Track the iteration level of loop when test fails in Travis CI Further clarification on the issue would be greatly appreciated to narrow down what can be fixed or updated. Tree-SHA512: 228524aad54c09979d990a0fc6818e13a11e1ab5e78b606b892e897bba7b1e09bf25447feb631049e45ac017eeb61fbf1c1f11e8bd5103648f976a05099ba9f9
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.
PR for #12453
New contributor looking at mainly the 'good first issues'.
From my understanding of the issue:
Further clarification on the issue would be greatly appreciated to narrow down what can be fixed or updated.