Merge pull request #19 from ScreenPyHQ/17and18/fix-target-weirdness

17 and 18: Fix Target weirdness.

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!
+]
 
 
 ################################################################################

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
@@ -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, FrameLocator):
     """A described element on a webpage.
 
     Uses Playwright's Locator API to describe an element on a webpage, with a
@@ -138,15 +146,26 @@ 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)
+        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 is_right_type or is_right_str:
+                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 +180,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
@@ -196,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
 

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: ...

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,33 @@ 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_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"
@@ -141,11 +162,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: