Page MenuHomePhabricator

test: allows test_runner command line to receive parameters for each test
AbandonedPublic

Authored by PiRK on Sep 21 2020, 17:30.

Details

Reviewers
jasonbcox
deadalnix
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Summary

Backport of Bitcoin Core PR14795

Test Plan

ninja && ./test/functional/test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Branch
backport-pr14795
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 12831
Build 25732: Build Diffbuild-without-wallet · build-clang-tidy · build-clang-10 · build-diff
Build 25731: arc lint + arc unit

Event Timeline

Owners added a reviewer: Restricted Owners Package.Sep 21 2020, 17:30
PiRK requested review of this revision.Sep 21 2020, 17:30

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

jasonbcox requested changes to this revision.Sep 21 2020, 20:34
jasonbcox added a subscriber: jasonbcox.

The test plan does not test the supposed change, nor does it test the wildcard behavior that differs from the original PR.

This revision now requires changes to proceed.Sep 21 2020, 20:34

fix regression during backport for test names ending with a wildcard

I missed the wildcard when fixing the cherry-pick conflict. I believe I fixed that, and just tested it with the following command:

./test/functional/test_runner.py interface_*

Regarding the test plan, I need to search for a test that takes parameters. I don't know yet if there is any. Maybe I'll need to backport Core PR 12510 for that. I will look into that issue tomorrow.

deadalnix requested changes to this revision.Sep 21 2020, 23:26
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
test/functional/test_runner.py
274

Can we replace this hideous hack by some proper use of https://docs.python.org/3/library/glob.html ?

This revision now requires changes to proceed.Sep 21 2020, 23:26

Also, @jasonbcox is right, the test plan does not cover the behavior change.

I'm having a closer look at test_runner.py, and it seems our code has diverged quite a lot with Bitcoin Core. I'm sorry, should have started with that before naively trying to apply the backport.

The test detection and extra command line arguments are done very differently. In our version, we always run all variations of tests that accept params. I'm not sure that allowing to pass extra arguments to test_runner.py makes sense for us. I should probably scrap the entire diff.

When you try passing extra parameters to a test that is listed as one of the tests with options, our script always runs multiple tests, so the create_cache.py script is called first, and it chokes on the unrecognized extra argument that is passed to him.
In the bitcoin core version of the script, when you pass an extra argument it necessarily means that you are running a single test, so the create_cache.py step is avoided.