From 2018e49039f0ecf73aa82b9a7780db1cd54bb75b Mon Sep 17 00:00:00 2001 From: Eli Black Date: Tue, 29 Apr 2025 15:20:56 +0800 Subject: [PATCH 1/7] Speed up connection by not asking for context when inspecting stack --- playwright/_impl/_connection.py | 7 +++++-- tests/async/test_asyncio.py | 13 +++++++++++++ tests/sync/test_sync.py | 13 +++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/playwright/_impl/_connection.py b/playwright/_impl/_connection.py index 027daf69d..3dd5df633 100644 --- a/playwright/_impl/_connection.py +++ b/playwright/_impl/_connection.py @@ -365,7 +365,6 @@ def _send_message_to_server( if ( self._tracing_count > 0 and frames - and frames and object._guid != "localUtils" ): self.local_utils.add_stack_to_tracing_no_reply(id, frames) @@ -519,7 +518,11 @@ async def wrap_api_call( if self._api_zone.get(): return await cb() task = asyncio.current_task(self._loop) - st: List[inspect.FrameInfo] = getattr(task, "__pw_stack__", inspect.stack()) + st: List[inspect.FrameInfo] = getattr(task, "__pw_stack__", None) + + if st == None: + st = inspect.stack(0) + parsed_st = _extract_stack_trace_information_from_stack(st, is_internal) self._api_zone.set(parsed_st) try: diff --git a/tests/async/test_asyncio.py b/tests/async/test_asyncio.py index 33edc71ce..e7ed72d9e 100644 --- a/tests/async/test_asyncio.py +++ b/tests/async/test_asyncio.py @@ -87,3 +87,16 @@ async def raise_exception() -> None: assert "Something went wrong" in str(exc_info.value.exceptions[0]) assert isinstance(exc_info.value.exceptions[0], ValueError) assert await page.evaluate("() => 11 * 11") == 121 + +async def test_should_return_proper_api_name_on_error(page: Page) -> None: + try: + await page.evaluate("does_not_exist") + + assert False, "Accessing undefined JavaScript variable should have thrown exception" + except Exception as error: + print(':-:' + error.message + ':-:') + assert error.message == """Page.evaluate: ReferenceError: does_not_exist is not defined + at eval (eval at evaluate (:234:30), :1:1) + at eval () + at UtilityScript.evaluate (:234:30) + at UtilityScript. (:1:44)""" \ No newline at end of file diff --git a/tests/sync/test_sync.py b/tests/sync/test_sync.py index 64eace1e9..c673efad0 100644 --- a/tests/sync/test_sync.py +++ b/tests/sync/test_sync.py @@ -346,3 +346,16 @@ def test_call_sync_method_after_playwright_close_with_own_loop( p.start() p.join() assert p.exitcode == 0 + +def test_should_return_proper_api_name_on_error(page: Page) -> None: + try: + page.evaluate("does_not_exist") + + assert False, "Accessing undefined JavaScript variable should have thrown exception" + except Exception as error: + print(':-:' + error.message + ':-:') + assert error.message == """Page.evaluate: ReferenceError: does_not_exist is not defined + at eval (eval at evaluate (:234:30), :1:1) + at eval () + at UtilityScript.evaluate (:234:30) + at UtilityScript. (:1:44)""" \ No newline at end of file From 5a92f4f5713c985daa0f9cd3ea46a9431208bee4 Mon Sep 17 00:00:00 2001 From: Eli Black Date: Tue, 29 Apr 2025 15:30:56 +0800 Subject: [PATCH 2/7] Formatting changes from the pre-commit hook --- playwright/_impl/_connection.py | 13 ++++--------- tests/async/test_asyncio.py | 14 ++++++++++---- tests/sync/test_sync.py | 14 ++++++++++---- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/playwright/_impl/_connection.py b/playwright/_impl/_connection.py index 3dd5df633..647414295 100644 --- a/playwright/_impl/_connection.py +++ b/playwright/_impl/_connection.py @@ -362,11 +362,7 @@ def _send_message_to_server( "params": self._replace_channels_with_guids(params), "metadata": metadata, } - if ( - self._tracing_count > 0 - and frames - and object._guid != "localUtils" - ): + if self._tracing_count > 0 and frames and object._guid != "localUtils": self.local_utils.add_stack_to_tracing_no_reply(id, frames) self._transport.send(message) @@ -518,10 +514,9 @@ async def wrap_api_call( if self._api_zone.get(): return await cb() task = asyncio.current_task(self._loop) - st: List[inspect.FrameInfo] = getattr(task, "__pw_stack__", None) - - if st == None: - st = inspect.stack(0) + st: List[inspect.FrameInfo] = getattr( + task, "__pw_stack__", None + ) or inspect.stack(0) parsed_st = _extract_stack_trace_information_from_stack(st, is_internal) self._api_zone.set(parsed_st) diff --git a/tests/async/test_asyncio.py b/tests/async/test_asyncio.py index e7ed72d9e..1e4857ad5 100644 --- a/tests/async/test_asyncio.py +++ b/tests/async/test_asyncio.py @@ -88,15 +88,21 @@ async def raise_exception() -> None: assert isinstance(exc_info.value.exceptions[0], ValueError) assert await page.evaluate("() => 11 * 11") == 121 + async def test_should_return_proper_api_name_on_error(page: Page) -> None: try: await page.evaluate("does_not_exist") - assert False, "Accessing undefined JavaScript variable should have thrown exception" + assert ( + False + ), "Accessing undefined JavaScript variable should have thrown exception" except Exception as error: - print(':-:' + error.message + ':-:') - assert error.message == """Page.evaluate: ReferenceError: does_not_exist is not defined + print(":-:" + error.message + ":-:") + assert ( + error.message + == """Page.evaluate: ReferenceError: does_not_exist is not defined at eval (eval at evaluate (:234:30), :1:1) at eval () at UtilityScript.evaluate (:234:30) - at UtilityScript. (:1:44)""" \ No newline at end of file + at UtilityScript. (:1:44)""" + ) diff --git a/tests/sync/test_sync.py b/tests/sync/test_sync.py index c673efad0..d0f630f67 100644 --- a/tests/sync/test_sync.py +++ b/tests/sync/test_sync.py @@ -347,15 +347,21 @@ def test_call_sync_method_after_playwright_close_with_own_loop( p.join() assert p.exitcode == 0 + def test_should_return_proper_api_name_on_error(page: Page) -> None: try: page.evaluate("does_not_exist") - assert False, "Accessing undefined JavaScript variable should have thrown exception" + assert ( + False + ), "Accessing undefined JavaScript variable should have thrown exception" except Exception as error: - print(':-:' + error.message + ':-:') - assert error.message == """Page.evaluate: ReferenceError: does_not_exist is not defined + print(":-:" + error.message + ":-:") + assert ( + error.message + == """Page.evaluate: ReferenceError: does_not_exist is not defined at eval (eval at evaluate (:234:30), :1:1) at eval () at UtilityScript.evaluate (:234:30) - at UtilityScript. (:1:44)""" \ No newline at end of file + at UtilityScript. (:1:44)""" + ) From e973932c0099f5e65d462d6619edfc17d1e87cd5 Mon Sep 17 00:00:00 2001 From: Eli Black Date: Tue, 29 Apr 2025 16:06:56 +0800 Subject: [PATCH 3/7] Also use inspect.stack(0) for the sync version of _connect --- playwright/_impl/_connection.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/playwright/_impl/_connection.py b/playwright/_impl/_connection.py index 647414295..2d1dad933 100644 --- a/playwright/_impl/_connection.py +++ b/playwright/_impl/_connection.py @@ -533,7 +533,9 @@ def wrap_api_call_sync( if self._api_zone.get(): return cb() task = asyncio.current_task(self._loop) - st: List[inspect.FrameInfo] = getattr(task, "__pw_stack__", inspect.stack()) + st: List[inspect.FrameInfo] = getattr( + task, "__pw_stack__", None + ) or inspect.stack(0) parsed_st = _extract_stack_trace_information_from_stack(st, is_internal) self._api_zone.set(parsed_st) try: From ebddce42e287f5142948cef37e02fcb093140a57 Mon Sep 17 00:00:00 2001 From: Eli Black Date: Tue, 29 Apr 2025 17:24:16 +0800 Subject: [PATCH 4/7] Fix pre-commit warnings --- tests/async/test_asyncio.py | 3 +-- tests/sync/test_sync.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/async/test_asyncio.py b/tests/async/test_asyncio.py index 1e4857ad5..5d878157d 100644 --- a/tests/async/test_asyncio.py +++ b/tests/async/test_asyncio.py @@ -97,9 +97,8 @@ async def test_should_return_proper_api_name_on_error(page: Page) -> None: False ), "Accessing undefined JavaScript variable should have thrown exception" except Exception as error: - print(":-:" + error.message + ":-:") assert ( - error.message + str(error) == """Page.evaluate: ReferenceError: does_not_exist is not defined at eval (eval at evaluate (:234:30), :1:1) at eval () diff --git a/tests/sync/test_sync.py b/tests/sync/test_sync.py index d0f630f67..de9514634 100644 --- a/tests/sync/test_sync.py +++ b/tests/sync/test_sync.py @@ -356,9 +356,8 @@ def test_should_return_proper_api_name_on_error(page: Page) -> None: False ), "Accessing undefined JavaScript variable should have thrown exception" except Exception as error: - print(":-:" + error.message + ":-:") assert ( - error.message + str(error) == """Page.evaluate: ReferenceError: does_not_exist is not defined at eval (eval at evaluate (:234:30), :1:1) at eval () From 5ac20ef84f3f6577ce19de5ed734da8dc701a84f Mon Sep 17 00:00:00 2001 From: Eli Black Date: Tue, 29 Apr 2025 23:30:30 +0800 Subject: [PATCH 5/7] Simplify test cases, so that they pass on all browsers --- tests/async/test_asyncio.py | 10 ++-------- tests/sync/test_sync.py | 10 ++-------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/tests/async/test_asyncio.py b/tests/async/test_asyncio.py index 5d878157d..971c65473 100644 --- a/tests/async/test_asyncio.py +++ b/tests/async/test_asyncio.py @@ -97,11 +97,5 @@ async def test_should_return_proper_api_name_on_error(page: Page) -> None: False ), "Accessing undefined JavaScript variable should have thrown exception" except Exception as error: - assert ( - str(error) - == """Page.evaluate: ReferenceError: does_not_exist is not defined - at eval (eval at evaluate (:234:30), :1:1) - at eval () - at UtilityScript.evaluate (:234:30) - at UtilityScript. (:1:44)""" - ) + # Each browser returns slightly different error messages, but they should all start with "Page.evaluate:", because that was the Playwright method where the error originated + assert str(error).startswith("Page.evaluate:") diff --git a/tests/sync/test_sync.py b/tests/sync/test_sync.py index de9514634..92d40c19a 100644 --- a/tests/sync/test_sync.py +++ b/tests/sync/test_sync.py @@ -356,11 +356,5 @@ def test_should_return_proper_api_name_on_error(page: Page) -> None: False ), "Accessing undefined JavaScript variable should have thrown exception" except Exception as error: - assert ( - str(error) - == """Page.evaluate: ReferenceError: does_not_exist is not defined - at eval (eval at evaluate (:234:30), :1:1) - at eval () - at UtilityScript.evaluate (:234:30) - at UtilityScript. (:1:44)""" - ) + # Each browser returns slightly different error messages, but they should all start with "Page.evaluate:", because that was the Playwright method where the error originated + assert str(error).startswith("Page.evaluate:") From 4c41c7fd6940c2c2c5382025a48ba1a8d48c51ff Mon Sep 17 00:00:00 2001 From: Eli Black Date: Sat, 3 May 2025 18:49:32 +0800 Subject: [PATCH 6/7] On Windows, fixed error '_browser_type.py:174: error: Incompatible return value type (got "Path", expected "str") [return-value]' --- playwright/_impl/_browser_type.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/playwright/_impl/_browser_type.py b/playwright/_impl/_browser_type.py index b34d224d6..bedc5ea73 100644 --- a/playwright/_impl/_browser_type.py +++ b/playwright/_impl/_browser_type.py @@ -171,7 +171,7 @@ def _user_data_dir(self, userDataDir: Optional[Union[str, Path]]) -> str: # Can be dropped once we drop Python 3.9 support (10/2025): # https://github.com/python/cpython/issues/82852 if sys.platform == "win32" and sys.version_info[:2] < (3, 10): - return pathlib.Path.cwd() / userDataDir + return str(pathlib.Path.cwd() / userDataDir) return str(Path(userDataDir).resolve()) return str(Path(userDataDir)) From 0e4d0a4d124f356b626bc6a5a6364cff7c4d8e7d Mon Sep 17 00:00:00 2001 From: Eli Black Date: Sat, 3 May 2025 18:50:12 +0800 Subject: [PATCH 7/7] On Windows, fixed type warnings about os.setpgrp(), os.getpgid(), and os.killpg() --- tests/common/test_signals.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/common/test_signals.py b/tests/common/test_signals.py index 472e74042..174eaf6f2 100644 --- a/tests/common/test_signals.py +++ b/tests/common/test_signals.py @@ -27,6 +27,10 @@ def _test_signals_async( browser_name: str, launch_arguments: Dict, wait_queue: "multiprocessing.Queue[str]" ) -> None: + # On Windows, hint to mypy and pyright that they shouldn't check this function + if sys.platform == "win32": + return + os.setpgrp() sigint_received = False @@ -67,6 +71,10 @@ async def main() -> None: def _test_signals_sync( browser_name: str, launch_arguments: Dict, wait_queue: "multiprocessing.Queue[str]" ) -> None: + # On Windows, hint to mypy and pyright that they shouldn't check this function + if sys.platform == "win32": + return + os.setpgrp() sigint_received = False @@ -103,6 +111,10 @@ def my_sig_handler(signum: int, frame: Any) -> None: def _create_signals_test( target: Any, browser_name: str, launch_arguments: Dict ) -> None: + # On Windows, hint to mypy and pyright that they shouldn't check this function + if sys.platform == "win32": + return + wait_queue: "multiprocessing.Queue[str]" = multiprocessing.Queue() process = multiprocessing.Process( target=target, args=[browser_name, launch_arguments, wait_queue]