From 9a7ad7eaae1cc2f9ad5acc51d334f75dc1c5f366 Mon Sep 17 00:00:00 2001 From: perrygoy Date: Fri, 12 Jul 2024 15:13:00 -0500 Subject: [PATCH 1/4] 17 and 18: fix logging after .first etc. and make Targets look like Locators. --- screenpy_playwright/target.py | 35 +++++++++++++++++++++++++---------- tests/test_target.py | 25 ++++++++++++++++--------- 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/screenpy_playwright/target.py b/screenpy_playwright/target.py index 4d12daf..6b0b561 100644 --- a/screenpy_playwright/target.py +++ b/screenpy_playwright/target.py @@ -44,6 +44,10 @@ class _Manipulation(UserString): This class allows the ScreenPy Playwright Target to behave just like a Playwright Locator, which has a robust, chainable API for describing elements. + + This approach necessary because Locators are built from a Page base. We + don't have a Page to build from until the Actor is provided during the + :meth:`Target.found_by` call. """ target: Target @@ -73,7 +77,7 @@ def __call__( self.kwargs = kwargs return self.target - def __repr__(self) -> str: + def get_locator(self) -> str: """Reconstruct the locator function/attribute string.""" args = kwargs = left_paren = right_paren = comma = "" if self.args is not None or self.kwargs is not None: @@ -87,11 +91,15 @@ def __repr__(self) -> str: comma = ", " return f"{self.name}{left_paren}{args}{comma}{kwargs}{right_paren}" + def __repr__(self) -> str: + """Return the Target for representation.""" + return repr(self.target) + # make sure we handle str here as well __str__ = __repr__ -class Target: +class Target(Locator): """A described element on a webpage. Uses Playwright's Locator API to describe an element on a webpage, with a @@ -138,15 +146,22 @@ def in_frame(self, frame_locator: str) -> Target: ) return self - def __getattr__(self, name: str) -> _Manipulation: + def __getattribute__(self, name: str) -> _Manipulation: """Convert a Playwright Locator strategy into a Manipulation.""" - if not hasattr(Locator, name): - msg = f"'{name}' is not a valid Playwright Locator strategy." - raise AttributeError(msg) + if hasattr(Locator, name): + attr = getattr(Locator, name) + is_property = type(attr) is property + r_type = getattr(attr, "__annotations__", {}).get("return") + + if is_property or r_type is Locator or r_type == "Locator": + manipulation = _Manipulation(self, name) + self.manipulations.append(manipulation) + return manipulation + + msg = f"'{name}' cannot be accessed until `found_by` is called." + raise TargetingError(msg) - manipulation = _Manipulation(self, name) - self.manipulations.append(manipulation) - return manipulation + return super().__getattribute__(name) @property def target_name(self) -> str: @@ -161,7 +176,7 @@ def target_name(self) -> str: if self._description: target_name = self._description elif self.manipulations: - target_name = ".".join(map(repr, self.manipulations)) + target_name = ".".join(m.get_locator() for m in self.manipulations) else: target_name = "None" return target_name diff --git a/tests/test_target.py b/tests/test_target.py index df0b19a..be76560 100644 --- a/tests/test_target.py +++ b/tests/test_target.py @@ -36,11 +36,11 @@ def test_proper_display(self) -> None: m_with_args_but_no_kwargs = _Manipulation(Target(), name, args=args) m_with_no_args_but_kwargs = _Manipulation(Target(), name, kwargs=kwargs) - assert repr(m_for_attribute) == name - assert repr(m_with_neither) == f"{name}()" - assert repr(m_with_both) == f"{name}({args_str}, {kwargs_str})" - assert repr(m_with_args_but_no_kwargs) == f"{name}({args_str})" - assert repr(m_with_no_args_but_kwargs) == f"{name}({kwargs_str})" + assert m_for_attribute.get_locator() == name + assert m_with_neither.get_locator() == f"{name}()" + assert m_with_both.get_locator() == f"{name}({args_str}, {kwargs_str})" + assert m_with_args_but_no_kwargs.get_locator() == f"{name}({args_str})" + assert m_with_no_args_but_kwargs.get_locator() == f"{name}({kwargs_str})" def test_defers_to_target_for_unknown_attributes(self) -> None: target = Target.the("spam") @@ -71,12 +71,22 @@ def test_auto_describe(self) -> None: t3 = Target() t4 = Target("").get_by_label("baz") t5 = Target().located_by("foo").get_by_label("bar").first + t6 = Target("a dead parrot").last assert t1.target_name == "locator('#yellow')" assert t2.target_name == "favorite color" assert t3.target_name == "None" assert t4.target_name == "get_by_label('baz')" assert t5.target_name == "locator('foo').get_by_label('bar').first" + assert t6.target_name == "a dead parrot" + + def test_invalid_method_raises(self) -> None: + with pytest.raises(AttributeError): + Target("acquired").not_a_real_method() + + def test_method_too_soon_raises(self) -> None: + with pytest.raises(TargetingError): + Target("is having a FIRE sale! Oh god!! Help!!!").click() def test_found_by(self, Tester: Actor) -> None: test_locator = "#spam>baked-beans>eggs>sausage+spam" @@ -141,11 +151,8 @@ def test_found_by_chain(self, Tester: Actor) -> None: mocked_btws = Tester.ability_to(BrowseTheWebSynchronously) mocked_btws.current_page = mock.Mock() - manipulation = Target.the("test").located_by(test_locator).first - # mypy thinks this will be a Target. It will not be. - target = manipulation.get_by_label("foo") # type: ignore[operator] + target = Target.the("test").located_by(test_locator).first.get_by_label("foo") - assert isinstance(manipulation, _Manipulation) assert isinstance(target, Target) def test_found_by_raises_if_no_locator(self, Tester: Actor) -> None: From 6190e30c398e1a42bc6b739e2a43a9ec77a9e1be Mon Sep 17 00:00:00 2001 From: perrygoy Date: Fri, 12 Jul 2024 16:15:00 -0500 Subject: [PATCH 2/4] fix access to dunders (h/t @bandophahita). --- screenpy_playwright/target.py | 2 +- tests/test_target.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/screenpy_playwright/target.py b/screenpy_playwright/target.py index 6b0b561..196c219 100644 --- a/screenpy_playwright/target.py +++ b/screenpy_playwright/target.py @@ -148,7 +148,7 @@ def in_frame(self, frame_locator: str) -> Target: def __getattribute__(self, name: str) -> _Manipulation: """Convert a Playwright Locator strategy into a Manipulation.""" - if hasattr(Locator, name): + if not name.startswith("_") and hasattr(Locator, name): attr = getattr(Locator, name) is_property = type(attr) is property r_type = getattr(attr, "__annotations__", {}).get("return") diff --git a/tests/test_target.py b/tests/test_target.py index be76560..9d22fc4 100644 --- a/tests/test_target.py +++ b/tests/test_target.py @@ -88,6 +88,17 @@ def test_method_too_soon_raises(self) -> None: with pytest.raises(TargetingError): Target("is having a FIRE sale! Oh god!! Help!!!").click() + def test_dunders_are_accessible(self) -> None: + target = Target("Dunder Mifflin, this is Pam.") + redirected_target = Target().get_by_label("Dunder Mifflin, Jim speaking.") + manipulation = Target().first + + # dir accesses __dict__ behind the scenes + # (and is how this issue was found) + dir(target) + dir(redirected_target) + dir(manipulation) + def test_found_by(self, Tester: Actor) -> None: test_locator = "#spam>baked-beans>eggs>sausage+spam" mocked_btws = Tester.ability_to(BrowseTheWebSynchronously) From 986d6c17cdcc82a81ab71fbc95ff5e121221d042 Mon Sep 17 00:00:00 2001 From: perrygoy Date: Fri, 12 Jul 2024 17:25:01 -0500 Subject: [PATCH 3/4] use stubfile to override typing (h/t @bandophahita). --- pyproject.toml | 3 + screenpy_playwright/target.py | 28 +++++--- screenpy_playwright/target.pyi | 122 +++++++++++++++++++++++++++++++++ tests/test_target.py | 2 + 4 files changed, 147 insertions(+), 8 deletions(-) create mode 100644 screenpy_playwright/target.pyi diff --git a/pyproject.toml b/pyproject.toml index 952f526..3121fb2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -123,6 +123,9 @@ split-on-trailing-comma = false "PLR", # likewise using specific numbers and strings in tests. ] "__version__.py" = ["D"] +"*.pyi" = [ + "PLR0913", # it's not our fault they have too many parameters! +] ################################################################################ diff --git a/screenpy_playwright/target.py b/screenpy_playwright/target.py index 196c219..50ea6b7 100644 --- a/screenpy_playwright/target.py +++ b/screenpy_playwright/target.py @@ -6,7 +6,7 @@ from dataclasses import dataclass from typing import TYPE_CHECKING, Pattern, Tuple, TypedDict, Union -from playwright.sync_api import Locator +from playwright.sync_api import FrameLocator, Locator from .abilities import BrowseTheWebSynchronously from .exceptions import TargetingError @@ -99,7 +99,7 @@ def __repr__(self) -> str: __str__ = __repr__ -class Target(Locator): +class Target(Locator, FrameLocator): """A described element on a webpage. Uses Playwright's Locator API to describe an element on a webpage, with a @@ -148,12 +148,16 @@ def in_frame(self, frame_locator: str) -> Target: def __getattribute__(self, name: str) -> _Manipulation: """Convert a Playwright Locator strategy into a Manipulation.""" - if not name.startswith("_") and hasattr(Locator, name): - attr = getattr(Locator, name) + is_private = name.startswith("_") + superclass_has_attr = hasattr(Locator, name) or hasattr(FrameLocator, name) + if not is_private and superclass_has_attr: + attr = getattr(Locator, name) or getattr(FrameLocator, name) is_property = type(attr) is property r_type = getattr(attr, "__annotations__", {}).get("return") + is_right_type = r_type is Locator or r_type is FrameLocator + is_right_str = r_type in ["Locator", "FrameLocator"] - if is_property or r_type is Locator or r_type == "Locator": + if is_property or is_right_type or is_right_str: manipulation = _Manipulation(self, name) self.manipulations.append(manipulation) return manipulation @@ -211,9 +215,17 @@ def found_by(self, the_actor: Actor) -> Locator: if manipulation.args is None and manipulation.kwargs is None: locator = getattr(locator, manipulation.name) else: - locator = getattr(locator, manipulation.name)( - *manipulation.args, **manipulation.kwargs - ) + args = [] + for arg in manipulation.args: + arg_to_append = arg + if isinstance(arg, Target): + arg_to_append = arg.found_by(the_actor) + args.append(arg_to_append) + kwargs = { + k: v.found_by(the_actor) if isinstance(v, Target) else v + for k, v in manipulation.kwargs.items() + } + locator = getattr(locator, manipulation.name)(*args, **kwargs) return locator diff --git a/screenpy_playwright/target.pyi b/screenpy_playwright/target.pyi new file mode 100644 index 0000000..434f8c0 --- /dev/null +++ b/screenpy_playwright/target.pyi @@ -0,0 +1,122 @@ +"""Used to overwrite type hints for Locator methods from Playwright. + +Hopefully this is only temporary. -pgoy 2024-JUL-12 +""" + +from collections import UserString +from dataclasses import dataclass +from typing import Pattern, TypedDict + +from playwright.sync_api import FrameLocator, Locator +from screenpy import Actor as Actor +from typing_extensions import NotRequired, Self, Unpack + +from .abilities import BrowseTheWebSynchronously as BrowseTheWebSynchronously +from .exceptions import TargetingError as TargetingError + +_ManipulationArgsType = tuple[str | int | None, ...] + +class _ManipulationKwargsType(TypedDict): + has_text: NotRequired[str | Pattern[str] | None] + has_not_text: NotRequired[str | Pattern[str] | None] + has: NotRequired[Locator | None] + has_not: NotRequired[Locator | None] + exact: NotRequired[bool | None] + checked: NotRequired[bool | None] + disabled: NotRequired[bool | None] + expanded: NotRequired[bool | None] + include_hidden: NotRequired[bool | None] + level: NotRequired[int | None] + name: NotRequired[str | Pattern[str] | None] + pressed: NotRequired[bool | None] + selected: NotRequired[bool | None] + +@dataclass +class _Manipulation(UserString): + def __hash__(self) -> int: ... + def __eq__(self, other: object) -> bool: ... + def __getattr__(self, name: str) -> Target | _Manipulation: ... + def __call__( + self, + *args: Unpack[_ManipulationArgsType], + **kwargs: Unpack[_ManipulationKwargsType], + ) -> Target: ... + def get_locator(self) -> str: ... + def __init__( + self, + target: Target, + name: str, + args: _ManipulationArgsType | None = ..., + kwargs: _ManipulationKwargsType | None = ..., + ) -> None: ... + +class Target(FrameLocator, Locator): + manipulations: list[_Manipulation] + first: Target + last: Target + owner: Target + content_frame: Target + @classmethod + def the(cls, name: str) -> Self: ... + def located_by(self, locator: str) -> Target: ... + def in_frame(self, frame_locator: str) -> Target: ... + def __getattribute__(self, name: str) -> _Manipulation: ... + @property + def target_name(self) -> str: ... + @target_name.setter + def target_name(self, value: str) -> None: ... + def found_by(self, the_actor: Actor) -> Locator: ... + def __init__(self, name: str | None = None) -> None: ... + + # overridden Locator methods + def and_(self, locator: Locator | Target) -> Target: ... + def filter( + self, + *, + has_text: str | Pattern | None = None, + has_not_text: str | Pattern | None = None, + has: Locator | Target | None = None, + has_not: Locator | Target | None = None, + ) -> Target: ... + def frame_locator(self, selector: str) -> Target: ... + def get_by_alt_text( + self, text: str | Pattern, *, exact: bool | None = None + ) -> Target: ... + def get_by_label( + self, text: str | Pattern, *, exact: bool | None = None + ) -> Target: ... + def get_by_placeholder( + self, text: str | Pattern, *, exact: bool | None = None + ) -> Target: ... + def get_by_role( + self, + role: str, + *, + checked: bool | None = None, + disabled: bool | None = None, + expanded: bool | None = None, + include_hidden: bool | None = None, + level: int | None = None, + name: str | Pattern | None = None, + pressed: bool | None = None, + selected: bool | None = None, + exact: bool | None = None, + ) -> Target: ... + def get_by_test_id(self, test_id: str | Pattern) -> Target: ... + def get_by_text( + self, text: str | Pattern, *, exact: bool | None = None + ) -> Target: ... + def get_by_title( + self, text: str | Pattern, *, exact: bool | None = None + ) -> Target: ... + def locator( + self, + selector: str | Locator, + *, + has_text: str | Pattern | None = None, + has_not_text: str | Pattern | None = None, + has: Locator | Target | None = None, + has_not: Locator | Target | None = None, + ) -> Target: ... + def nth(self, index: int) -> Target: ... + def or_(self, locator: Locator | Target) -> Target: ... diff --git a/tests/test_target.py b/tests/test_target.py index 9d22fc4..3167648 100644 --- a/tests/test_target.py +++ b/tests/test_target.py @@ -57,6 +57,7 @@ def test_can_be_instantiated(self) -> None: t4 = Target().located_by("test") t5 = Target() t6 = Target("test").get_by_label("test", exact=True) + t7 = Target("frame").frame_locator("test").content_frame assert isinstance(t1, Target) assert isinstance(t2, Target) @@ -64,6 +65,7 @@ def test_can_be_instantiated(self) -> None: assert isinstance(t4, Target) assert isinstance(t5, Target) assert isinstance(t6, Target) + assert isinstance(t7, Target) def test_auto_describe(self) -> None: t1 = Target().located_by("#yellow") From 3a9f64d7a52bee7e4112d600ab8bfab54076fd64 Mon Sep 17 00:00:00 2001 From: perrygoy Date: Tue, 16 Jul 2024 15:29:48 -0500 Subject: [PATCH 4/4] remove incorrect test. --- tests/test_target.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_target.py b/tests/test_target.py index 3167648..9d22fc4 100644 --- a/tests/test_target.py +++ b/tests/test_target.py @@ -57,7 +57,6 @@ def test_can_be_instantiated(self) -> None: t4 = Target().located_by("test") t5 = Target() t6 = Target("test").get_by_label("test", exact=True) - t7 = Target("frame").frame_locator("test").content_frame assert isinstance(t1, Target) assert isinstance(t2, Target) @@ -65,7 +64,6 @@ def test_can_be_instantiated(self) -> None: assert isinstance(t4, Target) assert isinstance(t5, Target) assert isinstance(t6, Target) - assert isinstance(t7, Target) def test_auto_describe(self) -> None: t1 = Target().located_by("#yellow")