Fix handler wrapping for keyword-only parameters (#3084)

Author: rohan-patnaik

playwright/_impl/_impl_to_api_mapping.py

@@ -119,7 +119,23 @@ def to_impl(
 
     def wrap_handler(self, handler: Callable[..., Any]) -> Callable[..., None]:
         def wrapper_func(*args: Any) -> Any:
-            arg_count = len(inspect.signature(handler).parameters)
+            parameters = inspect.signature(handler).parameters
+            has_varargs = any(
+                parameter.kind == inspect.Parameter.VAR_POSITIONAL
+                for parameter in parameters.values()
+            )
+            arg_count = (
+                len(args)
+                if has_varargs
+                else sum(
+                    parameter.kind
+                    in (
+                        inspect.Parameter.POSITIONAL_ONLY,
+                        inspect.Parameter.POSITIONAL_OR_KEYWORD,
+                    )
+                    for parameter in parameters.values()
+                )
+            )
             return handler(
                 *list(map(lambda a: self.from_maybe_impl(a), args))[:arg_count]
             )

tests/async/test_page_add_locator_handler.py

@@ -91,6 +91,57 @@ async def handler() -> None:
         await expect(page.locator("#interstitial")).not_to_be_visible()
 
 
+async def test_should_work_with_keyword_only_handler(
+    page: Page, server: Server
+) -> None:
+    await page.goto(server.PREFIX + "/input/handle-locator.html")
+
+    called = False
+
+    async def handler(*, timeout: object = None) -> None:
+        nonlocal called
+        assert timeout is None
+        called = True
+        await page.locator("#close").click()
+
+    await page.add_locator_handler(
+        page.get_by_text("This interstitial covers the button"), handler
+    )
+
+    await page.locator("#aside").hover()
+    await page.evaluate(
+        '() => { window.clicked = 0; window.setupAnnoyingInterstitial("mouseover", 1); }'
+    )
+    await page.locator("#target").click()
+    assert called
+    assert await page.evaluate("window.clicked") == 1
+    await expect(page.locator("#interstitial")).not_to_be_visible()
+
+
+async def test_should_work_with_varargs_handler(page: Page, server: Server) -> None:
+    await page.goto(server.PREFIX + "/input/handle-locator.html")
+
+    called = False
+    original_locator = page.get_by_text("This interstitial covers the button")
+
+    async def handler(*locators: Locator) -> None:
+        nonlocal called
+        assert locators == (original_locator,)
+        called = True
+        await page.locator("#close").click()
+
+    await page.add_locator_handler(original_locator, handler)
+
+    await page.locator("#aside").hover()
+    await page.evaluate(
+        '() => { window.clicked = 0; window.setupAnnoyingInterstitial("mouseover", 1); }'
+    )
+    await page.locator("#target").click()
+    assert called
+    assert await page.evaluate("window.clicked") == 1
+    await expect(page.locator("#interstitial")).not_to_be_visible()
+
+
 async def test_should_work_with_locator_hover(page: Page, server: Server) -> None:
     await page.goto(server.PREFIX + "/input/handle-locator.html")
 

tests/sync/test_page_add_locator_handler.py

@@ -90,6 +90,55 @@ def handler() -> None:
         expect(page.locator("#interstitial")).not_to_be_visible()
 
 
+def test_should_work_with_keyword_only_handler(page: Page, server: Server) -> None:
+    page.goto(server.PREFIX + "/input/handle-locator.html")
+
+    called = False
+
+    def handler(*, timeout: object = None) -> None:
+        nonlocal called
+        assert timeout is None
+        called = True
+        page.locator("#close").click()
+
+    page.add_locator_handler(
+        page.get_by_text("This interstitial covers the button"), handler
+    )
+
+    page.locator("#aside").hover()
+    page.evaluate(
+        '() => { window.clicked = 0; window.setupAnnoyingInterstitial("mouseover", 1); }'
+    )
+    page.locator("#target").click()
+    assert called
+    assert page.evaluate("window.clicked") == 1
+    expect(page.locator("#interstitial")).not_to_be_visible()
+
+
+def test_should_work_with_varargs_handler(page: Page, server: Server) -> None:
+    page.goto(server.PREFIX + "/input/handle-locator.html")
+
+    called = False
+    original_locator = page.get_by_text("This interstitial covers the button")
+
+    def handler(*locators: Locator) -> None:
+        nonlocal called
+        assert locators == (original_locator,)
+        called = True
+        page.locator("#close").click()
+
+    page.add_locator_handler(original_locator, handler)
+
+    page.locator("#aside").hover()
+    page.evaluate(
+        '() => { window.clicked = 0; window.setupAnnoyingInterstitial("mouseover", 1); }'
+    )
+    page.locator("#target").click()
+    assert called
+    assert page.evaluate("window.clicked") == 1
+    expect(page.locator("#interstitial")).not_to_be_visible()
+
+
 def test_should_work_with_locator_hover(page: Page, server: Server) -> None:
     page.goto(server.PREFIX + "/input/handle-locator.html")