Merge pull request #4358 from blueyed/instance

Node: do not add "::()" to nodeid

changelog/4358.removal.rst

@@ -0,0 +1,18 @@
+Remove the ``::()`` notation to denote a test class instance in node ids.
+
+Previously, node ids that contain test instances would use ``::()`` to denote the instance like this::
+
+    test_foo.py::Test::()::test_bar
+
+The extra ``::()`` was puzzling to most users and has been removed, so that the test id becomes now::
+
+    test_foo.py::Test::test_bar
+
+This change could not accompany a deprecation period as is usual when user-facing functionality changes because
+it was not really possible to detect when the functionality was being used explicitly.
+
+The extra ``::()`` might have been removed in some places internally already,
+which then led to confusion in places where it was expected, e.g. with
+``--deselect`` (`#4127 <https://github.com/pytest-dev/pytest/issues/4127>`_).
+
+Test class instances are also not listed with ``--collect-only`` anymore.

src/_pytest/nodes.py

@@ -27,7 +27,7 @@ def _splitnode(nodeid):
         ''
         'testing/code'
         'testing/code/test_excinfo.py'
-        'testing/code/test_excinfo.py::TestFormattedExcinfo::()'
+        'testing/code/test_excinfo.py::TestFormattedExcinfo'
 
     Return values are lists e.g.
         []
@@ -39,7 +39,7 @@ def _splitnode(nodeid):
         # If there is no root node at all, return an empty list so the caller's logic can remain sane
         return []
     parts = nodeid.split(SEP)
-    # Replace single last element 'test_foo.py::Bar::()' with multiple elements 'test_foo.py', 'Bar', '()'
+    # Replace single last element 'test_foo.py::Bar' with multiple elements 'test_foo.py', 'Bar'
     parts[-1:] = parts[-1].split("::")
     return parts
 
@@ -47,7 +47,7 @@ def _splitnode(nodeid):
 def ischildnode(baseid, nodeid):
     """Return True if the nodeid is a child node of the baseid.
 
-    E.g. 'foo/bar::Baz::()' is a child of 'foo', 'foo/bar' and 'foo/bar::Baz', but not of 'foo/blorp'
+    E.g. 'foo/bar::Baz' is a child of 'foo', 'foo/bar' and 'foo/bar::Baz', but not of 'foo/blorp'
     """
     base_parts = _splitnode(baseid)
     node_parts = _splitnode(nodeid)
@@ -107,10 +107,12 @@ class Node(object):
         self._name2pseudofixturedef = {}
 
         if nodeid is not None:
+            assert "::()" not in nodeid
             self._nodeid = nodeid
         else:
-            assert parent is not None
+            self._nodeid = self.parent.nodeid
-            self._nodeid = self.parent.nodeid + "::" + self.name
+            if self.name != "()":
+                self._nodeid += "::" + self.name
 
     @property
     def ihook(self):

src/_pytest/runner.py

@@ -60,8 +60,7 @@ def pytest_terminal_summary(terminalreporter):
             tr.write_line("")
             tr.write_line("(0.00 durations hidden.  Use -vv to show these durations.)")
             break
-        nodeid = rep.nodeid.replace("::()::", "::")
+        tr.write_line("%02.2fs %-8s %s" % (rep.duration, rep.when, rep.nodeid))
-        tr.write_line("%02.2fs %-8s %s" % (rep.duration, rep.when, nodeid))
 
 
 def pytest_sessionstart(session):

src/_pytest/terminal.py

@@ -605,9 +605,7 @@ class TerminalReporter(object):
                     self._tw.line("%s: %d" % (name, count))
             else:
                 for item in items:
-                    nodeid = item.nodeid
+                    self._tw.line(item.nodeid)
-                    nodeid = nodeid.replace("::()::", "::")
-                    self._tw.line(nodeid)
             return
         stack = []
         indent = ""
@@ -619,8 +617,8 @@ class TerminalReporter(object):
                 stack.pop()
             for col in needed_collectors[len(stack) :]:
                 stack.append(col)
