bpo-30540: regrtest: add --matchfile option #1909

vstinner · 2017-06-01T19:40:36Z

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

serhiy-storchaka · 2017-06-03T15:22:14Z

Lib/test/libregrtest/cmdline.py

@@ -189,8 +189,11 @@ def _create_parser():
                       help='single step through a set of tests.' +
                            more_details)
    group.add_argument('-m', '--match', metavar='PAT',
-                       dest='match_tests',
+                       dest='match_tests', action='append',


Support of multiple -m options -- I like this!

serhiy-storchaka · 2017-06-03T15:22:54Z

Lib/test/libregrtest/cmdline.py

                       help='match test cases and methods with glob pattern PAT')
+    group.add_argument('--matchfile', metavar='FILENAME',
+                       dest='match_filename',
+                       help='similar to --match but get pattersn from a text file, one pattern per line')


serhiy-storchaka · 2017-06-03T15:27:56Z

Lib/test/test_regrtest.py

@@ -159,7 +159,7 @@ def test_match(self):
        for opt in '-m', '--match':
            with self.subTest(opt=opt):
                ns = libregrtest._parse_args([opt, 'pattern'])
-                self.assertEqual(ns.match_tests, 'pattern')
+                self.assertEqual(ns.match_tests, ['pattern'])


Add tests for multiple -m and for --matchfile.

serhiy-storchaka · 2017-06-03T15:32:04Z

Lib/test/libregrtest/cmdline.py

@@ -350,5 +353,10 @@ def _parse_args(args, **kwargs):
        print("WARNING: Disable --verbose3 because it's incompatible with "
              "--huntrleaks: see http://bugs.python.org/issue27103",
              file=sys.stderr)
+    if ns.match_filename:
+        ns.match_tests = []


What if specify both --match and --matchfile?

--matchfile overrides --match.

It could be defined as extending match_tests.

vstinner · 2017-06-08T16:58:20Z

@serhiy-storchaka: Does it look better now?

serhiy-storchaka · 2017-06-08T17:17:10Z

Lib/test/test_regrtest.py

+        regex = re.compile("^(test[^ ]+).*ok$", flags=re.MULTILINE)
+        return [match.group(1) for match in regex.finditer(output)]
+
+    def test_matchfile(self):


Oh, it would be enough to add a test only in ParseArgsTestCase! But more comprehensive test is good.

serhiy-storchaka · 2017-06-08T17:17:45Z

Lib/test/test_regrtest.py

+        with open(filename, "w") as fp:
+            for name in subset:
+                print(name, file=fp)
+            fp.flush()


This is redundant.

I removed the flush.

serhiy-storchaka · 2017-06-08T17:22:39Z

Lib/test/test_regrtest.py

+                    pass
+        """)
+        all_methods = ['test_method1', 'test_method2',
+                       'test_method3', 'test_method4']


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

I don't understand this rule. The flake8 tool doesn't complain.

serhiy-storchaka · 2017-06-08T17:23:30Z

Lib/test/test_regrtest.py

+            # only match the method name
+            'test_method1',
+            # match the full identifier
+            '%s.Tests.test_method3' % testname]


The closing bracket doesn't conform PEP 8.

See also "When to use trailing commas".

serhiy-storchaka · 2017-06-08T17:26:56Z

Lib/test/test_regrtest.py

+        methods = self.parse_methods(output)
+        subset = ['test_method1', 'test_method3']
+        self.assertEqual(methods, subset)
+


What if the match file is empty?

Ah, nothing interesting. This doesn't deserve special testing.

serhiy-storchaka · 2017-06-08T17:28:52Z

Lib/test/libregrtest/cmdline.py

@@ -350,5 +353,10 @@ def _parse_args(args, **kwargs):
        print("WARNING: Disable --verbose3 because it's incompatible with "
              "--huntrleaks: see http://bugs.python.org/issue27103",
              file=sys.stderr)
+    if ns.match_filename:
+        ns.match_tests = []


It could be defined as extending match_tests.

serhiy-storchaka · 2017-06-08T17:32:41Z

Lib/test/libregrtest/cmdline.py

@@ -117,6 +117,10 @@
 To enable all resources except one, use '-uall,-<resource>'.  For
 example, to run all the tests except for the gui tests, give the
 option '-uall,-gui'.
+
+--matchfile filters the test methods using a text file. Each line must be a
+test method (ex: test_method) or a test identifier (ex:


Or class, or file.

* 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 · 2017-06-09T08:02:00Z

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 · 2017-06-09T08:19:05Z

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

* 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)

the-knights-who-say-ni added the CLA signed label Jun 1, 2017

serhiy-storchaka reviewed Jun 3, 2017

View reviewed changes

serhiy-storchaka reviewed Jun 8, 2017

View reviewed changes

serhiy-storchaka approved these changes Jun 9, 2017

View reviewed changes

vstinner merged commit ef8320c into python:master Jun 9, 2017

vstinner deleted the regrtest_matchfile branch June 9, 2017 08:18

vstinner mentioned this pull request Jun 16, 2017

[3.6] Synchronize libregrtest from master to 3.6 #2244

Merged

Uh oh!

bpo-30540: regrtest: add --matchfile option #1909

bpo-30540: regrtest: add --matchfile option #1909

Uh oh!

Conversation

vstinner commented Jun 1, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner commented Jun 8, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner commented Jun 9, 2017

Uh oh!

vstinner commented Jun 9, 2017

Uh oh!

Uh oh!