From 4e5fb520b620b9d50479b8e4e9ac85863f1558c0 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 6 Oct 2021 22:39:49 +0300 Subject: [PATCH 1/8] python: remove an unneeded normalize_mark_list call `callspec.mark` is already `List[Mark]` so no need to normalize it. --- src/_pytest/python.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 513214e08..98c28b3d2 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1666,7 +1666,7 @@ class Function(PyobjMixin, nodes.Item): # feel free to cry, this was broken for years before # and keywords can't fix it per design self.keywords[mark.name] = mark - self.own_markers.extend(normalize_mark_list(callspec.marks)) + self.own_markers.extend(callspec.marks) if keywords: self.keywords.update(keywords) From e9bb1aa233c882de53172cdd5e762a8a49e427a7 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Thu, 7 Oct 2021 00:42:43 +0300 Subject: [PATCH 2/8] python: be consistent with what value marks have in keywords Marks are added to keywords in three places: - `Node.add_marker`: name -> `Mark` - `Function.__init__(callspec)`: name -> `Mark` - `Function.__init__ iter_markers`: name -> True I think it should be consistent, which will also help with some upcoming code cleaning. The `Mark` seems more useful than just a `True`, so switch to that. --- src/_pytest/python.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 98c28b3d2..d07c01d7b 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1675,7 +1675,7 @@ class Function(PyobjMixin, nodes.Item): self.keywords.update( { - mark.name: True + mark.name: mark for mark in self.iter_markers() if mark.name not in self.keywords } From 8713c3246212b04ae75739a02c72c581ea5570d7 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Thu, 7 Oct 2021 00:37:19 +0300 Subject: [PATCH 3/8] python: unpacked marks need to be added to keywords on all node types (except `Instance`) Currently, `Function` does this manually, but other node types don't get their markers added to their `keywords`, but they should, if only for consistency. --- src/_pytest/python.py | 3 +++ testing/test_collection.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index d07c01d7b..71c998c8e 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -303,6 +303,9 @@ class PyobjMixin(nodes.Node): # used to avoid Function marker duplication if self._ALLOW_MARKERS: self.own_markers.extend(get_unpacked_marks(self.obj)) + # This assumes that `obj` is called before there is a chance + # to add custom keys to `self.keywords`, so no fear of overriding. + self.keywords.update({mark.name: mark for mark in self.own_markers}) return obj @obj.setter diff --git a/testing/test_collection.py b/testing/test_collection.py index 6a8a5c1ce..e79ae384d 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -881,6 +881,36 @@ class TestNodeKeywords: assert item.keywords["kw"] == "method" assert len(item.keywords) == len(set(item.keywords)) + def test_unpacked_marks_added_to_keywords(self, pytester: Pytester) -> None: + item = pytester.getitem( + """ + import pytest + pytestmark = pytest.mark.foo + class TestClass: + pytestmark = pytest.mark.bar + def test_method(self): pass + test_method.pytestmark = pytest.mark.baz + """, + "test_method", + ) + assert isinstance(item, pytest.Function) + cls = item.getparent(pytest.Class) + assert cls is not None + mod = item.getparent(pytest.Module) + assert mod is not None + + assert item.keywords["foo"] == pytest.mark.foo.mark + assert item.keywords["bar"] == pytest.mark.bar.mark + assert item.keywords["baz"] == pytest.mark.baz.mark + + assert cls.keywords["foo"] == pytest.mark.foo.mark + assert cls.keywords["bar"] == pytest.mark.bar.mark + assert "baz" not in cls.keywords + + assert mod.keywords["foo"] == pytest.mark.foo.mark + assert "bar" not in mod.keywords + assert "baz" not in mod.keywords + COLLECTION_ERROR_PY_FILES = dict( test_01_failure=""" From d9bcfa0c2bb0ebf8e0a59c8296625a1eca77da05 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 6 Oct 2021 22:41:04 +0300 Subject: [PATCH 4/8] python: don't redundantly duplicate parent markers to own keywords This does have a slight semantic change: in a node hierarchy parent -> child, if parent has a marker applied, then child is constructed, then `parent.themarker = "overridden"`, previously `child.keywords['themarker']` would return `True`, now it returns `"overridden"`. But that's actually what I would have expected so I see it as more of a bugfix. --- src/_pytest/python.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 71c998c8e..60b2ae20f 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1676,10 +1676,11 @@ class Function(PyobjMixin, nodes.Item): # todo: this is a hell of a hack # https://github.com/pytest-dev/pytest/issues/4569 + # Take own_markers only; NodeKeywords handles parent traversal on its own. self.keywords.update( { mark.name: mark - for mark in self.iter_markers() + for mark in self.own_markers if mark.name not in self.keywords } ) From 3c69bc919c1cc5c45f52b90ef1ebdd136952d577 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 6 Oct 2021 22:59:34 +0300 Subject: [PATCH 5/8] python: remove broken/ineffectual keywords marks initialization By my analysis, this deleted code block has no effect: 1. `self.keywords` is `update`d with `callspec.marks`. 2. `self.own_markers` is `update`d with `callspec.marks`. 3. `self.keywords` is `update`d with `self.own_markers`. So together steps 2+3 completely undo step 1. --- src/_pytest/python.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 60b2ae20f..63701a535 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1662,13 +1662,6 @@ class Function(PyobjMixin, nodes.Item): self.own_markers.extend(get_unpacked_marks(self.obj)) if callspec: self.callspec = callspec - # this is total hostile and a mess - # keywords are broken by design by now - # this will be redeemed later - for mark in callspec.marks: - # feel free to cry, this was broken for years before - # and keywords can't fix it per design - self.keywords[mark.name] = mark self.own_markers.extend(callspec.marks) if keywords: self.keywords.update(keywords) From 456a2538acfe72ba080e1beaf907cbea822153c8 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 6 Oct 2021 22:50:36 +0300 Subject: [PATCH 6/8] python: optimize node keywords initialization If we do the `update`s in the right order, we can avoid the `mark.name not in self.keywords` check, since `self.keywords` starts out clean and `update` will override previously set keywords. --- src/_pytest/python.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 63701a535..b71f1bbfc 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1658,25 +1658,20 @@ class Function(PyobjMixin, nodes.Item): # Note: when FunctionDefinition is introduced, we should change ``originalname`` # to a readonly property that returns FunctionDefinition.name. - self.keywords.update(self.obj.__dict__) self.own_markers.extend(get_unpacked_marks(self.obj)) if callspec: self.callspec = callspec self.own_markers.extend(callspec.marks) - if keywords: - self.keywords.update(keywords) # todo: this is a hell of a hack # https://github.com/pytest-dev/pytest/issues/4569 - + # Note: the order of the updates is important here; indicates what + # takes priority (ctor argument over function attributes over markers). # Take own_markers only; NodeKeywords handles parent traversal on its own. - self.keywords.update( - { - mark.name: mark - for mark in self.own_markers - if mark.name not in self.keywords - } - ) + self.keywords.update({mark.name: mark for mark in self.own_markers}) + self.keywords.update(self.obj.__dict__) + if keywords: + self.keywords.update(keywords) if fixtureinfo is None: fixtureinfo = self.session._fixturemanager.getfixtureinfo( From 74571ba55f66cba9ed18945212af4afab6a2cab6 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 6 Oct 2021 23:29:48 +0300 Subject: [PATCH 7/8] Add missing `keywords` type annotations --- src/_pytest/python.py | 2 +- src/_pytest/reports.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index b71f1bbfc..9b1d3119b 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1637,7 +1637,7 @@ class Function(PyobjMixin, nodes.Item): config: Optional[Config] = None, callspec: Optional[CallSpec2] = None, callobj=NOTSET, - keywords=None, + keywords: Optional[Mapping[str, Any]] = None, session: Optional[Session] = None, fixtureinfo: Optional[FuncFixtureInfo] = None, originalname: Optional[str] = None, diff --git a/src/_pytest/reports.py b/src/_pytest/reports.py index a68e68bc5..725fdf617 100644 --- a/src/_pytest/reports.py +++ b/src/_pytest/reports.py @@ -7,6 +7,7 @@ from typing import Dict from typing import Iterable from typing import Iterator from typing import List +from typing import Mapping from typing import Optional from typing import Tuple from typing import Type @@ -254,7 +255,7 @@ class TestReport(BaseReport): self, nodeid: str, location: Tuple[str, Optional[int], str], - keywords, + keywords: Mapping[str, Any], outcome: "Literal['passed', 'failed', 'skipped']", longrepr: Union[ None, ExceptionInfo[BaseException], Tuple[str, int, str], str, TerminalRepr From 6d128cd52ef70e401999acda8d45dd5f6139b071 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Thu, 7 Oct 2021 01:15:22 +0300 Subject: [PATCH 8/8] python: use a more memory-friendly generator --- src/_pytest/python.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 9b1d3119b..6fda4d544 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -305,7 +305,7 @@ class PyobjMixin(nodes.Node): self.own_markers.extend(get_unpacked_marks(self.obj)) # This assumes that `obj` is called before there is a chance # to add custom keys to `self.keywords`, so no fear of overriding. - self.keywords.update({mark.name: mark for mark in self.own_markers}) + self.keywords.update((mark.name, mark) for mark in self.own_markers) return obj @obj.setter @@ -1668,7 +1668,7 @@ class Function(PyobjMixin, nodes.Item): # Note: the order of the updates is important here; indicates what # takes priority (ctor argument over function attributes over markers). # Take own_markers only; NodeKeywords handles parent traversal on its own. - self.keywords.update({mark.name: mark for mark in self.own_markers}) + self.keywords.update((mark.name, mark) for mark in self.own_markers) self.keywords.update(self.obj.__dict__) if keywords: self.keywords.update(keywords)