-                # if col.name == "()":
+                if col.name == "()":  # Skip Instances.
-                #    continue
+                    continue
                 indent = (len(stack) - 1) * "  "
                 self._tw.line("%s%s" % (indent, col))
 
@@ -687,7 +685,7 @@ class TerminalReporter(object):
         # collect_fspath comes from testid which has a "/"-normalized path
 
         if fspath:
-            res = mkrel(nodeid).replace("::()", "")  # parens-normalization
+            res = mkrel(nodeid)
             if self.verbosity >= 2 and nodeid.split("::")[0] != fspath.replace(
                 "\\", nodes.SEP
             ):

testing/python/collect.py

@@ -1408,9 +1408,7 @@ def test_customize_through_attributes(testdir):
     """
     )
     result = testdir.runpytest("--collect-only")
-    result.stdout.fnmatch_lines(
+    result.stdout.fnmatch_lines(["*MyClass*", "*MyFunction*test_hello*"])
-        ["*MyClass*", "*MyInstance*", "*MyFunction*test_hello*"]
-    )
 
 
 def test_unorderable_types(testdir):

testing/test_collection.py

@@ -510,13 +510,8 @@ class TestSession(object):
                     pass
         """
         )
-        normid = p.basename + "::TestClass::()::test_method"
+        normid = p.basename + "::TestClass::test_method"
-        for id in [
+        for id in [p.basename, p.basename + "::TestClass", normid]:
-            p.basename,
-            p.basename + "::TestClass",
-            p.basename + "::TestClass::()",
-            normid,
-        ]:
             items, hookrec = testdir.inline_genitems(id)
             assert len(items) == 1
             assert items[0].name == "test_method"
@@ -625,7 +620,7 @@ class TestSession(object):
         items, hookrec = testdir.inline_genitems(arg)
         assert len(items) == 1
         item, = items
-        assert item.nodeid.endswith("TestClass::()::test_method")
+        assert item.nodeid.endswith("TestClass::test_method")
         # ensure we are reporting the collection of the single test item (#2464)
         assert [x.name for x in self.get_reported_items(hookrec)] == ["test_method"]
 

testing/test_nodes.py

@@ -8,11 +8,11 @@ from _pytest import nodes
         ("", "", True),
         ("", "foo", True),
         ("", "foo/bar", True),
-        ("", "foo/bar::TestBaz::()", True),
+        ("", "foo/bar::TestBaz", True),
         ("foo", "food", False),
-        ("foo/bar::TestBaz::()", "foo/bar", False),
+        ("foo/bar::TestBaz", "foo/bar", False),
-        ("foo/bar::TestBaz::()", "foo/bar::TestBop::()", False),
+        ("foo/bar::TestBaz", "foo/bar::TestBop", False),
-        ("foo/bar", "foo/bar::TestBop::()", True),
+        ("foo/bar", "foo/bar::TestBop", True),
     ),
 )
 def test_ischildnode(baseid, nodeid, expected):

testing/test_session.py

@@ -274,16 +274,26 @@ def test_deselect(testdir):
     testdir.makepyfile(
         test_a="""
         import pytest
+
         def test_a1(): pass
+
         @pytest.mark.parametrize('b', range(3))
         def test_a2(b): pass
+
+        class TestClass:
+            def test_c1(self): pass
+
+            def test_c2(self): pass
     """
     )
     result = testdir.runpytest(
-        "-v", "--deselect=test_a.py::test_a2[1]", "--deselect=test_a.py::test_a2[2]"
+        "-v",
+        "--deselect=test_a.py::test_a2[1]",
+        "--deselect=test_a.py::test_a2[2]",
+        "--deselect=test_a.py::TestClass::test_c1",
     )
     assert result.ret == 0
-    result.stdout.fnmatch_lines(["*2 passed, 2 deselected*"])
+    result.stdout.fnmatch_lines(["*3 passed, 3 deselected*"])
     for line in result.stdout.lines:
         assert not line.startswith(("test_a.py::test_a2[1]", "test_a.py::test_a2[2]"))