Merge #13022: [qa] Attach node index to test_node AssertionError and print messages
ClosedPublic
Actions

Authored by jasonbcox on Aug 20 2019, 01:03.

Details

Reviewers

deadalnix
Fabien

Group Reviewers

Restricted Project

Commits

rSTAGING1f1acdc42eb5: Merge #13022: [qa] Attach node index to test_node AssertionError and print…
rABC1f1acdc42eb5: Merge #13022: [qa] Attach node index to test_node AssertionError and print…

Summary

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

Pull request description:

In the midst of fighting with https://github.com/bitcoin/bitcoin/pull/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

Backport of Core PR13022
https://github.com/bitcoin/bitcoin/pull/13022/files

Depends on D3914

Test Plan

test_runner.py

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jasonbcox created this revision.Aug 20 2019, 01:03

Herald added a reviewer: deadalnix. · View Herald TranscriptAug 20 2019, 01:03

Build Bitcoin-ABC / Diffs / Diff Testing started.

jasonbcox planned changes to this revision.Aug 20 2019, 01:03

jasonbcox edited the summary of this revision. (Show Details)

jasonbcox added a parent revision: D3914: Merge #12853: qa: Match full plain text by default.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7198: Diff 10877.Aug 20 2019, 01:09

jasonbcox requested review of this revision.Aug 20 2019, 01:32

Fabien requested changes to this revision.Aug 20 2019, 07:19

Fabien added a subscriber: Fabien.

Fabien added inline comments.

test/functional/test_framework/test_node.py
109 ↗	(On Diff #10877)	Note that this notation discards support for python 3.4 (this syntax is from 3.5 https://docs.python.org/3/library/typing.html). While not being a big issue (python 3.5 is default in Xenial and stretch, and is already pretty old), we might want to bump the minimum python version along with this diff (see https://github.com/bitcoin/bitcoin/pull/14954 and https://github.com/bitcoin/bitcoin/pull/15601). Looks like core did it out of order ?
216 ↗	(On Diff #10877)	That should be in par with the output from `__getattr__`, whether keep the error messages separated or just squash them into one, up to you.