From 87a2f81095936ca954ef4d2634d7c8bb610aa7d7 Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 00:02:04 -0300 Subject: [PATCH 01/41] refactor(tests): replace for/append loop with list comprehension in test_locators_should_query_existing_elements --- tests/sync/test_locators.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/sync/test_locators.py b/tests/sync/test_locators.py index 1c191f826..ca7ddbf71 100644 --- a/tests/sync/test_locators.py +++ b/tests/sync/test_locators.py @@ -225,9 +225,7 @@ def test_locators_should_query_existing_elements(page: Page) -> None: html = page.locator("html") elements = html.locator("div").element_handles() assert len(elements) == 2 - result = [] - for element in elements: - result.append(page.evaluate("e => e.textContent", element)) + result = [page.evaluate("e => e.textContent", element) for element in elements] assert result == ["A", "B"] From a51ece9a67e21ca18ac887cb06f40543bca4ddb3 Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 00:06:18 -0300 Subject: [PATCH 02/41] refactor(tests): replace for/append loop with list comprehension in test_locator_all_should_work --- tests/sync/test_locators.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/sync/test_locators.py b/tests/sync/test_locators.py index ca7ddbf71..170a075d8 100644 --- a/tests/sync/test_locators.py +++ b/tests/sync/test_locators.py @@ -981,9 +981,7 @@ def blur_event() -> None: def test_locator_all_should_work(page: Page) -> None: page.set_content("

A

B

C

") - texts = [] - for p in page.locator("p").all(): - texts.append(p.text_content()) + texts = [p.text_content() for p in page.locator("p").all()] assert texts == ["A", "B", "C"] From f6f751c2e7fa4754f403253ec0a4a0f2a7635b30 Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 00:08:33 -0300 Subject: [PATCH 03/41] refactor(tests): use contextlib.suppress in test_should_throw_when_handler_times_out (Exception Handling + Non-Functional Statement) --- tests/sync/test_page_add_locator_handler.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/sync/test_page_add_locator_handler.py b/tests/sync/test_page_add_locator_handler.py index b16aca9f4..5827e17c2 100644 --- a/tests/sync/test_page_add_locator_handler.py +++ b/tests/sync/test_page_add_locator_handler.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import contextlib import sys from typing import TYPE_CHECKING @@ -207,10 +208,8 @@ def handler() -> None: nonlocal called called += 1 # Deliberately timeout. - try: + with contextlib.suppress(Exception): page.wait_for_timeout(9999999) - except Exception: - pass page.add_locator_handler( page.get_by_text("This interstitial covers the button"), handler From 95c79a68116751882b1fdf1ecb57f6d967aca539 Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 00:10:18 -0300 Subject: [PATCH 04/41] refactor(tests): move try/finally to fixture in test_should_be_able_to_set_custom_global_timeout --- tests/sync/test_assertions.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/tests/sync/test_assertions.py b/tests/sync/test_assertions.py index 90cf5a3be..e218388a4 100644 --- a/tests/sync/test_assertions.py +++ b/tests/sync/test_assertions.py @@ -14,6 +14,7 @@ import datetime import re +from typing import Generator import pytest @@ -972,16 +973,21 @@ def test_should_be_able_to_set_custom_timeout(page: Page) -> None: ) -def test_should_be_able_to_set_custom_global_timeout(page: Page) -> None: - try: - expect.set_options(timeout=111) - with pytest.raises(AssertionError) as exc_info: - expect(page.locator("#a1")).to_be_visible() - assert 'Expect "to_be_visible" locator("#a1") with timeout 111ms' in str( - exc_info.value - ) - finally: - expect.set_options(timeout=5_000) +@pytest.fixture +def custom_global_timeout() -> Generator[None, None, None]: + expect.set_options(timeout=111) + yield + expect.set_options(timeout=5_000) + + +def test_should_be_able_to_set_custom_global_timeout( + page: Page, custom_global_timeout: None +) -> None: + with pytest.raises(AssertionError) as exc_info: + expect(page.locator("#a1")).to_be_visible() + assert 'Expect "to_be_visible" locator("#a1") with timeout 111ms' in str( + exc_info.value + ) def test_to_have_accessible_name(page: Page) -> None: From 08d011f2fcb2c44dc8cfce8b7245b45441b45737 Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 00:12:31 -0300 Subject: [PATCH 05/41] refactor(tests): replace browser if/elif with dict lookup in test_should_not_chain_abort --- tests/sync/test_browsercontext_request_fallback.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/sync/test_browsercontext_request_fallback.py b/tests/sync/test_browsercontext_request_fallback.py index 6feb19942..eee3e8172 100644 --- a/tests/sync/test_browsercontext_request_fallback.py +++ b/tests/sync/test_browsercontext_request_fallback.py @@ -117,8 +117,7 @@ def test_should_not_chain_abort( page: Page, context: BrowserContext, server: Server, - is_webkit: bool, - is_firefox: bool, + browser_name: str, ) -> None: failed = [False] @@ -131,12 +130,11 @@ def handler(route: Route) -> None: with pytest.raises(Error) as excinfo: page.goto(server.EMPTY_PAGE) - if is_webkit: - assert "Blocked by Web Inspector" in excinfo.value.message - elif is_firefox: - assert "NS_ERROR_FAILURE" in excinfo.value.message - else: - assert "net::ERR_FAILED" in excinfo.value.message + expected_message = { + "webkit": "Blocked by Web Inspector", + "firefox": "NS_ERROR_FAILURE", + }.get(browser_name, "net::ERR_FAILED") + assert expected_message in excinfo.value.message assert not failed[0] From 2eaa785e0ea5ef4c895ccbd24f917435305ba72d Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 00:15:40 -0300 Subject: [PATCH 06/41] refactor(tests): replace browser if/elif with dict lookup in test_by_default_should_abort_requests_not_found_in_har --- tests/sync/test_har.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/sync/test_har.py b/tests/sync/test_har.py index a401038b6..f652a60f7 100644 --- a/tests/sync/test_har.py +++ b/tests/sync/test_har.py @@ -258,8 +258,7 @@ def test_by_default_should_abort_requests_not_found_in_har( context: BrowserContext, server: Server, assetdir: Path, - is_chromium: bool, - is_webkit: bool, + browser_name: str, ) -> None: context.route_from_har(har=assetdir / "har-fulfill.har") page = context.new_page() @@ -267,12 +266,11 @@ def test_by_default_should_abort_requests_not_found_in_har( with pytest.raises(Error) as exc_info: page.goto(server.EMPTY_PAGE) assert exc_info.value - if is_chromium: - assert "net::ERR_FAILED" in exc_info.value.message - elif is_webkit: - assert "Blocked by Web Inspector" in exc_info.value.message - else: - assert "NS_ERROR_FAILURE" in exc_info.value.message + expected_message = { + "chromium": "net::ERR_FAILED", + "webkit": "Blocked by Web Inspector", + }.get(browser_name, "NS_ERROR_FAILURE") + assert expected_message in exc_info.value.message def test_fallback_continue_should_continue_requests_on_bad_har( From 41423671446a4e3804f837a8eb89a8fd53618796 Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 00:17:39 -0300 Subject: [PATCH 07/41] refactor(tests): parametrize method in test_should_throw_an_error_when_max_redirects_is_exceeded --- tests/sync/test_fetch_global.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/sync/test_fetch_global.py b/tests/sync/test_fetch_global.py index 15a11fca8..b3db353b4 100644 --- a/tests/sync/test_fetch_global.py +++ b/tests/sync/test_fetch_global.py @@ -275,8 +275,9 @@ def test_storage_state_should_round_trip_through_file( assert state2 == expected +@pytest.mark.parametrize("method", ["GET", "PUT", "POST", "OPTIONS", "HEAD", "PATCH"]) def test_should_throw_an_error_when_max_redirects_is_exceeded( - playwright: Playwright, server: Server + playwright: Playwright, server: Server, method: str ) -> None: server.set_redirect("/a/redirect1", "/b/c/redirect2") server.set_redirect("/b/c/redirect2", "/b/c/redirect3") @@ -284,15 +285,14 @@ def test_should_throw_an_error_when_max_redirects_is_exceeded( server.set_redirect("/b/c/redirect4", "/simple.json") request = playwright.request.new_context() - for method in ["GET", "PUT", "POST", "OPTIONS", "HEAD", "PATCH"]: - for max_redirects in [1, 2, 3]: - with pytest.raises(Error) as exc_info: - request.fetch( - server.PREFIX + "/a/redirect1", - method=method, - max_redirects=max_redirects, - ) - assert "Max redirect count exceeded" in str(exc_info) + for max_redirects in [1, 2, 3]: + with pytest.raises(Error) as exc_info: + request.fetch( + server.PREFIX + "/a/redirect1", + method=method, + max_redirects=max_redirects, + ) + assert "Max redirect count exceeded" in str(exc_info) def test_should_not_follow_redirects_when_max_redirects_is_set_to_0( From 0002ae451bea78462c6408cde67568b13d5e817f Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 00:18:48 -0300 Subject: [PATCH 08/41] refactor(tests): parametrize max_redirects in test_should_throw_an_error_when_max_redirects_is_exceeded --- tests/sync/test_fetch_global.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/sync/test_fetch_global.py b/tests/sync/test_fetch_global.py index b3db353b4..d71bc45a9 100644 --- a/tests/sync/test_fetch_global.py +++ b/tests/sync/test_fetch_global.py @@ -276,8 +276,9 @@ def test_storage_state_should_round_trip_through_file( @pytest.mark.parametrize("method", ["GET", "PUT", "POST", "OPTIONS", "HEAD", "PATCH"]) +@pytest.mark.parametrize("max_redirects", [1, 2, 3]) def test_should_throw_an_error_when_max_redirects_is_exceeded( - playwright: Playwright, server: Server, method: str + playwright: Playwright, server: Server, method: str, max_redirects: int ) -> None: server.set_redirect("/a/redirect1", "/b/c/redirect2") server.set_redirect("/b/c/redirect2", "/b/c/redirect3") @@ -285,14 +286,13 @@ def test_should_throw_an_error_when_max_redirects_is_exceeded( server.set_redirect("/b/c/redirect4", "/simple.json") request = playwright.request.new_context() - for max_redirects in [1, 2, 3]: - with pytest.raises(Error) as exc_info: - request.fetch( - server.PREFIX + "/a/redirect1", - method=method, - max_redirects=max_redirects, - ) - assert "Max redirect count exceeded" in str(exc_info) + with pytest.raises(Error) as exc_info: + request.fetch( + server.PREFIX + "/a/redirect1", + method=method, + max_redirects=max_redirects, + ) + assert "Max redirect count exceeded" in str(exc_info) def test_should_not_follow_redirects_when_max_redirects_is_set_to_0( From 4ff9d27a8121751bb4e104a57e36fe18d38385ec Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 00:19:27 -0300 Subject: [PATCH 09/41] refactor(tests): parametrize method in test_should_not_follow_redirects_when_max_redirects_is_set_to_0 --- tests/sync/test_fetch_global.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/sync/test_fetch_global.py b/tests/sync/test_fetch_global.py index d71bc45a9..6ed58bf13 100644 --- a/tests/sync/test_fetch_global.py +++ b/tests/sync/test_fetch_global.py @@ -295,19 +295,19 @@ def test_should_throw_an_error_when_max_redirects_is_exceeded( assert "Max redirect count exceeded" in str(exc_info) +@pytest.mark.parametrize("method", ["GET", "PUT", "POST", "OPTIONS", "HEAD", "PATCH"]) def test_should_not_follow_redirects_when_max_redirects_is_set_to_0( - playwright: Playwright, server: Server + playwright: Playwright, server: Server, method: str ) -> None: server.set_redirect("/a/redirect1", "/b/c/redirect2") server.set_redirect("/b/c/redirect2", "/simple.json") request = playwright.request.new_context() - for method in ["GET", "PUT", "POST", "OPTIONS", "HEAD", "PATCH"]: - response = request.fetch( - server.PREFIX + "/a/redirect1", method=method, max_redirects=0 - ) - assert response.headers["location"] == "/b/c/redirect2" - assert response.status == 302 + response = request.fetch( + server.PREFIX + "/a/redirect1", method=method, max_redirects=0 + ) + assert response.headers["location"] == "/b/c/redirect2" + assert response.status == 302 def test_should_throw_an_error_when_max_redirects_is_less_than_0( From fd3e1ff26b163afc62342ad173b99be73c30ec41 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 02:21:20 -0300 Subject: [PATCH 10/41] test(screencast): use context manager in show_actions cursor test Replace the try/finally around screencast.start() with a with block, so stop() runs on exit without manual exception handling. Smell: Exception Handling --- tests/sync/test_screencast.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/sync/test_screencast.py b/tests/sync/test_screencast.py index 06ae46704..87a1b6ddc 100644 --- a/tests/sync/test_screencast.py +++ b/tests/sync/test_screencast.py @@ -147,11 +147,8 @@ def test_on_frame_receives_viewport_size(browser: Browser, server: Server) -> No def test_show_actions_should_accept_cursor_param(page: Page) -> None: - page.screencast.start(on_frame=lambda f: None) - try: + with page.screencast.start(on_frame=lambda f: None): with page.screencast.show_actions(duration=100, cursor="pointer"): pass with page.screencast.show_actions(duration=100, cursor="none"): pass - finally: - page.screencast.stop() From 95deaf6524d25dccd348629e81bd016cdd3363f4 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 02:22:34 -0300 Subject: [PATCH 11/41] test(launcher): drop browser if/elif chain in browser type name test Compare browser_type.name directly with the browser_name fixture instead of branching on is_webkit/is_firefox/is_chromium, so the test has a single assertion path for every browser. Smell: Conditional Test Logic --- tests/sync/test_launcher.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/tests/sync/test_launcher.py b/tests/sync/test_launcher.py index 6b89b79a8..b719897fd 100644 --- a/tests/sync/test_launcher.py +++ b/tests/sync/test_launcher.py @@ -59,16 +59,9 @@ def test_browser_type_executable_path_should_work( def test_browser_type_name_should_work( - browser_type: BrowserType, is_webkit: bool, is_firefox: bool, is_chromium: bool + browser_type: BrowserType, browser_name: str ) -> None: - if is_webkit: - assert browser_type.name == "webkit" - elif is_firefox: - assert browser_type.name == "firefox" - elif is_chromium: - assert browser_type.name == "chromium" - else: - raise ValueError("Unknown browser") + assert browser_type.name == browser_name def test_browser_close_should_fire_close_event_for_all_contexts( From b68890f1efecfc058922da2bc40ecafbe3ce8713 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 02:23:20 -0300 Subject: [PATCH 12/41] test(screencast): reuse ensure_some_frames in viewport size test The inline loop duplicated the ensure_some_frames helper already defined in this file (100 paint cycles followed by a screenshot), so call it instead of repeating the loop. Smell: Conditional Test Logic --- tests/sync/test_screencast.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/sync/test_screencast.py b/tests/sync/test_screencast.py index 87a1b6ddc..684b6cfb6 100644 --- a/tests/sync/test_screencast.py +++ b/tests/sync/test_screencast.py @@ -133,11 +133,7 @@ def test_on_frame_receives_viewport_size(browser: Browser, server: Server) -> No page.screencast.start(on_frame=lambda f: received.append(f), size=size) page.goto(server.EMPTY_PAGE) page.evaluate("() => document.body.style.backgroundColor = 'red'") - for _ in range(100): - page.evaluate( - "() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f)))" - ) - page.screenshot() + ensure_some_frames(page) page.screencast.stop() assert len(received) >= 1 assert any(frame["viewportWidth"] == 1000 for frame in received) From cc3974be33b790ec748e583b79d3f91a91eefd31 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 02:24:05 -0300 Subject: [PATCH 13/41] test(screencast): replace for loop with all() in viewport size test Assert the viewport height and timestamp type of every received frame with all() over the frames instead of looping with assertions inside. Smell: Conditional Test Logic --- tests/sync/test_screencast.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/sync/test_screencast.py b/tests/sync/test_screencast.py index 684b6cfb6..7935df9c5 100644 --- a/tests/sync/test_screencast.py +++ b/tests/sync/test_screencast.py @@ -137,9 +137,8 @@ def test_on_frame_receives_viewport_size(browser: Browser, server: Server) -> No page.screencast.stop() assert len(received) >= 1 assert any(frame["viewportWidth"] == 1000 for frame in received) - for frame in received: - assert frame["viewportHeight"] == 400 - assert isinstance(frame["timestamp"], (int, float)) + assert all(frame["viewportHeight"] == 400 for frame in received) + assert all(isinstance(frame["timestamp"], (int, float)) for frame in received) def test_show_actions_should_accept_cursor_param(page: Page) -> None: From a85f2cc273b6f7d548a2a6019e830b4a6fb68bf2 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 03:21:50 -0300 Subject: [PATCH 14/41] test(screencast): click a button inside the first show_actions block The pointer-cursor show_actions block had an empty pass body. Render a button and click it inside the block so the test exercises show_actions instead of entering and leaving it without doing anything. Smell: Non-Functional Statement --- tests/sync/test_screencast.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/sync/test_screencast.py b/tests/sync/test_screencast.py index 7935df9c5..d6e7561f3 100644 --- a/tests/sync/test_screencast.py +++ b/tests/sync/test_screencast.py @@ -142,8 +142,9 @@ def test_on_frame_receives_viewport_size(browser: Browser, server: Server) -> No def test_show_actions_should_accept_cursor_param(page: Page) -> None: + page.set_content("") with page.screencast.start(on_frame=lambda f: None): with page.screencast.show_actions(duration=100, cursor="pointer"): - pass + page.locator("button").click() with page.screencast.show_actions(duration=100, cursor="none"): pass From 6cca894b7fe00f13e1aa4ed98098bbd2ee660eb6 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 03:22:45 -0300 Subject: [PATCH 15/41] test(screencast): click a button inside the second show_actions block The cursor="none" show_actions block also had an empty pass body. Click the same button inside it so both cursor values are exercised by a real action. Smell: Non-Functional Statement --- tests/sync/test_screencast.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sync/test_screencast.py b/tests/sync/test_screencast.py index d6e7561f3..781d6ca81 100644 --- a/tests/sync/test_screencast.py +++ b/tests/sync/test_screencast.py @@ -147,4 +147,4 @@ def test_show_actions_should_accept_cursor_param(page: Page) -> None: with page.screencast.show_actions(duration=100, cursor="pointer"): page.locator("button").click() with page.screencast.show_actions(duration=100, cursor="none"): - pass + page.locator("button").click() From 3fc11d198a8aa855a77c903202ef6e262c2a58d8 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 03:23:39 -0300 Subject: [PATCH 16/41] test(sync): use pytest.raises for the API name error test Replace the try/except plus assert False with pytest.raises(Error), so the test fails loudly if evaluate() does not raise, and check the error message afterwards. Smell: Exception Handling --- tests/sync/test_sync.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/tests/sync/test_sync.py b/tests/sync/test_sync.py index 7e3d36977..8e11a0e17 100644 --- a/tests/sync/test_sync.py +++ b/tests/sync/test_sync.py @@ -354,15 +354,10 @@ def test_call_sync_method_after_playwright_close_with_own_loop( def test_should_return_proper_api_name_on_error(page: Page) -> None: - try: + with pytest.raises(Error) as exc_info: page.evaluate("does_not_exist") - - assert ( - False - ), "Accessing undefined JavaScript variable should have thrown exception" - except Exception as error: - # 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:") + # 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(exc_info.value).startswith("Page.evaluate:") def test_should_not_orphan_callback_on_non_serializable_params( From 41e364409ccae4da4746346a0eb181c649e606ee Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 03:24:21 -0300 Subject: [PATCH 17/41] test(screencast): use context manager in starting twice test Replace the try/finally around the first screencast.start() with a with block, so stop() runs on exit while pytest.raises keeps checking the "already started" error. Smell: Exception Handling --- tests/sync/test_screencast.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/sync/test_screencast.py b/tests/sync/test_screencast.py index 781d6ca81..ece9bf70a 100644 --- a/tests/sync/test_screencast.py +++ b/tests/sync/test_screencast.py @@ -116,12 +116,9 @@ def on_frame(_: object) -> None: def test_starting_twice_should_throw(page: Page) -> None: - page.screencast.start(on_frame=lambda f: None) - try: + with page.screencast.start(on_frame=lambda f: None): with pytest.raises(Exception, match="already started"): page.screencast.start(on_frame=lambda f: None) - finally: - page.screencast.stop() def test_on_frame_receives_viewport_size(browser: Browser, server: Server) -> None: From f324d7d3d6da3a6b114db2973b00215a8070d807 Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 10:03:05 -0300 Subject: [PATCH 18/41] refactor(tests): parametrize method in test_should_throw_an_error_when_max_redirects_is_less_than_0 --- tests/sync/test_fetch_global.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/sync/test_fetch_global.py b/tests/sync/test_fetch_global.py index 6ed58bf13..ea50bdc53 100644 --- a/tests/sync/test_fetch_global.py +++ b/tests/sync/test_fetch_global.py @@ -310,17 +310,16 @@ def test_should_not_follow_redirects_when_max_redirects_is_set_to_0( assert response.status == 302 +@pytest.mark.parametrize("method", ["GET", "PUT", "POST", "OPTIONS", "HEAD", "PATCH"]) def test_should_throw_an_error_when_max_redirects_is_less_than_0( playwright: Playwright, server: Server, + method: str, ) -> None: request = playwright.request.new_context() - for method in ["GET", "PUT", "POST", "OPTIONS", "HEAD", "PATCH"]: - with pytest.raises(AssertionError) as exc_info: - request.fetch( - server.PREFIX + "/a/redirect1", method=method, max_redirects=-1 - ) - assert "'max_redirects' must be greater than or equal to '0'" in str(exc_info) + with pytest.raises(AssertionError) as exc_info: + request.fetch(server.PREFIX + "/a/redirect1", method=method, max_redirects=-1) + assert "'max_redirects' must be greater than or equal to '0'" in str(exc_info) def test_should_serialize_null_values_in_json( From 04cf3fbdcb6c7a0f8bfad6d7f49eb5cc97dfa99a Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 10:04:10 -0300 Subject: [PATCH 19/41] refactor(tests): parametrize button index in test_locators_should_scroll_into_view --- tests/sync/test_locators.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tests/sync/test_locators.py b/tests/sync/test_locators.py index 170a075d8..f085bb565 100644 --- a/tests/sync/test_locators.py +++ b/tests/sync/test_locators.py @@ -337,20 +337,19 @@ def test_locators_should_press(page: Page) -> None: assert page.eval_on_selector("input", "input => input.value") == "h" -def test_locators_should_scroll_into_view(page: Page, server: Server) -> None: +@pytest.mark.parametrize("i", range(11)) +def test_locators_should_scroll_into_view(page: Page, server: Server, i: int) -> None: page.goto(server.PREFIX + "/offscreenbuttons.html") - for i in range(11): - button = page.locator(f"#btn{i}") - before = button.evaluate( - "button => button.getBoundingClientRect().right - window.innerWidth" - ) - assert before == 10 * i - button.scroll_into_view_if_needed() - after = button.evaluate( - "button => button.getBoundingClientRect().right - window.innerWidth" - ) - assert after <= 0 - page.evaluate("window.scrollTo(0, 0)") + button = page.locator(f"#btn{i}") + before = button.evaluate( + "button => button.getBoundingClientRect().right - window.innerWidth" + ) + assert before == 10 * i + button.scroll_into_view_if_needed() + after = button.evaluate( + "button => button.getBoundingClientRect().right - window.innerWidth" + ) + assert after <= 0 def test_locators_should_select_textarea( From c27038a202f6bb77296fcccc19db6a52cdfbc04a Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 10:06:01 -0300 Subject: [PATCH 20/41] refactor(tests): parametrize scenarios in test_should_work (add_locator_handler) --- tests/sync/test_page_add_locator_handler.py | 53 +++++++++++---------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/tests/sync/test_page_add_locator_handler.py b/tests/sync/test_page_add_locator_handler.py index 5827e17c2..a2816b1ff 100644 --- a/tests/sync/test_page_add_locator_handler.py +++ b/tests/sync/test_page_add_locator_handler.py @@ -27,7 +27,21 @@ from playwright.sync_api import Locator as TypeCheckingOnlyLocator -def test_should_work(page: Page, server: Server) -> None: +@pytest.mark.parametrize( + "args", + [ + ["mouseover", 1], + ["mouseover", 1, "capture"], + ["mouseover", 2], + ["mouseover", 2, "capture"], + ["pointerover", 1], + ["pointerover", 1, "capture"], + ["none", 1], + ["remove", 1], + ["hide", 1], + ], +) +def test_should_work(page: Page, server: Server, args: list) -> None: page.goto(server.PREFIX + "/input/handle-locator.html") before_count = 0 @@ -45,31 +59,18 @@ def handler(locator: Locator) -> None: page.add_locator_handler(original_locator, handler) - for args in [ - ["mouseover", 1], - ["mouseover", 1, "capture"], - ["mouseover", 2], - ["mouseover", 2, "capture"], - ["pointerover", 1], - ["pointerover", 1, "capture"], - ["none", 1], - ["remove", 1], - ["hide", 1], - ]: - page.locator("#aside").hover() - before_count = 0 - after_count = 0 - page.evaluate( - "(args) => { window.clicked = 0; window.setupAnnoyingInterstitial(...args); }", - args, - ) - assert before_count == 0 - assert after_count == 0 - page.locator("#target").click() - assert before_count == args[1] - assert after_count == args[1] - assert page.evaluate("window.clicked") == 1 - expect(page.locator("#interstitial")).not_to_be_visible() + page.locator("#aside").hover() + page.evaluate( + "(args) => { window.clicked = 0; window.setupAnnoyingInterstitial(...args); }", + args, + ) + assert before_count == 0 + assert after_count == 0 + page.locator("#target").click() + assert before_count == args[1] + assert after_count == args[1] + assert page.evaluate("window.clicked") == 1 + expect(page.locator("#interstitial")).not_to_be_visible() def test_should_work_with_a_custom_check(page: Page, server: Server) -> None: From f171b05cdaac177e25b66ac5ce75eb2a499988cb Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 10:07:37 -0300 Subject: [PATCH 21/41] refactor(tests): parametrize scenarios in test_should_work_with_a_custom_check --- tests/sync/test_page_add_locator_handler.py | 35 ++++++++++++--------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/tests/sync/test_page_add_locator_handler.py b/tests/sync/test_page_add_locator_handler.py index a2816b1ff..05307efc8 100644 --- a/tests/sync/test_page_add_locator_handler.py +++ b/tests/sync/test_page_add_locator_handler.py @@ -73,7 +73,18 @@ def handler(locator: Locator) -> None: expect(page.locator("#interstitial")).not_to_be_visible() -def test_should_work_with_a_custom_check(page: Page, server: Server) -> None: +@pytest.mark.parametrize( + "args", + [ + ["mouseover", 2], + ["none", 1], + ["remove", 1], + ["hide", 1], + ], +) +def test_should_work_with_a_custom_check( + page: Page, server: Server, args: list +) -> None: page.goto(server.PREFIX + "/input/handle-locator.html") def handler() -> None: @@ -82,20 +93,14 @@ def handler() -> None: page.add_locator_handler(page.locator("body"), handler, no_wait_after=True) - for args in [ - ["mouseover", 2], - ["none", 1], - ["remove", 1], - ["hide", 1], - ]: - page.locator("#aside").hover() - page.evaluate( - "(args) => { window.clicked = 0; window.setupAnnoyingInterstitial(...args); }", - args, - ) - page.locator("#target").click() - assert page.evaluate("window.clicked") == 1 - expect(page.locator("#interstitial")).not_to_be_visible() + page.locator("#aside").hover() + page.evaluate( + "(args) => { window.clicked = 0; window.setupAnnoyingInterstitial(...args); }", + args, + ) + page.locator("#target").click() + assert page.evaluate("window.clicked") == 1 + expect(page.locator("#interstitial")).not_to_be_visible() def test_should_work_with_keyword_only_handler(page: Page, server: Server) -> None: From 1522467f1e33868caed4c6bcaab34e0a1e9db185 Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 10:10:24 -0300 Subject: [PATCH 22/41] refactor(tests): split test_locators_should_select_textarea by browser using markers --- tests/sync/test_locators.py | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/tests/sync/test_locators.py b/tests/sync/test_locators.py index f085bb565..2f85da16d 100644 --- a/tests/sync/test_locators.py +++ b/tests/sync/test_locators.py @@ -352,19 +352,29 @@ def test_locators_should_scroll_into_view(page: Page, server: Server, i: int) -> assert after <= 0 -def test_locators_should_select_textarea( - page: Page, server: Server, browser_name: str -) -> None: +@pytest.fixture +def selected_textarea(page: Page, server: Server) -> Locator: page.goto(server.PREFIX + "/input/textarea.html") textarea = page.locator("textarea") textarea.evaluate("textarea => textarea.value = 'some value'") textarea.select_text() textarea.select_text(timeout=25_000) - if browser_name == "firefox" or browser_name == "webkit": - assert textarea.evaluate("el => el.selectionStart") == 0 - assert textarea.evaluate("el => el.selectionEnd") == 10 - else: - assert page.evaluate("window.getSelection().toString()") == "some value" + return textarea + + +@pytest.mark.skip_browser("chromium") +def test_locators_should_select_textarea_firefox_webkit( + selected_textarea: Locator, +) -> None: + assert selected_textarea.evaluate("el => el.selectionStart") == 0 + assert selected_textarea.evaluate("el => el.selectionEnd") == 10 + + +@pytest.mark.only_browser("chromium") +def test_locators_should_select_textarea_chromium( + page: Page, selected_textarea: Locator +) -> None: + assert page.evaluate("window.getSelection().toString()") == "some value" def test_locators_should_type(page: Page) -> None: From a8600d9df756d70fae28124b463b425a2bd3d6a2 Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 10:12:36 -0300 Subject: [PATCH 23/41] refactor(tests): parametrize locator factories in test_locators_has_does_not_encode_unicode --- tests/sync/test_locators.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/tests/sync/test_locators.py b/tests/sync/test_locators.py index 2f85da16d..04415118b 100644 --- a/tests/sync/test_locators.py +++ b/tests/sync/test_locators.py @@ -939,17 +939,22 @@ def test_locators_should_support_locator_and(page: Page) -> None: ).to_have_count(2) -def test_locators_has_does_not_encode_unicode(page: Page, server: Server) -> None: +@pytest.mark.parametrize( + "make_locator", + [ + lambda page: page.locator("button", has_text="Драматург"), + lambda page: page.locator("button", has_text=re.compile("Драматург")), + lambda page: page.locator("button", has=page.locator("text=Драматург")), + ], +) +def test_locators_has_does_not_encode_unicode( + page: Page, server: Server, make_locator: Callable[[Page], Locator] +) -> None: page.goto(server.EMPTY_PAGE) - locators = [ - page.locator("button", has_text="Драматург"), - page.locator("button", has_text=re.compile("Драматург")), - page.locator("button", has=page.locator("text=Драматург")), - ] - for locator in locators: - with pytest.raises(Error) as exc_info: - locator.click(timeout=1_000) - assert "Драматург" in exc_info.value.message + locator = make_locator(page) + with pytest.raises(Error) as exc_info: + locator.click(timeout=1_000) + assert "Драматург" in exc_info.value.message def test_locators_should_focus_and_blur_a_button(page: Page, server: Server) -> None: From f981cdba6f5e115512533f0067d1a119f1a79ada Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 10:14:59 -0300 Subject: [PATCH 24/41] refactor(tests): extract fetch JS constant and module-level handler in test_should_delete_header_with_undefined_value --- .../test_browsercontext_request_fallback.py | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/tests/sync/test_browsercontext_request_fallback.py b/tests/sync/test_browsercontext_request_fallback.py index eee3e8172..a13e26822 100644 --- a/tests/sync/test_browsercontext_request_fallback.py +++ b/tests/sync/test_browsercontext_request_fallback.py @@ -178,6 +178,25 @@ def handler_with_header_mods(route: Route) -> None: assert values == ["bar", "bar", "bar"] +FETCH_WITH_FOO_AND_BAR_HEADERS_JS = """ + async url => { + const data = await fetch(url, { + headers: { + foo: 'a', + bar: 'b', + } + }); + return data.text(); + } + """ + + +def delete_foo_header(route: Route, request: Request) -> None: + headers = request.all_headers() + del headers["foo"] + route.fallback(headers=headers) + + def test_should_delete_header_with_undefined_value( page: Page, context: BrowserContext, server: Server ) -> None: @@ -198,27 +217,10 @@ def capture_and_continue(route: Route, request: Request) -> None: route.continue_() context.route("**/*", capture_and_continue) - - def delete_foo_header(route: Route, request: Request) -> None: - headers = request.all_headers() - del headers["foo"] - route.fallback(headers=headers) - context.route(server.PREFIX + "/something", delete_foo_header) with server.expect_request("/something") as server_req_info: text = page.evaluate( - """ - async url => { - const data = await fetch(url, { - headers: { - foo: 'a', - bar: 'b', - } - }); - return data.text(); - } - """, - server.PREFIX + "/something", + FETCH_WITH_FOO_AND_BAR_HEADERS_JS, server.PREFIX + "/something" ) server_req = server_req_info.value assert text == "done" From 90b3ef5c5dd6ba83190115469d841363171d2662 Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 10:16:47 -0300 Subject: [PATCH 25/41] refactor(tests): split test_ignore_case_regex into value, array and not variants --- tests/sync/test_assertions.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/sync/test_assertions.py b/tests/sync/test_assertions.py index e218388a4..70b7b6aa9 100644 --- a/tests/sync/test_assertions.py +++ b/tests/sync/test_assertions.py @@ -356,7 +356,16 @@ def test_ignore_case_regex(page: Page, server: Server, method: str) -> None: ) assert expected_error_msg in str(excinfo.value) - # Array Variants + +@pytest.mark.parametrize( + "method", + ["to_have_text", "to_contain_text"], +) +def test_ignore_case_regex_array_variants( + page: Page, server: Server, method: str +) -> None: + page.goto(server.EMPTY_PAGE) + page.set_content("
apple BANANA
orange
") getattr(expect(page.locator("div")), method)( [re.compile("apple BANANA"), re.compile("orange")] ) @@ -369,6 +378,7 @@ def test_ignore_case_regex(page: Page, server: Server, method: str) -> None: [re.compile("apple banana"), re.compile("ORANGE")], timeout=300, ) + expected_error_msg = method.replace("_", " ") assert expected_error_msg in str(excinfo.value) # overrides regex flag with pytest.raises(AssertionError) as excinfo: @@ -382,7 +392,17 @@ def test_ignore_case_regex(page: Page, server: Server, method: str) -> None: ) assert expected_error_msg in str(excinfo.value) - # not variant + +@pytest.mark.parametrize( + "method", + ["to_have_text", "to_contain_text"], +) +def test_ignore_case_regex_not_variant( + page: Page, server: Server, method: str +) -> None: + page.goto(server.EMPTY_PAGE) + page.set_content("
apple BANANA
orange
") + expected_error_msg = method.replace("_", " ") getattr(expect(page.locator("div#target")), f"not_{method}")( re.compile("apple banana") ) From 3b9e9dced66767b6ba50b520a0a9d604bd4a6e3b Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 10:23:02 -0300 Subject: [PATCH 26/41] refactor(tests): parametrize repeated blocks in test_should_print_response_with_text_content_type_if_to_be_ok_fails --- tests/sync/test_assertions.py | 97 +++++++++++++++++------------------ 1 file changed, 48 insertions(+), 49 deletions(-) diff --git a/tests/sync/test_assertions.py b/tests/sync/test_assertions.py index 70b7b6aa9..16235ab5b 100644 --- a/tests/sync/test_assertions.py +++ b/tests/sync/test_assertions.py @@ -14,7 +14,7 @@ import datetime import re -from typing import Generator +from typing import Any, Callable, Generator import pytest @@ -810,62 +810,61 @@ def test_assertions_response_is_ok_fail(page: Page, server: Server) -> None: assert "← 404 Not Found" in error_message -def test_should_print_response_with_text_content_type_if_to_be_ok_fails( - page: Page, server: Server -) -> None: - server.set_route( - "/text-content-type", - lambda r: ( - r.setResponseCode(404), - r.setHeader("content-type", "text/plain"), - r.write(b"Text error"), - r.finish(), +@pytest.mark.parametrize( + "path, handler, body, shows_text", + [ + ( + "/text-content-type", + lambda r: ( + r.setResponseCode(404), + r.setHeader("content-type", "text/plain"), + r.write(b"Text error"), + r.finish(), + ), + "Text error", + True, ), - ) - server.set_route( - "/no-content-type", - lambda r: ( - r.setResponseCode(404), - r.write(b"No content type error"), - r.finish(), + ( + "/no-content-type", + lambda r: ( + r.setResponseCode(404), + r.write(b"No content type error"), + r.finish(), + ), + "No content type error", + False, ), - ) - server.set_route( - "/binary-content-type", - lambda r: ( - r.setResponseCode(404), - r.setHeader("content-type", "image/bmp"), - r.write(b"Image content type error"), - r.finish(), + ( + "/binary-content-type", + lambda r: ( + r.setResponseCode(404), + r.setHeader("content-type", "image/bmp"), + r.write(b"Image content type error"), + r.finish(), + ), + "Image content type error", + False, ), - ) - - response = page.request.get(server.PREFIX + "/text-content-type") - with pytest.raises(AssertionError) as excinfo: - expect(response).to_be_ok() - error_message = str(excinfo.value) - assert ("→ GET " + server.PREFIX + "/text-content-type") in error_message - assert "← 404 Not Found" in error_message - assert "Response Text:" in error_message - assert "Text error" in error_message - - response = page.request.get(server.PREFIX + "/no-content-type") - with pytest.raises(AssertionError) as excinfo: - expect(response).to_be_ok() - error_message = str(excinfo.value) - assert ("→ GET " + server.PREFIX + "/no-content-type") in error_message - assert "← 404 Not Found" in error_message - assert "Response Text:" not in error_message - assert "No content type error" not in error_message + ], +) +def test_should_print_response_with_text_content_type_if_to_be_ok_fails( + page: Page, + server: Server, + path: str, + handler: Callable[[Any], Any], + body: str, + shows_text: bool, +) -> None: + server.set_route(path, handler) - response = page.request.get(server.PREFIX + "/binary-content-type") + response = page.request.get(server.PREFIX + path) with pytest.raises(AssertionError) as excinfo: expect(response).to_be_ok() error_message = str(excinfo.value) - assert ("→ GET " + server.PREFIX + "/binary-content-type") in error_message + assert ("→ GET " + server.PREFIX + path) in error_message assert "← 404 Not Found" in error_message - assert "Response Text:" not in error_message - assert "Image content type error" not in error_message + assert ("Response Text:" in error_message) == shows_text + assert (body in error_message) == shows_text def test_should_print_users_message_for_page_based_assertion( From b9baa5175d32cc541ebb4bc63da57477ec7bfb79 Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Wed, 28 Oct 2026 10:26:30 -0300 Subject: [PATCH 27/41] refactor(tests): compare HAR entries against expected-value tables in test_should_attach_content --- tests/sync/test_har.py | 65 ++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 41 deletions(-) diff --git a/tests/sync/test_har.py b/tests/sync/test_har.py index f652a60f7..addd976e8 100644 --- a/tests/sync/test_har.py +++ b/tests/sync/test_har.py @@ -82,51 +82,34 @@ def test_should_attach_content( page.goto(server.PREFIX + "/har.html") page.evaluate("() => fetch('/pptr.png').then(r => r.arrayBuffer())") context.close() + html_hash = "75841480e2606c03389077304342fac2c58ccb1b" + css_hash = "79f739d7bc88e80f55b9891a22bf13a2b4e18adb" + png_hash = "a4c3a18f0bb83f5d9fe7ce561e065c36205762fa" with zipfile.ZipFile(path) as z: with z.open("har.har") as har: entries = json.load(har)["log"]["entries"] - assert "encoding" not in entries[0]["response"]["content"] - assert ( - entries[0]["response"]["content"]["mimeType"] - == "text/html; charset=utf-8" - ) - assert ( - "75841480e2606c03389077304342fac2c58ccb1b" - in entries[0]["response"]["content"]["_file"] - ) - assert entries[0]["response"]["content"]["size"] >= 96 - assert entries[0]["response"]["content"]["compression"] == 0 - - assert "encoding" not in entries[1]["response"]["content"] - assert ( - entries[1]["response"]["content"]["mimeType"] - == "text/css; charset=utf-8" - ) - assert ( - "79f739d7bc88e80f55b9891a22bf13a2b4e18adb" - in entries[1]["response"]["content"]["_file"] - ) - assert entries[1]["response"]["content"]["size"] >= 37 - assert entries[1]["response"]["content"]["compression"] == 0 - - assert "encoding" not in entries[2]["response"]["content"] - assert entries[2]["response"]["content"]["mimeType"] == "image/png" - assert ( - "a4c3a18f0bb83f5d9fe7ce561e065c36205762fa" - in entries[2]["response"]["content"]["_file"] - ) - assert entries[2]["response"]["content"]["size"] >= 6000 - assert entries[2]["response"]["content"]["compression"] == 0 - - with z.open("75841480e2606c03389077304342fac2c58ccb1b.html") as f: - assert b"HAR Page" in f.read() - - with z.open("79f739d7bc88e80f55b9891a22bf13a2b4e18adb.css") as f: - assert b"pink" in f.read() - - with z.open("a4c3a18f0bb83f5d9fe7ce561e065c36205762fa.png") as f: - assert len(f.read()) == entries[2]["response"]["content"]["size"] + contents = [entry["response"]["content"] for entry in entries[:3]] + assert all("encoding" not in content for content in contents) + assert [content["mimeType"] for content in contents] == [ + "text/html; charset=utf-8", + "text/css; charset=utf-8", + "image/png", + ] + files = [content["_file"] for content in contents] + assert all(h in f for h, f in zip([html_hash, css_hash, png_hash], files)) + sizes = [content["size"] for content in contents] + assert all(size >= low for size, low in zip(sizes, [96, 37, 6000])) + assert [content["compression"] for content in contents] == [0, 0, 0] + + with z.open(f"{html_hash}.html") as f: + assert b"HAR Page" in f.read() + + with z.open(f"{css_hash}.css") as f: + assert b"pink" in f.read() + + with z.open(f"{png_hash}.png") as f: + assert len(f.read()) == contents[2]["size"] def test_should_include_content( From 02bca998e8056f3d0fa459fdbb0478933aec0614 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 12:41:33 -0300 Subject: [PATCH 28/41] test(screencast): use context manager in acknowledge frames test Replace the try/finally around screencast.start() with a with block, so stop() runs on exit without manual exception handling. Smell: Exception Handling --- tests/sync/test_screencast.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/sync/test_screencast.py b/tests/sync/test_screencast.py index ece9bf70a..dc96b95e4 100644 --- a/tests/sync/test_screencast.py +++ b/tests/sync/test_screencast.py @@ -60,8 +60,7 @@ def on_frame(_: object) -> None: frame_count += 1 assert page.title() == "" - page.screencast.start(on_frame=on_frame) - try: + with page.screencast.start(on_frame=on_frame): page.goto(server.EMPTY_PAGE) page.evaluate("() => document.body.style.backgroundColor = 'red'") ensure_some_frames(page) @@ -69,8 +68,6 @@ def on_frame(_: object) -> None: page.evaluate("() => document.body.style.backgroundColor = 'blue'") ensure_some_frames(page) assert frame_count > frames_before_repaint - finally: - page.screencast.stop() def test_should_report_callback_errors_and_continue_delivering_frames( From 7eacb4ef3b7640c6ac2944a7b80bcfc29cdb746a Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 12:42:24 -0300 Subject: [PATCH 29/41] test(screencast): reuse ensure_some_frames in deliver frames test Give ensure_some_frames a count parameter (default 100, so other callers are unchanged) and use it with count=3 instead of repeating the inline paint-cycle loop followed by a screenshot. Smell: Conditional Test Logic --- tests/sync/test_screencast.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/sync/test_screencast.py b/tests/sync/test_screencast.py index dc96b95e4..b61179d52 100644 --- a/tests/sync/test_screencast.py +++ b/tests/sync/test_screencast.py @@ -20,8 +20,8 @@ from tests.server import Server -def ensure_some_frames(page: Page) -> None: - for _ in range(100): +def ensure_some_frames(page: Page, count: int = 100) -> None: + for _ in range(count): page.evaluate( "() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f)))" ) @@ -39,11 +39,7 @@ def test_start_should_deliver_frames_via_callback(page: Page, server: Server) -> page.evaluate("() => document.body.style.backgroundColor = 'red'") # Force a couple of paint cycles so engines that only emit on visual change # still produce a frame. Mirrors upstream `ensureSomeFrames`. - for _ in range(3): - page.evaluate( - "() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f)))" - ) - page.screenshot() + ensure_some_frames(page, count=3) deadline = time.time() + 10 while not received and time.time() < deadline: page.wait_for_timeout(100) From b192e3fe017dc1d9c32c675ba1ad21cd7283574a Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 12:43:35 -0300 Subject: [PATCH 30/41] test(element-handle): parametrize the scroll test over the buttons Run test_scroll once per offscreen button with pytest.mark.parametrize instead of looping inside a single test. Each case now gets a fresh page, so the scrollTo(0, 0) reset between iterations is no longer needed. Smell: Conditional Test Logic --- tests/sync/test_element_handle.py | 39 +++++++++++++++---------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/tests/sync/test_element_handle.py b/tests/sync/test_element_handle.py index c2faa4a6e..3ff5619f6 100644 --- a/tests/sync/test_element_handle.py +++ b/tests/sync/test_element_handle.py @@ -362,27 +362,26 @@ def test_hover_when_node_is_removed(page: Page, server: Server) -> None: assert page.evaluate('document.querySelector("button:hover").id') == "button-6" -def test_scroll(page: Page, server: Server) -> None: +@pytest.mark.parametrize("i", range(11)) +def test_scroll(page: Page, server: Server, i: int) -> None: page.goto(server.PREFIX + "/offscreenbuttons.html") - for i in range(11): - button = page.query_selector(f"#btn{i}") - assert button - before = button.evaluate( - """button => { - return button.getBoundingClientRect().right - window.innerWidth - }""" - ) - - assert before == 10 * i - button.scroll_into_view_if_needed() - after = button.evaluate( - """button => { - return button.getBoundingClientRect().right - window.innerWidth - }""" - ) - - assert after <= 0 - page.evaluate("() => window.scrollTo(0, 0)") + button = page.query_selector(f"#btn{i}") + assert button + before = button.evaluate( + """button => { + return button.getBoundingClientRect().right - window.innerWidth + }""" + ) + + assert before == 10 * i + button.scroll_into_view_if_needed() + after = button.evaluate( + """button => { + return button.getBoundingClientRect().right - window.innerWidth + }""" + ) + + assert after <= 0 def test_scroll_should_throw_for_detached_element(page: Page) -> None: From c4e0784fae1898d78885b33a45bf7d17e547eb87 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 12:44:06 -0300 Subject: [PATCH 31/41] test(sync): unroll the closed pages loop in multiple pages test Check each closed page with its own pytest.raises block instead of looping over a list of pages, so a failure points at the exact page. Smell: Conditional Test Logic --- tests/sync/test_sync.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/sync/test_sync.py b/tests/sync/test_sync.py index 8e11a0e17..d952fc4b0 100644 --- a/tests/sync/test_sync.py +++ b/tests/sync/test_sync.py @@ -144,9 +144,10 @@ def test_sync_handle_multiple_pages(context: BrowserContext) -> None: assert len(context.pages) == 1 page2.close() assert len(context.pages) == 0 - for page in [page1, page2]: - with pytest.raises(Error): - page.content() + with pytest.raises(Error): + page1.content() + with pytest.raises(Error): + page2.content() def test_sync_wait_for_event(page: Page, server: Server) -> None: From 6672e7eac68d6a3d1377aeff0bc3857f5c62f239 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 13:09:38 -0300 Subject: [PATCH 32/41] test(storage-state): split local storage and indexed db checks test_should_set_local_storage verified two unrelated things in about 50 lines. Move the shared storage state, context and page setup into a fixture and check localStorage and IndexedDB in separate tests. Smell: Verbose Test --- tests/sync/test_browsercontext_storage_state.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/sync/test_browsercontext_storage_state.py b/tests/sync/test_browsercontext_storage_state.py index db2a06748..a0c9a9497 100644 --- a/tests/sync/test_browsercontext_storage_state.py +++ b/tests/sync/test_browsercontext_storage_state.py @@ -14,6 +14,7 @@ import json from pathlib import Path +from typing import Generator import pytest @@ -43,7 +44,8 @@ def test_should_capture_local_storage(context: BrowserContext) -> None: } -def test_should_set_local_storage(browser: Browser) -> None: +@pytest.fixture +def page_with_storage_state(browser: Browser) -> Generator[Page, None, None]: storage_state: StorageState = { "origins": [ { @@ -72,10 +74,17 @@ def test_should_set_local_storage(browser: Browser) -> None: page = context.new_page() page.route("**/*", lambda route: route.fulfill(body="")) page.goto("https://www.example.com") - local_storage = page.evaluate("window.localStorage") + yield page + context.close() + + +def test_should_set_local_storage(page_with_storage_state: Page) -> None: + local_storage = page_with_storage_state.evaluate("window.localStorage") assert local_storage == {"name1": "value1"} - indexed_db = page.evaluate( + +def test_should_set_indexed_db(page_with_storage_state: Page) -> None: + indexed_db = page_with_storage_state.evaluate( """async () => { return new Promise((resolve, reject) => { const openRequest = indexedDB.open('db', 42); @@ -91,7 +100,6 @@ def test_should_set_local_storage(browser: Browser) -> None: }""" ) assert indexed_db == "foo" - context.close() def test_should_round_trip_through_the_file( From 3a4f98aa921b66b024d5b7023f39573478a99b06 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 13:10:33 -0300 Subject: [PATCH 33/41] test(browsertype-connect): use pytest.approx for file timestamps Replace the index loop with one assertion comparing all last-modified timestamps with pytest.approx(abs=1000). The tolerance is the same, but the bound is now inclusive (<= 1000 instead of < 1000). Smell: Conditional Test Logic --- tests/sync/test_browsertype_connect.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/sync/test_browsertype_connect.py b/tests/sync/test_browsertype_connect.py index 74970c545..6157e24b0 100644 --- a/tests/sync/test_browsertype_connect.py +++ b/tests/sync/test_browsertype_connect.py @@ -247,5 +247,4 @@ def test_set_input_files_should_preserve_last_modified_timestamp( # On Linux browser sometimes reduces the timestamp by 1ms: 1696272058110.0715 -> 1696272058109 or even # rounds it to seconds in WebKit: 1696272058110 -> 1696272058000. - for i in range(len(timestamps)): - assert abs(timestamps[i] - expected_timestamps[i]) < 1000 + assert timestamps == pytest.approx(expected_timestamps, abs=1000) From 621fc631a0482b8e96b9462c88c10941c905db98 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 13:11:11 -0300 Subject: [PATCH 34/41] test(sync): pick expected console text from a dict Choose the expected console message text with a dict lookup keyed by browser_name instead of an if/else, keeping the same test and the same assertions for every browser. Smell: Conditional Test Logic --- tests/sync/test_sync.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/sync/test_sync.py b/tests/sync/test_sync.py index d952fc4b0..c7b669538 100644 --- a/tests/sync/test_sync.py +++ b/tests/sync/test_sync.py @@ -206,12 +206,11 @@ def test_console_should_work(page: Page, browser_name: str) -> None: page.evaluate('() => console.log("hello", 5, {foo: "bar"})') assert len(messages) == 1 message = messages[0] - if browser_name != "firefox": - assert message.text == "hello 5 {foo: bar}" - assert str(message) == "hello 5 {foo: bar}" - else: - assert message.text == "hello 5 JSHandle@object" - assert str(message) == "hello 5 JSHandle@object" + expected_text = {"firefox": "hello 5 JSHandle@object"}.get( + browser_name, "hello 5 {foo: bar}" + ) + assert message.text == expected_text + assert str(message) == expected_text assert message.type == "log" assert message.args[0].json_value() == "hello" assert message.args[1].json_value() == 5 From 25445a0342fd4cd1c3bcd56fa4956930bc8eba3f Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 13:11:33 -0300 Subject: [PATCH 35/41] test(console): pick expected console text from a dict Choose the expected console message text with a dict lookup keyed by browser_name instead of an if/else, keeping the same test and the same assertions for every browser. Smell: Conditional Test Logic --- tests/sync/test_console.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/sync/test_console.py b/tests/sync/test_console.py index 60680a46b..534f5d4f5 100644 --- a/tests/sync/test_console.py +++ b/tests/sync/test_console.py @@ -32,12 +32,11 @@ def test_console_should_work(page: Page, browser_name: str) -> None: page.evaluate('() => console.log("hello", 5, {foo: "bar"})') assert len(messages) == 1 message = messages[0] - if browser_name != "firefox": - assert message.text == "hello 5 {foo: bar}" - assert str(message) == "hello 5 {foo: bar}" - else: - assert message.text == "hello 5 JSHandle@object" - assert str(message) == "hello 5 JSHandle@object" + expected_text = {"firefox": "hello 5 JSHandle@object"}.get( + browser_name, "hello 5 {foo: bar}" + ) + assert message.text == expected_text + assert str(message) == expected_text assert message.type == "log" assert message.args[0].json_value() == "hello" assert message.args[1].json_value() == 5 From b5265d32e7202fae002db8933da9d4551b1ff8b4 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 15:03:23 -0300 Subject: [PATCH 36/41] test(console): pick window object console text from a dict Replace the if/else on browser_name with a lookup of the expected text, keeping the same test, ID and assertion. Smell: Conditional Test Logic --- tests/sync/test_console.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/sync/test_console.py b/tests/sync/test_console.py index 534f5d4f5..241c95311 100644 --- a/tests/sync/test_console.py +++ b/tests/sync/test_console.py @@ -101,10 +101,8 @@ def test_console_should_not_fail_for_window_object( with page.expect_console_message(): page.evaluate("() => console.error(window)") assert len(messages) == 1 - if browser_name != "firefox": - assert messages[0].text == "Window" - else: - assert messages[0].text == "JSHandle@object" + expected_text = {"firefox": "JSHandle@object"}.get(browser_name, "Window") + assert messages[0].text == expected_text # Upstream issue https://bugs.webkit.org/show_bug.cgi?id=229515 From 4bf95896e0e7e9bba5d7e98fbd30373582fcaa15 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 15:04:50 -0300 Subject: [PATCH 37/41] test(fetch-browser-context): move http server lifecycle into a fixture The try/finally that stopped the HTTPServer now lives in a yield fixture, so the timing test only holds the assertions. Assertions are unchanged. Smell: Exception Handling --- tests/sync/test_fetch_browser_context.py | 43 +++++++++++++----------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/tests/sync/test_fetch_browser_context.py b/tests/sync/test_fetch_browser_context.py index 91390791e..795befedd 100644 --- a/tests/sync/test_fetch_browser_context.py +++ b/tests/sync/test_fetch_browser_context.py @@ -15,7 +15,7 @@ import json import time from pathlib import Path -from typing import Any, Dict, List, cast +from typing import Any, Dict, Generator, List, cast from urllib.parse import parse_qs import pytest @@ -60,24 +60,29 @@ def test_fetch_should_work(context: BrowserContext, server: Server) -> None: assert response.text() == '{"foo": "bar"}\n' -def test_should_return_timing(context: BrowserContext) -> None: - fresh_server = HTTPServer() - fresh_server.start() - try: - response = context.request.get(fresh_server.EMPTY_PAGE) - timing = response.timing - assert abs(timing["startTime"] - time.time() * 1000) < 10000 - assert timing["domainLookupStart"] == 0 - assert timing["domainLookupEnd"] >= timing["domainLookupStart"] - assert timing["connectStart"] == timing["domainLookupEnd"] - assert timing["secureConnectionStart"] == -1 - assert timing["connectEnd"] >= timing["connectStart"] - assert timing["requestStart"] == timing["connectEnd"] - assert timing["responseStart"] >= timing["requestStart"] - assert timing["responseEnd"] >= timing["responseStart"] - assert timing["responseEnd"] < 60000 - finally: - fresh_server.stop() +@pytest.fixture +def fresh_server() -> Generator[HTTPServer, None, None]: + http_server = HTTPServer() + http_server.start() + yield http_server + http_server.stop() + + +def test_should_return_timing( + context: BrowserContext, fresh_server: HTTPServer +) -> None: + response = context.request.get(fresh_server.EMPTY_PAGE) + timing = response.timing + assert abs(timing["startTime"] - time.time() * 1000) < 10000 + assert timing["domainLookupStart"] == 0 + assert timing["domainLookupEnd"] >= timing["domainLookupStart"] + assert timing["connectStart"] == timing["domainLookupEnd"] + assert timing["secureConnectionStart"] == -1 + assert timing["connectEnd"] >= timing["connectStart"] + assert timing["requestStart"] == timing["connectEnd"] + assert timing["responseStart"] >= timing["requestStart"] + assert timing["responseEnd"] >= timing["responseStart"] + assert timing["responseEnd"] < 60000 def test_should_throw_on_network_error(context: BrowserContext, server: Server) -> None: From a1166b1d389bb4dec9e217c34a6c68ddff6763e5 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 15:06:58 -0300 Subject: [PATCH 38/41] test(queryselector): split main and isolated world checks into smaller tests Move the engine registration and page setup into a fixture and split the long test into main world, isolated world and chaining tests. Every assertion is kept as is. Smell: Verbose Test --- tests/sync/test_queryselector.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/tests/sync/test_queryselector.py b/tests/sync/test_queryselector.py index 27b972e95..ce40c8cbd 100644 --- a/tests/sync/test_queryselector.py +++ b/tests/sync/test_queryselector.py @@ -75,9 +75,8 @@ def test_selectors_register_should_work_with_path( assert page.eval_on_selector("foo=whatever", "e => e.nodeName") == "SECTION" -def test_selectors_register_should_work_in_main_and_isolated_world( - selectors: Selectors, page: Page, utils: Utils -) -> None: +@pytest.fixture +def main_and_isolated_engines(selectors: Selectors, page: Page, utils: Utils) -> None: dummy_selector_script = """{ create(root, target) { }, query(root, selector) { @@ -94,6 +93,10 @@ def test_selectors_register_should_work_in_main_and_isolated_world( ) page.set_content("
") page.evaluate('() => window.__answer = document.querySelector("span")') + + +@pytest.mark.usefixtures("main_and_isolated_engines") +def test_selectors_register_should_work_in_main_world(page: Page) -> None: # Works in main if asked. assert page.eval_on_selector("main=ignored", "e => e.nodeName") == "SPAN" assert page.eval_on_selector("css=div >> main=ignored", "e => e.nodeName") == "SPAN" @@ -103,6 +106,10 @@ def test_selectors_register_should_work_in_main_and_isolated_world( assert ( page.eval_on_selector_all("main=ignored", "es => es.filter(e => e).length") == 3 ) + + +@pytest.mark.usefixtures("main_and_isolated_engines") +def test_selectors_register_should_work_in_isolated_world(page: Page) -> None: # Works in isolated by default. assert page.query_selector("isolated=ignored") is None assert page.query_selector("css=div >> isolated=ignored") is None @@ -114,6 +121,10 @@ def test_selectors_register_should_work_in_main_and_isolated_world( page.eval_on_selector_all("isolated=ignored", "es => es.filter(e => e).length") == 3 ) + + +@pytest.mark.usefixtures("main_and_isolated_engines") +def test_selectors_register_should_chain_main_and_isolated_engines(page: Page) -> None: # At least one engine in main forces all to be in main. assert ( page.eval_on_selector("main=ignored >> isolated=ignored", "e => e.nodeName") From 3f00884845c51efba0983c617914fd81cf0a7e83 Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Wed, 28 Oct 2026 15:10:05 -0300 Subject: [PATCH 39/41] test(screencast): extract the frame polling loop into a helper Move the deadline while loop to wait_for_received_frames, next to ensure_some_frames. Same 10s deadline and 100ms polling step, so timing is unchanged. Smell: Conditional Test Logic --- tests/sync/test_screencast.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/sync/test_screencast.py b/tests/sync/test_screencast.py index b61179d52..95d2b02cf 100644 --- a/tests/sync/test_screencast.py +++ b/tests/sync/test_screencast.py @@ -28,6 +28,12 @@ def ensure_some_frames(page: Page, count: int = 100) -> None: page.screenshot() +def wait_for_received_frames(page: Page, received: list, timeout: float = 10) -> None: + deadline = time.time() + timeout + while not received and time.time() < deadline: + page.wait_for_timeout(100) + + def test_should_expose_screencast_property(page: Page) -> None: assert page.screencast is page.screencast @@ -40,9 +46,7 @@ def test_start_should_deliver_frames_via_callback(page: Page, server: Server) -> # Force a couple of paint cycles so engines that only emit on visual change # still produce a frame. Mirrors upstream `ensureSomeFrames`. ensure_some_frames(page, count=3) - deadline = time.time() + 10 - while not received and time.time() < deadline: - page.wait_for_timeout(100) + wait_for_received_frames(page, received) page.screencast.stop() assert len(received) >= 1 assert all(isinstance(d, bytes) and len(d) > 0 for d in received) From cf5867b791c35ccdf1bfd1327de1ed7233c2c8ad Mon Sep 17 00:00:00 2001 From: ClidenorFilho Date: Fri, 30 Oct 2026 16:14:44 -0300 Subject: [PATCH 40/41] refactor(tests): replace browser if/elif with dict lookup in test_should_not_chain_abort (page_request_fallback) --- tests/sync/test_page_request_fallback.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/sync/test_page_request_fallback.py b/tests/sync/test_page_request_fallback.py index 53570960c..f450d9b19 100644 --- a/tests/sync/test_page_request_fallback.py +++ b/tests/sync/test_page_request_fallback.py @@ -110,7 +110,7 @@ def handler(route: Route) -> None: def test_should_not_chain_abort( - page: Page, server: Server, is_webkit: bool, is_firefox: bool + page: Page, server: Server, browser_name: str ) -> None: failed: List[bool] = [False] @@ -123,12 +123,11 @@ def handler(route: Route) -> None: with pytest.raises(Error) as excinfo: page.goto(server.EMPTY_PAGE) - if is_webkit: - assert "Blocked by Web Inspector" in excinfo.value.message - elif is_firefox: - assert "NS_ERROR_FAILURE" in excinfo.value.message - else: - assert "net::ERR_FAILED" in excinfo.value.message + expected_message = { + "webkit": "Blocked by Web Inspector", + "firefox": "NS_ERROR_FAILURE", + }.get(browser_name, "net::ERR_FAILED") + assert expected_message in excinfo.value.message assert not failed[0] From 596031c7e968fe21545f70c1618cd1328fbc8ebe Mon Sep 17 00:00:00 2001 From: Cauan Ricardo Date: Sun, 1 Nov 2026 15:34:08 -0300 Subject: [PATCH 41/41] style(tests): format signatures with black --- tests/sync/test_assertions.py | 4 +--- tests/sync/test_page_request_fallback.py | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/sync/test_assertions.py b/tests/sync/test_assertions.py index 16235ab5b..99af93286 100644 --- a/tests/sync/test_assertions.py +++ b/tests/sync/test_assertions.py @@ -397,9 +397,7 @@ def test_ignore_case_regex_array_variants( "method", ["to_have_text", "to_contain_text"], ) -def test_ignore_case_regex_not_variant( - page: Page, server: Server, method: str -) -> None: +def test_ignore_case_regex_not_variant(page: Page, server: Server, method: str) -> None: page.goto(server.EMPTY_PAGE) page.set_content("
apple BANANA
orange
") expected_error_msg = method.replace("_", " ") diff --git a/tests/sync/test_page_request_fallback.py b/tests/sync/test_page_request_fallback.py index f450d9b19..7fd1bd8c2 100644 --- a/tests/sync/test_page_request_fallback.py +++ b/tests/sync/test_page_request_fallback.py @@ -109,9 +109,7 @@ def handler(route: Route) -> None: assert not failed[0] -def test_should_not_chain_abort( - page: Page, server: Server, browser_name: str -) -> None: +def test_should_not_chain_abort(page: Page, server: Server, browser_name: str) -> None: failed: List[bool] = [False] def handler(route: Route) -> None: