tests: exclude all tests with difference parameters in --exclude list#14316
Conversation
--exclude list
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
jnewbery
left a comment
There was a problem hiding this comment.
utACK with one request for a comment.
There was a problem hiding this comment.
nit: could remove the use of the re module, since it is no longer required? "foo.py".split('.py')[0] == "foo".split('.py')[0]
jimmysong
left a comment
There was a problem hiding this comment.
Couple of nits , but otherwise looks good.
| # Remove the test cases that the user has explicitly asked to exclude. | ||
| if args.exclude: | ||
| exclude_tests = [re.sub("\.py$", "", test) + (".py" if ".py" not in test else "") for test in args.exclude.split(',')] | ||
| exclude_tests = [test.split('.py')[0] for test in args.exclude.split(',')] |
There was a problem hiding this comment.
nit: I prefer test.strip('.py') just in case there's a very unlikely '.py' in the middle of a file name.
There was a problem hiding this comment.
Wouldn't work with args?
>>> 'foo.py --arg'.strip('.py')
'foo.py --arg'
| test_list.remove(exclude_test) | ||
| else: | ||
| # Remove <test_name>.py and <test_name>.py --arg from the test list | ||
| exclude_list = [test for test in test_list if test.split('.py')[0] == exclude_test] |
There was a problem hiding this comment.
nit: same as above. test.strip('.py') is preferable over test.split('.py')[0] and reads a little better.
|
utACK c7b3e48 |
…ers in `--exclude` list c7b3e48 tests: exclude all tests with difference parameters (Chun Kuan Lee) Pull request description: Fix broken exclusion list in functional tests. See bitcoin#14007 (review) Tree-SHA512: b6c2b86fef13e3c00c695adaeeb3e47ee9b48877c71bc605d24201ce931b2ef3ae9f5f199071fa1ec5de2d7aadc478410094c380cc297922e683e9b2569cda03
Fix broken exclusion list in functional tests. See #14007 (review)