-
Notifications
You must be signed in to change notification settings - Fork 38.8k
tests: exclude all tests with difference parameters in --exclude list
#14316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK with one request for a comment.
test/functional/test_runner.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)