Page MenuHomePhabricator

qa: Normalize executable location
ClosedPublic

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

Details

Reviewers
deadalnix
jasonbcox
Group Reviewers
Restricted Project
Commits
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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Fabien created this revision.Mon, Jun 10, 11:00
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Jun 10, 11:00
Fabien planned changes to this revision.Mon, Jun 10, 11:07
Fabien edited the summary of this revision. (Show Details)Mon, Jun 10, 11:34
Fabien edited the test plan for this revision. (Show Details)
Fabien updated this revision to Diff 9281.Mon, Jun 10, 11:35

Fix comparison framework

jasonbcox requested changes to this revision.Mon, Jun 10, 17:11
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
test/functional/test_framework/test_framework.py
503 ↗(On Diff #9281)

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 ↗(On Diff #9281)

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.Mon, Jun 10, 17:11
Fabien added inline comments.Mon, Jun 10, 20:11
test/functional/test_framework/test_framework.py
503 ↗(On Diff #9281)

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 ↗(On Diff #9281)

Good catch !

Fabien edited the test plan for this revision. (Show Details)Mon, Jun 10, 20:12
Fabien edited the test plan for this revision. (Show Details)Mon, Jun 10, 20:12
Fabien updated this revision to Diff 9293.

Fix exception message.

jasonbcox added inline comments.Mon, Jun 10, 20:36
test/functional/test_framework/test_framework.py
503 ↗(On Diff #9281)

Got it. Thanks.

jasonbcox accepted this revision.Mon, Jun 10, 21:16
This revision is now accepted and ready to land.Mon, Jun 10, 21:16
This revision was automatically updated to reflect the committed changes.
markblundeberg added inline comments.
test/functional/test_framework/test_framework.py
88

note: bug here, fixed in D3311