qa: Normalize executable location
ClosedPublic
Actions

Authored by Fabien on Jun 10 2019, 11:00.

Details

Reviewers

deadalnix
jasonbcox

Group Reviewers

Restricted Project

Commits

rSTAGINGc0cfdb0c647a: qa: Normalize executable location
rABCc0cfdb0c647a: qa: Normalize executable location

Summary

This removes the need to override the executable locations by just
reading them from the config file. Beside making the code easier to
read, running individual test on Windows is now possible by default
(without providing further command line arguments).

Note: Of course, it is still possible to manually specify the location
through the BITCOIND environment variable, e.g. bitcoin-qt

Backport of core PR13051
https://github.com/bitcoin/bitcoin/pull/13051/files

Note to reviewers: the backport has some minor differences with the
original PR due to changes from D2535 and D2696. Also core removed the
comparison framework, but it is still widely used in our codebase
especially in abc-* tests, so this diff adds changes to support the
comparison framework as well.

Test Plan

This should work from an out of tree build:

./test/functional/test_runner.py
../test/functional/rpc_uptime.py --configfile=test/config.ini

Due to other issues from our test_runner.py file this diff is not
enough to let you run functional tests under windows.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

PR13051

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 6241
Build 10529: Bitcoin ABC Buildbot (legacy)
Build 10528: arc lint + arc unit

Event Timeline

Fabien created this revision.Jun 10 2019, 11:00

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 10 2019, 11:00

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing failed.

Harbormaster failed remote builds in B6240: Diff 9280!Jun 10 2019, 11:04

Fabien planned changes to this revision.Jun 10 2019, 11:07

Fabien edited the summary of this revision. (Show Details)Jun 10 2019, 11:34

Fabien edited the test plan for this revision. (Show Details)

Fix comparison framework

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6241: Diff 9281.Jun 10 2019, 11:41

Fabien added a child revision: D3274: [tests] Better stderr testing.Jun 10 2019, 11:48

jasonbcox requested changes to this revision.Jun 10 2019, 17:11

jasonbcox added a subscriber: jasonbcox.

jasonbcox added inline comments.

test/functional/test_framework/test_framework.py
503	Is there a backport we're missing here? This change doesn't look associated with the original PR or the other two linked changes.
test/functional/test_framework/test_node.py
84	`cli_path` -> `bitcoin_cli`. Please fix this and add this case to the test plan: `BITCOINCLI=<some/path/that/doesnt/exist> test_runner.py`

This revision now requires changes to proceed.Jun 10 2019, 17:11

Fabien added inline comments.Jun 10 2019, 20:11

test/functional/test_framework/test_framework.py
503	Yes there is a backport missing, which is the one that suppress the comparison framework. I did not port it because the comparison framework is massively used in the abc-* tests, this requires rewriting a lot of tests. As this is not blocking other backports (this should be one of the very rare case where the comparison framework has to be reworked) I'd prefer continue without it and do it last.
test/functional/test_framework/test_node.py
84	Good catch !