Skip to content

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Jun 1, 2017

  • 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

Copy link
Member

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!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"patterns"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Copy link
Member

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Member

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--matchfile overrides --match.

Copy link
Member

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, fixed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@vstinner
Copy link
Member Author

vstinner commented Jun 8, 2017

@serhiy-storchaka: Does it look better now?

Copy link
Member

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is redundant.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the flush.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Copy link
Member

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...

Copy link
Member Author

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.

Copy link
Member

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".

Copy link
Member

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?

Copy link
Member

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.

Copy link
Member

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or class, or file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Copy link
Member Author

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.
@vstinner
Copy link
Member Author

vstinner commented Jun 9, 2017

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.

@vstinner vstinner merged commit ef8320c into python:master Jun 9, 2017
@vstinner vstinner deleted the regrtest_matchfile branch June 9, 2017 08:18
@vstinner
Copy link
Member Author

vstinner commented Jun 9, 2017

As usual, thank you very much for your useful review @serhiy-storchaka!

vstinner added a commit that referenced this pull request Jun 16, 2017
* bpo-30523: regrtest: Add --list-cases option (#2238)
* bpo-30284: Fix regrtest for out of tree build (#1481)
* bpo-30540: regrtest: add --matchfile option (#1909)
* bpo-30258: regrtest: Fix run_tests_multiprocess() (#1479)
* bpo-30263: regrtest: log system load (#1452)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants