From 0310e11c8e614fe98aedd81f282299caf924dbe0 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Thu, 7 Jul 2022 02:17:12 -0700 Subject: [PATCH 1/3] chore: catch leaky tests Relates #1412. --- tests/async/conftest.py | 8 ++++++++ tests/async/test_har.py | 7 +++++++ tests/sync/conftest.py | 4 ++++ tests/sync/test_har.py | 7 +++++++ 4 files changed, 26 insertions(+) diff --git a/tests/async/conftest.py b/tests/async/conftest.py index 1048be55c..a96fd3ec5 100644 --- a/tests/async/conftest.py +++ b/tests/async/conftest.py @@ -57,6 +57,10 @@ async def launch(**kwargs): yield launch for browser in browsers: + if browser.contexts: + raise Exception( + "Test failed to close a context it created via browser_factory fixture. Explicitly close context, or use context_factory." + ) await browser.close() @@ -64,6 +68,10 @@ async def launch(**kwargs): async def browser(browser_factory): browser = await browser_factory() yield browser + if browser.contexts: + raise Exception( + "Test failed to close a context it created via browser fixture. Explicitly close context, or use context_factory." + ) await browser.close() diff --git a/tests/async/test_har.py b/tests/async/test_har.py index cd1c871a6..fb820232d 100644 --- a/tests/async/test_har.py +++ b/tests/async/test_har.py @@ -503,6 +503,7 @@ async def test_should_round_trip_har_zip( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() async def test_should_round_trip_har_with_post_data( @@ -536,6 +537,7 @@ async def test_should_round_trip_har_with_post_data( assert await page_2.evaluate(fetch_function, "3") == "3" with pytest.raises(Exception): await page_2.evaluate(fetch_function, "4") + await context_2.close() async def test_should_disambiguate_by_header( @@ -578,6 +580,7 @@ async def test_should_disambiguate_by_header( assert await page_2.evaluate(fetch_function, "baz2") == "baz2" assert await page_2.evaluate(fetch_function, "baz3") == "baz3" assert await page_2.evaluate(fetch_function, "baz4") == "baz1" + await context_2.close() async def test_should_produce_extracted_zip( @@ -605,6 +608,7 @@ async def test_should_produce_extracted_zip( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() async def test_should_update_har_zip_for_context( @@ -627,6 +631,7 @@ async def test_should_update_har_zip_for_context( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() async def test_should_update_har_zip_for_page( @@ -649,6 +654,7 @@ async def test_should_update_har_zip_for_page( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() async def test_should_update_extracted_har_zip_for_page( @@ -675,3 +681,4 @@ async def test_should_update_extracted_har_zip_for_page( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() diff --git a/tests/sync/conftest.py b/tests/sync/conftest.py index d7942bf90..ad2f4e297 100644 --- a/tests/sync/conftest.py +++ b/tests/sync/conftest.py @@ -63,6 +63,10 @@ def browser( ) -> Generator[Browser, None, None]: browser = browser_type.launch(**launch_arguments) yield browser + if browser.contexts: + raise Exception( + "Test failed to close a context it created via browser fixture. Explicitly close context, or use context fixture" + ) browser.close() diff --git a/tests/sync/test_har.py b/tests/sync/test_har.py index 81452c9de..c5b20ed56 100644 --- a/tests/sync/test_har.py +++ b/tests/sync/test_har.py @@ -438,6 +438,7 @@ def test_should_round_trip_har_zip( page_2.goto(server.PREFIX + "/one-style.html") assert "hello, world!" in page_2.content() expect(page_2.locator("body")).to_have_css("background-color", "rgb(255, 192, 203)") + context_2.close() def test_should_round_trip_har_with_post_data( @@ -471,6 +472,7 @@ def test_should_round_trip_har_with_post_data( assert page_2.evaluate(fetch_function, "3") == "3" with pytest.raises(Exception): page_2.evaluate(fetch_function, "4") + context_2.close() def test_should_disambiguate_by_header( @@ -512,6 +514,7 @@ def test_should_disambiguate_by_header( assert page_2.evaluate(fetch_function, "baz2") == "baz2" assert page_2.evaluate(fetch_function, "baz3") == "baz3" assert page_2.evaluate(fetch_function, "baz4") == "baz1" + context_2.close() def test_should_produce_extracted_zip( @@ -537,6 +540,7 @@ def test_should_produce_extracted_zip( page_2.goto(server.PREFIX + "/one-style.html") assert "hello, world!" in page_2.content() expect(page_2.locator("body")).to_have_css("background-color", "rgb(255, 192, 203)") + context_2.close() def test_should_update_har_zip_for_context( @@ -557,6 +561,7 @@ def test_should_update_har_zip_for_context( page_2.goto(server.PREFIX + "/one-style.html") assert "hello, world!" in page_2.content() expect(page_2.locator("body")).to_have_css("background-color", "rgb(255, 192, 203)") + context_2.close() def test_should_update_har_zip_for_page( @@ -577,6 +582,7 @@ def test_should_update_har_zip_for_page( page_2.goto(server.PREFIX + "/one-style.html") assert "hello, world!" in page_2.content() expect(page_2.locator("body")).to_have_css("background-color", "rgb(255, 192, 203)") + context_2.close() def test_should_update_extracted_har_zip_for_page( @@ -601,3 +607,4 @@ def test_should_update_extracted_har_zip_for_page( page_2.goto(server.PREFIX + "/one-style.html") assert "hello, world!" in page_2.content() expect(page_2.locator("body")).to_have_css("background-color", "rgb(255, 192, 203)") + context_2.close() From b0a684f03a957b0c043b563c69d6e3cb3f1e0d16 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Thu, 7 Jul 2022 02:57:49 -0700 Subject: [PATCH 2/3] close contexts and browsers --- tests/async/test_browsercontext.py | 1 + tests/async/test_browsercontext_add_cookies.py | 2 ++ tests/async/test_browsertype_connect.py | 10 ++++++++-- tests/async/test_tracing.py | 2 ++ tests/sync/test_browsertype_connect.py | 11 ++++++++--- tests/sync/test_tracing.py | 2 ++ 6 files changed, 23 insertions(+), 5 deletions(-) diff --git a/tests/async/test_browsercontext.py b/tests/async/test_browsercontext.py index a21996272..92e49389a 100644 --- a/tests/async/test_browsercontext.py +++ b/tests/async/test_browsercontext.py @@ -751,3 +751,4 @@ async def test_should_support_forced_colors(browser: Browser): page = await context.new_page() assert await page.evaluate("matchMedia('(forced-colors: active)').matches") assert not await page.evaluate("matchMedia('(forced-colors: none)').matches") + await context.close() diff --git a/tests/async/test_browsercontext_add_cookies.py b/tests/async/test_browsercontext_add_cookies.py index 6ce2e6088..7a34a31bb 100644 --- a/tests/async/test_browsercontext_add_cookies.py +++ b/tests/async/test_browsercontext_add_cookies.py @@ -174,12 +174,14 @@ async def test_should_isolate_cookies_between_launches(browser_factory, server): } ] ) + await context_1.close() await browser_1.close() browser_2 = await browser_factory() context_2 = await browser_2.new_context() cookies = await context_2.cookies() assert cookies == [] + await context_2.close() await browser_2.close() diff --git a/tests/async/test_browsertype_connect.py b/tests/async/test_browsertype_connect.py index f297892f4..6d90509a5 100644 --- a/tests/async/test_browsertype_connect.py +++ b/tests/async/test_browsertype_connect.py @@ -34,12 +34,14 @@ async def test_browser_type_connect_should_be_able_to_reconnect_to_a_browser( assert len(browser_context.pages) == 1 assert await page.evaluate("11 * 11") == 121 await page.goto(server.EMPTY_PAGE) + browser_context.close() await browser.close() browser = await browser_type.connect(remote_server.ws_endpoint) browser_context = await browser.new_context() page = await browser_context.new_page() await page.goto(server.EMPTY_PAGE) + browser_context.close() await browser.close() @@ -49,20 +51,22 @@ async def test_browser_type_connect_should_be_able_to_connect_two_browsers_at_th remote_server = launch_server() browser1 = await browser_type.connect(remote_server.ws_endpoint) assert len(browser1.contexts) == 0 - await browser1.new_context() + browser1_context = await browser1.new_context() assert len(browser1.contexts) == 1 browser2 = await browser_type.connect(remote_server.ws_endpoint) assert len(browser2.contexts) == 0 - await browser2.new_context() + browser2_context = await browser2.new_context() assert len(browser2.contexts) == 1 assert len(browser1.contexts) == 1 + await browser1_context.close() await browser1.close() page2 = await browser2.new_page() # original browser should still work assert await page2.evaluate("7 * 6") == 42 + await browser2_context.close() await browser2.close() @@ -297,3 +301,5 @@ async def test_should_upload_large_file( ) assert match.group("name") == b"file1" assert match.group("filename") == b"200MB.zip" + await context.close() + await browser.close() diff --git a/tests/async/test_tracing.py b/tests/async/test_tracing.py index f20578d46..3e5d51019 100644 --- a/tests/async/test_tracing.py +++ b/tests/async/test_tracing.py @@ -30,6 +30,7 @@ async def test_browser_context_output_trace( await page.goto(server.PREFIX + "/grid.html") await context.tracing.stop(path=tmp_path / "trace.zip") assert Path(tmp_path / "trace.zip").exists() + await context.close() async def test_browser_context_should_not_throw_when_stopping_without_start_but_not_exporting( @@ -56,6 +57,7 @@ async def test_browser_context_output_trace_chunk( await button.click() await context.tracing.stop_chunk(path=tmp_path / "trace2.zip") assert Path(tmp_path / "trace2.zip").exists() + await context.close() async def test_should_collect_sources( diff --git a/tests/sync/test_browsertype_connect.py b/tests/sync/test_browsertype_connect.py index bb8c5af5c..851dc87f6 100644 --- a/tests/sync/test_browsertype_connect.py +++ b/tests/sync/test_browsertype_connect.py @@ -47,12 +47,14 @@ def test_browser_type_connect_should_be_able_to_reconnect_to_a_browser( assert len(browser_context.pages) == 1 assert page.evaluate("11 * 11") == 121 page.goto(server.EMPTY_PAGE) + browser_context.close() browser.close() browser = browser_type.connect(remote_server.ws_endpoint) browser_context = browser.new_context() page = browser_context.new_page() page.goto(server.EMPTY_PAGE) + browser_context.close() browser.close() @@ -62,20 +64,22 @@ def test_browser_type_connect_should_be_able_to_connect_two_browsers_at_the_same remote_server = launch_server() browser1 = browser_type.connect(remote_server.ws_endpoint) assert len(browser1.contexts) == 0 - browser1.new_context() + browser1_context = browser1.new_context() assert len(browser1.contexts) == 1 browser2 = browser_type.connect(remote_server.ws_endpoint) assert len(browser2.contexts) == 0 - browser2.new_context() + browser2_context = browser2.new_context() assert len(browser2.contexts) == 1 assert len(browser1.contexts) == 1 + browser1_context.close() browser1.close() page2 = browser2.new_page() # original browser should still work assert page2.evaluate("7 * 6") == 42 + browser2_context.close() browser2.close() @@ -219,5 +223,6 @@ def handle_request(route: Route) -> None: assert response assert response.status == 200 assert response.json() == {"foo": "bar"} - + context.close() + browser.close() remote.kill() diff --git a/tests/sync/test_tracing.py b/tests/sync/test_tracing.py index 94107f1df..5d9145382 100644 --- a/tests/sync/test_tracing.py +++ b/tests/sync/test_tracing.py @@ -30,6 +30,7 @@ def test_browser_context_output_trace( page.goto(server.PREFIX + "/grid.html") context.tracing.stop(path=tmp_path / "trace.zip") assert Path(tmp_path / "trace.zip").exists() + context.close() def test_browser_context_should_not_throw_when_stopping_without_start_but_not_exporting( @@ -56,6 +57,7 @@ def test_browser_context_output_trace_chunk( button.click() context.tracing.stop_chunk(path=tmp_path / "trace2.zip") assert Path(tmp_path / "trace2.zip").exists() + context.close() def test_should_collect_sources( From 423edb5a432569e0a0475635bf807e02100183d1 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Thu, 7 Jul 2022 03:00:10 -0700 Subject: [PATCH 3/3] missing await --- tests/async/test_browsertype_connect.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/async/test_browsertype_connect.py b/tests/async/test_browsertype_connect.py index 6d90509a5..95c650632 100644 --- a/tests/async/test_browsertype_connect.py +++ b/tests/async/test_browsertype_connect.py @@ -34,14 +34,14 @@ async def test_browser_type_connect_should_be_able_to_reconnect_to_a_browser( assert len(browser_context.pages) == 1 assert await page.evaluate("11 * 11") == 121 await page.goto(server.EMPTY_PAGE) - browser_context.close() + await browser_context.close() await browser.close() browser = await browser_type.connect(remote_server.ws_endpoint) browser_context = await browser.new_context() page = await browser_context.new_page() await page.goto(server.EMPTY_PAGE) - browser_context.close() + await browser_context.close() await browser.close()