-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
bpo-30540: regrtest: add --matchfile option #1909
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
Lib/test/libregrtest/cmdline.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.
Support of multiple -m options -- I like this!
Lib/test/libregrtest/cmdline.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.
"patterns"
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.
Fixed
Lib/test/test_regrtest.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.
Add tests for multiple -m and for --matchfile.
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.
Done
Lib/test/libregrtest/cmdline.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.
What if specify both --match and --matchfile?
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.
--matchfile overrides --match.
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.
It could be defined as extending match_tests.
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.
Ok, fixed
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.
done
|
@serhiy-storchaka: Does it look better now? |
Lib/test/test_regrtest.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.
Oh, it would be enough to add a test only in ParseArgsTestCase! But more comprehensive test is good.
Lib/test/test_regrtest.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.
This is redundant.
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.
I removed the flush.
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.
removed
Lib/test/test_regrtest.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.
The closing bracket doesn't conform PEP 8.
The closing brace/bracket/parenthesis on multi-line constructs may either line up under the first non-whitespace character of the last line of list, ... or it may be lined up under the first character of the line that starts the multi-line construct...
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.
I don't understand this rule. The flake8 tool doesn't complain.
Lib/test/test_regrtest.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.
The closing bracket doesn't conform PEP 8.
See also "When to use trailing commas".
Lib/test/test_regrtest.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.
What if the match file is empty?
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.
Ah, nothing interesting. This doesn't deserve special testing.
Lib/test/libregrtest/cmdline.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.
It could be defined as extending match_tests.
Lib/test/libregrtest/cmdline.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.
Or class, or file.
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.
fixed
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.
fixed
* Add a new option taking a filename to get a list of test names to filter tests. * support.match_tests becomes a list. * Modify run_unittest() to accept to match the whole test identifier, not just a part of a test identifier. For example, the following command only runs test_default_timeout() of the BarrierTests class of test_threading: $ ./python -m test -v test_threading -m test.test_threading.BarrierTests.test_default_timeout Remove also some empty lines from test_regrtest.py to make flake8 tool happy.
|
I fixed a bug for relative filenames in --matchfile (use support.SAVEDCWD). All comments should be addressed, except of the comment about PEP 8 but flake8 doesn't complain. |
|
As usual, thank you very much for your useful review @serhiy-storchaka! |
filter tests.
not just a part of a test identifier.
For example, the following command only runs test_default_timeout()
of the BarrierTests class of test_threading:
$ ./python -m test -v test_threading -m test.test_threading.BarrierTests.test_default_timeout