From ac8b9c6e9dbb66114a1c3f79cd1fdeb0b2b2c54d Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 9 Nov 2018 11:03:07 +0100 Subject: [PATCH 1/3] Node: do not add "::()" to nodeid Fixes https://github.com/pytest-dev/pytest/issues/4127. --- changelog/4358.removal.rst | 16 ++++++++++++++++ src/_pytest/nodes.py | 12 +++++++----- src/_pytest/runner.py | 3 +-- src/_pytest/terminal.py | 6 ++---- testing/test_collection.py | 11 +++-------- testing/test_nodes.py | 8 ++++---- 6 files changed, 33 insertions(+), 23 deletions(-) create mode 100644 changelog/4358.removal.rst diff --git a/changelog/4358.removal.rst b/changelog/4358.removal.rst new file mode 100644 index 00000000000..1a562cfe769 --- /dev/null +++ b/changelog/4358.removal.rst @@ -0,0 +1,16 @@ +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 `_). diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 0d65dc96537..a559b55b538 100644 --- a/src/_pytest/nodes.py +++ b/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 @@ def __init__( 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.name + self._nodeid = self.parent.nodeid + if self.name != "()": + self._nodeid += "::" + self.name @property def ihook(self): diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py index 4d4b06d7cb3..4693be8071b 100644 --- a/src/_pytest/runner.py +++ b/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, nodeid)) + tr.write_line("%02.2fs %-8s %s" % (rep.duration, rep.when, rep.nodeid)) def pytest_sessionstart(session): diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index dde2750be7e..520caf86288 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -605,9 +605,7 @@ def _printcollecteditems(self, items): self._tw.line("%s: %d" % (name, count)) else: for item in items: - nodeid = item.nodeid - nodeid = nodeid.replace("::()::", "::") - self._tw.line(nodeid) + self._tw.line(item.nodeid) return stack = [] indent = "" @@ -687,7 +685,7 @@ def mkrel(nodeid): # 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 ): diff --git a/testing/test_collection.py b/testing/test_collection.py index 3d65b21dc9f..db2497c0eb2 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -510,13 +510,8 @@ def test_method(self): pass """ ) - normid = p.basename + "::TestClass::()::test_method" - for id in [ - p.basename, - p.basename + "::TestClass", - p.basename + "::TestClass::()", - normid, - ]: + normid = p.basename + "::TestClass::test_method" + for id in [p.basename, p.basename + "::TestClass", normid]: items, hookrec = testdir.inline_genitems(id) assert len(items) == 1 assert items[0].name == "test_method" @@ -625,7 +620,7 @@ def test_method(self): 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"] diff --git a/testing/test_nodes.py b/testing/test_nodes.py index d55184ef221..cc1ee1583ec 100644 --- a/testing/test_nodes.py +++ b/testing/test_nodes.py @@ -8,11 +8,11 @@ ("", "", 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::TestBop::()", False), - ("foo/bar", "foo/bar::TestBop::()", True), + ("foo/bar::TestBaz", "foo/bar", False), + ("foo/bar::TestBaz", "foo/bar::TestBop", False), + ("foo/bar", "foo/bar::TestBop", True), ), ) def test_ischildnode(baseid, nodeid, expected): From 87254ca59352124afecf13d3cbf62268ae54dd98 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 9 Nov 2018 11:28:51 +0100 Subject: [PATCH 2/3] Add test for --deselect without "::()" Closes: https://github.com/pytest-dev/pytest/issues/4127. --- testing/test_session.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/testing/test_session.py b/testing/test_session.py index c1785b91668..fed39195cf5 100644 --- a/testing/test_session.py +++ b/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]")) From f551cb9677bfd14829a403cbd583d590c2d49ce8 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Fri, 9 Nov 2018 11:32:44 +0100 Subject: [PATCH 3/3] Skip Instances with --collect-only --- changelog/4358.removal.rst | 2 ++ src/_pytest/terminal.py | 4 ++-- testing/python/collect.py | 4 +--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/changelog/4358.removal.rst b/changelog/4358.removal.rst index 1a562cfe769..d66fd483b39 100644 --- a/changelog/4358.removal.rst +++ b/changelog/4358.removal.rst @@ -14,3 +14,5 @@ it was not really possible to detect when the functionality was being used expli 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 `_). + +Test class instances are also not listed with ``--collect-only`` anymore. diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index 520caf86288..f19325088a5 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -617,8 +617,8 @@ def _printcollecteditems(self, items): stack.pop() for col in needed_collectors[len(stack) :]: stack.append(col) - # if col.name == "()": - # continue + if col.name == "()": # Skip Instances. + continue indent = (len(stack) - 1) * " " self._tw.line("%s%s" % (indent, col)) diff --git a/testing/python/collect.py b/testing/python/collect.py index 61039a50678..ffe4a727176 100644 --- a/testing/python/collect.py +++ b/testing/python/collect.py @@ -1408,9 +1408,7 @@ def test_hello(self): """ ) result = testdir.runpytest("--collect-only") - result.stdout.fnmatch_lines( - ["*MyClass*", "*MyInstance*", "*MyFunction*test_hello*"] - ) + result.stdout.fnmatch_lines(["*MyClass*", "*MyFunction*test_hello*"]) def test_unorderable_types(testdir):