Use pytest data to extract the parametrised decoration (#17881)

* Use pytest data to extract the parametrised decoration Rather than try and parse out the parametrized portion of the nodeid (delimited by square brackets but possibly containing square brackets), use native [pytest item attributes](https://docs.pytest.org/en/6.2.x/reference.html#function) to separate out the decoration. Better solution for #17357, fixing #17676. * Add news entry * Update news/2 Fixes/17676.md Co-authored-by: Karthik Nadig <kanadig@microsoft.com> Co-authored-by: Karthik Nadig <kanadig@microsoft.com>
microsoft · Nov 8, 2021 · 251cb30 · 251cb30
1 parent d5f0dc5
commit 251cb30
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 39 deletions.
diff --git a/news/2 Fixes/17676.md b/news/2 Fixes/17676.md
@@ -0,0 +1,2 @@
+When parsing pytest node ids with parameters, use native pytest information to separate out the parameter decoration rather than try and parse the nodeid as text.
+(thanks [Martijn Pieters](https://github.com/mjpieters))
diff --git a/pythonFiles/testing_tools/adapter/pytest/_pytest_item.py b/pythonFiles/testing_tools/adapter/pytest/_pytest_item.py
@@ -158,9 +158,21 @@ def parse_item(
     # Skip plugin generated tests
     if kind is None:
         return None, None
-    (nodeid, parents, fileid, testfunc, parameterized) = _parse_node_id(
-        item.nodeid, kind
-    )
+
+    if kind == "function" and item.originalname and item.originalname != item.name:
+        # split out parametrized decorations `node[params]`) before parsing
+        # and manually attach parametrized portion back in when done.
+        parameterized = item.name[len(item.originalname) :]
+        (parentid, parents, fileid, testfunc, _) = _parse_node_id(
+            item.nodeid[: -len(parameterized)], kind
+        )
+        nodeid = "{}{}".format(parentid, parameterized)
+        parents = [(parentid, item.originalname, kind)] + parents
+    else:
+        (nodeid, parents, fileid, testfunc, parameterized) = _parse_node_id(
+            item.nodeid, kind
+        )
+
     # Note: testfunc does not necessarily match item.function.__name__.
     # This can result from importing a test function from another module.
 
@@ -434,32 +446,6 @@ def _parse_node_id(
     )
 
 
-def _find_left_bracket(nodeid):
-    """Return tuple of part before final bracket open, separator [, and the remainder.
-    Notes:
-        Testcase names in case of parametrized tests are wrapped in [<test-case-name>].
-    Examples:
-        dirname[sometext]/dirname/testfile.py::testset::testname[testcase]
-        => ('dirname[sometext]/dirname/testfile.py::testset::testname', '[', 'testcase]')
-        dirname/dirname/testfile.py::testset::testname[testcase]
-        => ('dirname/dirname/testfile.py::testset::testname', '[', 'testcase]')
-        dirname/dirname/testfile.py::testset::testname[testcase[x]]
-        => ('dirname/dirname/testfile.py::testset::testname', '[', 'testcase[x]]')
-    """
-    if not nodeid.endswith("]"):
-        return nodeid, "", ""
-    bracketcount = 0
-    for index, char in enumerate(nodeid[::-1]):
-        if char == "]":
-            bracketcount += 1
-        elif char == "[":
-            bracketcount -= 1
-        if bracketcount == 0:
-            n = len(nodeid) - 1 - index
-            return nodeid[:n], nodeid[n], nodeid[n + 1 :]
-    return nodeid, "", ""
-
-
 def _iter_nodes(
     testid,
     kind,
@@ -473,16 +459,6 @@ def _iter_nodes(
     if len(nodeid) > len(testid):
         testid = "." + _pathsep + testid
 
-    if kind == "function" and nodeid.endswith("]"):
-        funcid, sep, parameterized = _find_left_bracket(nodeid)
-        if not sep:
-            raise should_never_reach_here(
-                nodeid,
-                # ...
-            )
-        yield (nodeid, sep + parameterized, "subtest")
-        nodeid = funcid
-
     parentid, _, name = nodeid.rpartition("::")
     if not parentid:
         if kind is None:

diff --git a/pythonFiles/tests/testing_tools/adapter/pytest/test_discovery.py b/pythonFiles/tests/testing_tools/adapter/pytest/test_discovery.py
@@ -530,6 +530,7 @@ def test_modifyitems(self):
                     stub,
                     nodeid="test_spam.py::SpamTests::test_one",
                     name="test_one",
+                    originalname=None,
                     location=("test_spam.py", 12, "SpamTests.test_one"),
                     fspath=adapter_util.PATH_JOIN(testroot, "test_spam.py"),
                     function=FakeFunc("test_one"),
@@ -538,6 +539,7 @@ def test_modifyitems(self):
                     stub,
                     nodeid="test_spam.py::SpamTests::test_other",
                     name="test_other",
+                    originalname=None,
                     location=("test_spam.py", 19, "SpamTests.test_other"),
                     fspath=adapter_util.PATH_JOIN(testroot, "test_spam.py"),
                     function=FakeFunc("test_other"),
@@ -546,6 +548,7 @@ def test_modifyitems(self):
                     stub,
                     nodeid="test_spam.py::test_all",
                     name="test_all",
+                    originalname=None,
                     location=("test_spam.py", 144, "test_all"),
                     fspath=adapter_util.PATH_JOIN(testroot, "test_spam.py"),
                     function=FakeFunc("test_all"),
@@ -554,6 +557,7 @@ def test_modifyitems(self):
                     stub,
                     nodeid="test_spam.py::test_each[10-10]",
                     name="test_each[10-10]",
+                    originalname="test_each",
                     location=("test_spam.py", 273, "test_each[10-10]"),
                     fspath=adapter_util.PATH_JOIN(testroot, "test_spam.py"),
                     function=FakeFunc("test_each"),
@@ -562,6 +566,7 @@ def test_modifyitems(self):
                     stub,
                     nodeid=relfile2 + "::All::BasicTests::test_first",
                     name="test_first",
+                    originalname=None,
                     location=(relfile2, 31, "All.BasicTests.test_first"),
                     fspath=adapter_util.PATH_JOIN(testroot, relfile2),
                     function=FakeFunc("test_first"),
@@ -570,6 +575,7 @@ def test_modifyitems(self):
                     stub,
                     nodeid=relfile2 + "::All::BasicTests::test_each[1+2-3]",
                     name="test_each[1+2-3]",
+                    originalname="test_each",
                     location=(relfile2, 62, "All.BasicTests.test_each[1+2-3]"),
                     fspath=adapter_util.PATH_JOIN(testroot, relfile2),
                     function=FakeFunc("test_each"),
@@ -781,6 +787,7 @@ def test_finish(self):
                 stub,
                 nodeid=relfile + "::SpamTests::test_spam",
                 name="test_spam",
+                originalname=None,
                 location=(relfile, 12, "SpamTests.test_spam"),
                 fspath=adapter_util.PATH_JOIN(testroot, relfile),
                 function=FakeFunc("test_spam"),
@@ -992,6 +999,7 @@ def test_nested_brackets(self):
                 stub,
                 nodeid=relfile + "::SpamTests::test_spam[a-[b]-c]",
                 name="test_spam[a-[b]-c]",
+                originalname="test_spam",
                 location=(relfile, 12, "SpamTests.test_spam[a-[b]-c]"),
                 fspath=adapter_util.PATH_JOIN(testroot, relfile),
                 function=FakeFunc("test_spam"),
@@ -1054,6 +1062,7 @@ def test_nested_suite(self):
                 stub,
                 nodeid=relfile + "::SpamTests::Ham::Eggs::test_spam",
                 name="test_spam",
+                originalname=None,
                 location=(relfile, 12, "SpamTests.Ham.Eggs.test_spam"),
                 fspath=adapter_util.PATH_JOIN(testroot, relfile),
                 function=FakeFunc("test_spam"),
@@ -1120,6 +1129,7 @@ def test_windows(self):
                 # pytest always uses "/" as the path separator in node IDs:
                 nodeid="X/Y/Z/test_Eggs.py::SpamTests::test_spam",
                 name="test_spam",
+                originalname=None,
                 # normal path separator (contrast with nodeid):
                 location=(relfile, 12, "SpamTests.test_spam"),
                 # path separator matches location:
@@ -1152,6 +1162,7 @@ def test_windows(self):
                     stub,
                     nodeid=fileid + "::test_spam",
                     name="test_spam",
+                    originalname=None,
                     location=(locfile, 12, "test_spam"),
                     fspath=fspath,
                     function=FakeFunc("test_spam"),
@@ -1412,6 +1423,7 @@ def test_mysterious_parens(self):
                 stub,
                 nodeid=relfile + "::SpamTests::()::()::test_spam",
                 name="test_spam",
+                originalname=None,
                 location=(relfile, 12, "SpamTests.test_spam"),
                 fspath=adapter_util.PATH_JOIN(testroot, relfile),
                 function=FakeFunc("test_spam"),
@@ -1472,6 +1484,7 @@ def test_imported_test(self):
                 stub,
                 nodeid=relfile + "::SpamTests::test_spam",
                 name="test_spam",
+                originalname=None,
                 location=(srcfile, 12, "SpamTests.test_spam"),
                 fspath=adapter_util.PATH_JOIN(testroot, relfile),
                 function=FakeFunc("test_spam"),
@@ -1480,6 +1493,7 @@ def test_imported_test(self):
                 stub,
                 nodeid=relfile + "::test_ham",
                 name="test_ham",
+                originalname=None,
                 location=(srcfile, 3, "test_ham"),
                 fspath=adapter_util.PATH_JOIN(testroot, relfile),
                 function=FakeFunc("test_spam"),