Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ This is the recurring high-stakes task. Use the dedicated skill:

It documents the full process: the upstream commit-range diff over `docs/src/api/`, how to classify each commit (PORT / MISMATCH / N/A), how to handle the `langs:` filter, the recurring failure modes, and the tests/sync-mirroring conventions.

## Working on PRs

- Never post comments, replies, or reviews on GitHub PRs/issues under my account without my explicit approval. Draft the proposed text and wait for me to approve before sending.

## House style

- Don't hand-edit generated files.
Expand Down
9 changes: 7 additions & 2 deletions playwright/_impl/_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from typing import (
TYPE_CHECKING,
Any,
Awaitable,
Callable,
Dict,
List,
Expand Down Expand Up @@ -54,8 +55,12 @@
from playwright._impl._network import Request, Response, Route, WebSocketRoute

URLMatch = Union[str, Pattern[str], Callable[[str], bool]]
URLMatchRequest = Union[str, Pattern[str], Callable[["Request"], bool]]
URLMatchResponse = Union[str, Pattern[str], Callable[["Response"], bool]]
URLMatchRequest = Union[
str, Pattern[str], Callable[["Request"], Union[bool, Awaitable[bool]]]
]
URLMatchResponse = Union[
str, Pattern[str], Callable[["Response"], Union[bool, Awaitable[bool]]]
]
RouteHandlerCallback = Union[
Callable[["Route"], Any], Callable[["Route", "Request"], Any]
]
Expand Down
5 changes: 3 additions & 2 deletions playwright/_impl/_page.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from typing import (
TYPE_CHECKING,
Any,
Awaitable,
Callable,
Dict,
List,
Expand Down Expand Up @@ -1278,7 +1279,7 @@ def expect_request(
urlOrPredicate: URLMatchRequest,
timeout: float = None,
) -> EventContextManagerImpl[Request]:
def my_predicate(request: Request) -> bool:
def my_predicate(request: Request) -> Union[bool, Awaitable[bool]]:
if not callable(urlOrPredicate):
return url_matches(
self._browser_context._base_url,
Expand Down Expand Up @@ -1310,7 +1311,7 @@ def expect_response(
urlOrPredicate: URLMatchResponse,
timeout: float = None,
) -> EventContextManagerImpl[Response]:
def my_predicate(request: Response) -> bool:
def my_predicate(request: Response) -> Union[bool, Awaitable[bool]]:
if not callable(urlOrPredicate):
return url_matches(
self._browser_context._base_url,
Expand Down
44 changes: 39 additions & 5 deletions playwright/_impl/_waiter.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@
# limitations under the License.

import asyncio
import inspect
import math
import uuid
from asyncio.tasks import Task
from typing import Any, Callable, List, Tuple, Union
from typing import Any, Callable, List, Optional, Tuple, Union

from pyee import EventEmitter

Expand Down Expand Up @@ -71,9 +72,11 @@ def reject_on_event(
error: Union[Error, Callable[..., Error]],
predicate: Callable = None,
) -> None:
def on_match() -> None:
self._reject(error() if callable(error) else error)

def listener(event_data: Any = None) -> None:
if not predicate or predicate(event_data):
self._reject(error() if callable(error) else error)
self._evaluate_predicate(predicate, event_data, on_match)

emitter.on(event, listener)
self._registered_listeners.append((emitter, event, listener))
Expand Down Expand Up @@ -117,12 +120,43 @@ def wait_for_event(
predicate: Callable = None,
) -> None:
def listener(event_data: Any = None) -> None:
if not predicate or predicate(event_data):
self._fulfill(event_data)
self._evaluate_predicate(
predicate, event_data, lambda: self._fulfill(event_data)
)

emitter.on(event, listener)
self._registered_listeners.append((emitter, event, listener))

def _evaluate_predicate(
self,
predicate: Optional[Callable],
event_data: Any,
on_match: Callable[[], None],
) -> None:
if predicate is None:
on_match()
return
try:
result = predicate(event_data)
except Exception as e:
self._reject(e)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this line a separate bugfix? I don't see where we would reject after a sync predicate throws before this PR. Perhaps add a test for this as well?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — yes, this is a coupled bugfix. Previously sync predicates that threw were silently swallowed by pyee's listener machinery, leaving the expect_* call to hang until timeout. Pushed a test (test_expect_response_should_reject_when_sync_predicate_throws) covering it.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cross-checked against JS waiter.ts's waitForEvent (packages/playwright-core/src/client/waiter.ts:113-124) — JS already awaits predicates and rejects on either sync or async predicate exceptions (try/catch wraps await predicate(eventArg)). This PR brings Python to parity.

return
if inspect.iscoroutine(result):

async def _await_predicate(coro: Any) -> None:
try:
matched = await coro
except Exception as e:
self._reject(e)
return
if matched and not self._result.done():
on_match()

self._pending_tasks.append(self._loop.create_task(_await_predicate(result)))
return
if result:
on_match()

def result(self) -> asyncio.Future:
return self._result

Expand Down
12 changes: 8 additions & 4 deletions playwright/async_api/_generated.py
Original file line number Diff line number Diff line change
Expand Up @@ -12306,7 +12306,9 @@ def expect_popup(
def expect_request(
self,
url_or_predicate: typing.Union[
str, typing.Pattern[str], typing.Callable[["Request"], bool]
str,
typing.Pattern[str],
typing.Callable[["Request"], typing.Union[bool, typing.Awaitable[bool]]],
],
*,
timeout: typing.Optional[float] = None,
Expand All @@ -12331,7 +12333,7 @@ def expect_request(

Parameters
----------
url_or_predicate : Union[Callable[[Request], bool], Pattern[str], str]
url_or_predicate : Union[Callable[[Request], Union[bool, typing.Awaitable[bool]]], Pattern[str], str]
Request URL string, regex or predicate receiving `Request` object. When a `baseURL` via the context options was
provided and the passed URL is a path, it gets merged via the
[`new URL()`](https://developer.mozilla.org/en-US/docs/Web/API/URL/URL) constructor.
Expand Down Expand Up @@ -12384,7 +12386,9 @@ def expect_request_finished(
def expect_response(
self,
url_or_predicate: typing.Union[
str, typing.Pattern[str], typing.Callable[["Response"], bool]
str,
typing.Pattern[str],
typing.Callable[["Response"], typing.Union[bool, typing.Awaitable[bool]]],
],
*,
timeout: typing.Optional[float] = None,
Expand All @@ -12411,7 +12415,7 @@ def expect_response(

Parameters
----------
url_or_predicate : Union[Callable[[Response], bool], Pattern[str], str]
url_or_predicate : Union[Callable[[Response], Union[bool, typing.Awaitable[bool]]], Pattern[str], str]
Request URL string, regex or predicate receiving `Response` object. When a `baseURL` via the context options was
provided and the passed URL is a path, it gets merged via the
[`new URL()`](https://developer.mozilla.org/en-US/docs/Web/API/URL/URL) constructor.
Expand Down
10 changes: 10 additions & 0 deletions scripts/documentation_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,16 @@ def serialize_python_type(self, value: Any, direction: str) -> str:
return f"{{{', '.join(signature)}}}"
if origin == Union:
args = get_args(value)
if not self.is_async:
# Sync API doesn't accept awaitable callbacks; drop the
# Awaitable arm so docstring types match the sync signature.
args = tuple(
a
for a in args
if str(get_origin(a)) != "<class 'collections.abc.Awaitable'>"
)
if len(args) == 1:
return self.serialize_python_type(args[0], direction)
if len(args) == 2 and str(args[1]) == "<class 'NoneType'>":
return self.make_optional(
self.serialize_python_type(args[0], direction)
Expand Down
4 changes: 4 additions & 0 deletions scripts/expected_api_mismatch.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,7 @@ Parameter type mismatch in BrowserContext.route_web_socket(handler=): documented
Parameter type mismatch in Page.route_web_socket(handler=): documented as Callable[[WebSocketRoute], Union[Any, Any]], code has Callable[[WebSocketRoute], Any]
Parameter type mismatch in WebSocketRoute.on_close(handler=): documented as Callable[[Union[int, undefined]], Union[Any, Any]], code has Callable[[Union[int, None], Union[str, None]], Any]
Parameter type mismatch in WebSocketRoute.on_message(handler=): documented as Callable[[str], Union[Any, Any]], code has Callable[[Union[bytes, str]], Any]

# Async API additionally accepts an `async def` predicate.
Parameter type mismatch in Page.expect_request(url_or_predicate=): documented as Union[Callable[[Request], bool], Pattern[str], str], code has Union[Callable[[Request], Union[bool, typing.Awaitable[bool]]], Pattern[str], str]
Parameter type mismatch in Page.expect_response(url_or_predicate=): documented as Union[Callable[[Response], bool], Pattern[str], str], code has Union[Callable[[Response], Union[bool, typing.Awaitable[bool]]], Pattern[str], str]
11 changes: 11 additions & 0 deletions scripts/generate_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@
from playwright._impl._video import Video
from playwright._impl._web_error import WebError

SYNC_API = False


def process_type(value: Any, param: bool = False) -> str:
value = str(value)
Expand All @@ -65,6 +67,15 @@ def process_type(value: Any, param: bool = False) -> str:
value = re.sub(r"playwright\._impl\._api_structures.([\w]+)", r"\1", value)
value = re.sub(r"playwright\._impl\.[\w]+\.([\w]+)", r'"\1"', value)
value = re.sub(r"typing.Literal", "Literal", value)
if SYNC_API:
# Sync API does not accept awaitable callbacks; collapse
# Union[X, Awaitable[X]] (used for predicates the async API also
# accepts as `async def`) down to just X.
value = re.sub(
r"typing\.Union\[([^\[\],]+),\s*typing\.Awaitable\[\1\]\]",
r"\1",
value,
)
if param:
value = re.sub(r"^typing.Union\[([^,]+), None\]$", r"\1 = None", value)
value = re.sub(
Expand Down
3 changes: 3 additions & 0 deletions scripts/generate_sync_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from types import FunctionType
from typing import Any

import generate_api
from documentation_provider import DocumentationProvider
from generate_api import (
api_globals,
Expand All @@ -33,6 +34,8 @@
signature,
)

generate_api.SYNC_API = True

documentation_provider = DocumentationProvider(False)


Expand Down
49 changes: 48 additions & 1 deletion tests/async/test_page.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import os
import re
from pathlib import Path
from typing import Dict, List, Optional
from typing import Any, Dict, List, Optional

import pytest

Expand Down Expand Up @@ -352,6 +352,53 @@ async def test_wait_for_response_should_work_with_predicate(
assert response.url == server.PREFIX + "/digits/2.png"


async def test_wait_for_response_should_work_with_async_predicate(
page: Page, server: Server
) -> None:
await page.goto(server.EMPTY_PAGE)

async def predicate(response: Any) -> bool:
await asyncio.sleep(0)
return response.url == server.PREFIX + "/digits/2.png"

async with page.expect_response(predicate) as response_info:
await page.evaluate(
"""() => {
fetch('/digits/1.png')
fetch('/digits/2.png')
fetch('/digits/3.png')
}"""
)
response = await response_info.value
assert response.url == server.PREFIX + "/digits/2.png"


async def test_expect_response_should_reject_when_async_predicate_throws(
page: Page, server: Server
) -> None:
await page.goto(server.EMPTY_PAGE)

async def predicate(response: Any) -> bool:
raise Exception("Async oops!")

with pytest.raises(Exception, match="Async oops!"):
async with page.expect_response(predicate):
await page.evaluate("() => fetch('/digits/1.png')")


async def test_expect_response_should_reject_when_sync_predicate_throws(
page: Page, server: Server
) -> None:
await page.goto(server.EMPTY_PAGE)

def predicate(response: Any) -> bool:
raise Exception("Sync oops!")

with pytest.raises(Exception, match="Sync oops!"):
async with page.expect_response(predicate):
await page.evaluate("() => fetch('/digits/1.png')")


async def test_wait_for_response_should_work_with_no_timeout(
page: Page, server: Server
) -> None:
Expand Down
Loading