Skip to content

Conversation

@jamesob
Copy link
Contributor

@jamesob jamesob commented Apr 18, 2018

In the midst of fighting with #12873 it became apparent that there're a number of assertions and print statements which are emitted by test nodes but don't identify the node in question. This change makes debugging a bit easier by adding identifying information to non-logger test_node-related error messages.

@jamesob jamesob force-pushed the 2018-04-18-func-test-debug-log branch from 7f0688e to da93b39 Compare April 18, 2018 19:44
@Empact
Copy link
Contributor

Empact commented Apr 18, 2018

utACK da93b39

A few style nits (ymmv):

  • _with_node_id feels like an implementation detail to me. A name like _node_msg could be clear without being overly so.
  • For raise_assertion - I'd either remove the indirection or apply it to all raises on the object, given they all use the node id treatment.

@fanquake fanquake added the Tests label Apr 18, 2018
@jamesob jamesob force-pushed the 2018-04-18-func-test-debug-log branch from da93b39 to 8e4ef3c Compare April 19, 2018 14:09
@jamesob
Copy link
Contributor Author

jamesob commented Apr 19, 2018

@Empact thanks for the review. I've incorporated your good feedback.

@jamesob jamesob force-pushed the 2018-04-18-func-test-debug-log branch from 8e4ef3c to 60087d1 Compare April 19, 2018 14:20
@jamesob jamesob force-pushed the 2018-04-18-func-test-debug-log branch from 60087d1 to 80a5e59 Compare April 19, 2018 14:21
@skeees
Copy link
Contributor

skeees commented Apr 22, 2018

concept ACK
might also be useful to print out the ports that each node{k} is on - afaict those aren't printed anywhere - and very useful as each bitcoind log identifies peer by a different index and the only way that i'm aware of to tie these to the python test framework indexing is by port num

@conscott
Copy link
Contributor

utACK 80a5e59

@laanwj laanwj merged commit 80a5e59 into bitcoin:master Apr 24, 2018
laanwj added a commit that referenced this pull request Apr 24, 2018
…print messages

80a5e59 [qa] Attach node index to test_node AssertionError and print messages (James O'Beirne)

Pull request description:

  In the midst of fighting with #12873 it became apparent that there're a number of assertions and print statements which are emitted by test nodes but don't identify the node in question. This change makes debugging a bit easier by adding identifying information to non-logger test_node-related error messages.

Tree-SHA512: 7cc86f2c81f4b3fdba15ec9a2d21a84c4b083629e845e82288087c3affbbdc5c68e74067621856cc97fe84fbc8cb4f5ca4977a51ef381e5d74515df8eb001239
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Nov 17, 2020
…or and print messages

80a5e59 [qa] Attach node index to test_node AssertionError and print messages (James O'Beirne)

Pull request description:

  In the midst of fighting with bitcoin#12873 it became apparent that there're a number of assertions and print statements which are emitted by test nodes but don't identify the node in question. This change makes debugging a bit easier by adding identifying information to non-logger test_node-related error messages.

Tree-SHA512: 7cc86f2c81f4b3fdba15ec9a2d21a84c4b083629e845e82288087c3affbbdc5c68e74067621856cc97fe84fbc8cb4f5ca4977a51ef381e5d74515df8eb001239

fix 13022

Signed-off-by: pasta <[email protected]>
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants