test: Enable passing wildcard test names to test runner from root #16374

jonatack commented at 8:29 PM on July 11, 2019: member

Currently, passing wildcard testname args to the test runner from outside the test/functional/ directory does not work, even though developers expect it to. See these recent IRC discussions for more background: http://www.erisian.com.au/bitcoin-core-dev/log-2019-07-10.html#l-262 (lines 262 to 323) and http://www.erisian.com.au/bitcoin-core-dev/log-2019-07-11.html#l-134.

[BUGFIX] Enable passing wildcards with paths. Examples:
- test/functional/test_runner.py test/functional/wallet*
- functional/test_runner.py functional/wallet*
- test/functional/test_runner.py ./test/functional/tool* test/functional/mempool*
- A current limitation this PR does not change: 9 test files with arguments in their filename are not picked up by wildcard search.
[Docs] Describe how to pass wildcard names (multiple and with paths) to the test runner in test/README.md.

in test/README.md:52 in 78bd2866dd outdated

  48 | @@ -49,6 +49,28 @@ You can run any combination (incl. duplicates) of tests by calling:
  49 |  test/functional/test_runner.py <testname1> <testname2> <testname3> ...
  50 |  ```
  51 |  
  52 | +Wildcard test names can be passed if the paths are coherent.

MarcoFalke commented at 8:38 PM on July 11, 2019:

Could mention that this requires calling it from a bash or similar, since it requires the shell to do the globbing

jonatack commented at 8:54 PM on July 11, 2019:

Good idea. Done.

jonatack force-pushed on Jul 11, 2019

in test/functional/test_runner.py:282 in 4a5698bfb5 outdated

 278 | +        #   `test/functional/test_runner.py ./test/functional/wallet*`
 279 | +        #   `test_runner.py wallet*`
 280 | +        #   but not:
 281 | +        #   `test/functional/test_runner.py wallet*`
 282 | +        # Multiple wildcards can be passed:
 283 | +        #   `test_runner.py tool* mempool*`

MarcoFalke commented at 9:09 PM on July 11, 2019:

Sorry for the follow up comment, but the test runner only sees a list of file names and not the wildcard, so mentioning it here is a bit confusing. Also, incoherent paths are accepted. You can pass e.g. test/functional/test_runner.py /asdf/wallet_disable.py /foo2/wallet_disable wallet_disable.

Maybe remove this here? (At most you could mention that path prefixes are stripped in the argparse doc of this script)

jonatack commented at 9:20 PM on July 11, 2019:

No problem. True. Though with wildcards those incoherent paths don't seem to be accepted. I'll rework this.

jonatack commented at 8:18 AM on July 15, 2019:

Reworded:

--- Specified tests can contain paths and wildcards, but the paths must be coherent
+++ Specified tests can contain wildcards, but in that case the supplied paths should be coherent

I'm happy to remove the added comments if they are still confusing.

fanquake added the label Tests on Jul 11, 2019

promag commented at 12:49 AM on July 12, 2019: member

Concept ACK, how about a -filter option like bench has?

MarcoFalke commented at 1:03 PM on July 14, 2019: member

ACK 4a5698bfb5b8ade8bdcd0fb92c3706c57f4d235f code changes look like a nice one-line diff with a clear usability improvement

promag commented at 10:27 PM on July 14, 2019: member

Above suggestion #16374 (comment) in #16390.

test: enable passing wildcards with path to test runner

Currently, passing wildcard testname args to the test runner from outside the `test/functional/` directory does not work. See this recent IRC discussion for more background: http://www.erisian.com.au/bitcoin-core-dev/log-2019-07-10.html#l-262 (lines 262 to 323).

This small change enables passing multiple wildcards with paths, as long as the paths are coherent. Examples:
  - test/functional/test_runner.py test/functional/wallet*
  - functional/test_runner.py functional/wallet*
  - test/functional/test_runner.py ./test/functional/tool* test/functional/mempool*

A current limitation that this PR does not change: 9 test files with arguments in their name are not picked up by wildcard search.

- Squashed commit: non-mutating version

- Squashed commit: minor code optimisation

6a7a70b8cf

doc: describe how to pass wildcard names to test runner e142ee03e7

in test/functional/test_runner.py:285 in 4a5698bfb5 outdated

 281 | +        #   `test/functional/test_runner.py wallet*`
 282 | +        # Multiple wildcards can be passed:
 283 | +        #   `test_runner.py tool* mempool*`
 284 |          tests = [test + ".py" if ".py" not in test else test for test in tests]
 285 |          for test in tests:
 286 | +            test = test.split("/")[-1]

jonatack commented at 8:25 AM on July 15, 2019:

I liked the simplicity of the one-line diff but made 2 minor additional changes:

moved tests = [test + ".py" if ".py" not in test else test for test in tests] into the main loop to avoid iterating twice through tests
added a local variable script to avoid mutating the test iterator

Happy to revert to the one-line version if that is preferred.

jonatack force-pushed on Jul 15, 2019

jnewbery commented at 2:51 PM on July 16, 2019: member

tested ACK e142ee03e7a139168aa1dbf5910c616f60d25042

MarcoFalke commented at 3:08 PM on July 16, 2019: member

test/functional/test_runner.py test/functional/wallet*

How is this different from test/functional/test_runner.py --filter='^wallet.*'?

jonatack commented at 11:39 AM on July 17, 2019: member

test/functional/test_runner.py test/functional/wallet*

How is this different from test/functional/test_runner.py --filter='^wallet.*'?

The first (this PR) fixes/improves an inconsistent existing interface that did not function as people expected, and adds missing documentation for it.

The second adds a new interface that in my opinion has merit and could be seen as complementary, but it does not fix the issue with the existing interface that this PR addresses, nor does it remove the existing interface.

Thus the two are different. The first is IMO a necessary fix, and the second is a new feature.

jachiang commented at 3:57 PM on July 17, 2019: contributor

Tested ACK https://github.com/bitcoin/bitcoin/commit/e142ee03e7a139168aa1dbf5910c616f60d25042. Thanks a lot for this fix!

MarcoFalke commented at 5:30 PM on July 17, 2019: member

ACK e142ee03e7a139168aa1dbf5910c616f60d25042, fine with me

pull[bot] referenced this in commit 0515406acb on Jul 18, 2019

fanquake merged this on Jul 18, 2019

fanquake closed this on Jul 18, 2019

PastaPastaPasta referenced this in commit d29e0f1616 on Jun 27, 2021

PastaPastaPasta referenced this in commit 7b829c9ff2 on Jun 28, 2021

PastaPastaPasta referenced this in commit 9d39047189 on Jun 29, 2021

PastaPastaPasta referenced this in commit 242ea3fb56 on Jul 1, 2021

PastaPastaPasta referenced this in commit 77a10bab61 on Jul 1, 2021

PastaPastaPasta referenced this in commit bd64eabf35 on Jul 12, 2021

PastaPastaPasta referenced this in commit d47df667bd on Jul 13, 2021

DrahtBot locked this on Dec 16, 2021