tests: cleanup tests (microsoft#717)

mxschmitt · web-flow · commit 633d8815bd2f · 2021-05-24T18:36:37.000+02:00
diff --git a/playwright/_impl/_connection.py b/playwright/_impl/_connection.py
@@ -338,7 +338,3 @@ def serialize_call_stack(stack_trace: traceback.StackSummary) -> List[Dict]:
         )
     stack.reverse()
     return stack
-
-
-def capture_call_stack() -> List[Dict]:
-    return serialize_call_stack(traceback.extract_stack())
diff --git a/playwright/_impl/_tracing.py b/playwright/_impl/_tracing.py
@@ -19,7 +19,7 @@
 from playwright._impl._connection import from_channel
 from playwright._impl._helper import locals_to_params
 
-if TYPE_CHECKING:
+if TYPE_CHECKING:  # pragma: no cover
     from playwright._impl._browser_context import BrowserContext
 
 
diff --git a/tests/async/test_browsercontext.py b/tests/async/test_browsercontext.py
@@ -26,6 +26,7 @@ async def test_page_event_should_create_new_context(browser):
     assert context in browser.contexts
     await context.close()
     assert len(browser.contexts) == 0
+    assert context.browser == browser
 
 
 async def test_window_open_should_use_parent_tab_context(browser, server):
diff --git a/tests/async/test_browsertype_connect_cdp.py b/tests/async/test_browsertype_connect_cdp.py
@@ -18,7 +18,7 @@
 import requests
 
 from playwright.async_api import BrowserType
-from tests.server import Server, find_free_port, wait_for_port
+from tests.server import Server, find_free_port
 
 pytestmark = pytest.mark.only_browser("chromium")
 
@@ -30,7 +30,6 @@ async def test_connect_to_an_existing_cdp_session(
     browser_server = await browser_type.launch(
         **launch_arguments, args=[f"--remote-debugging-port={port}"]
     )
-    wait_for_port(port)
     cdp_browser = await browser_type.connect_over_cdp(f"http://localhost:{port}")
     assert len(cdp_browser.contexts) == 1
     await cdp_browser.close()
@@ -44,7 +43,6 @@ async def test_connect_to_an_existing_cdp_session_twice(
     browser_server = await browser_type.launch(
         **launch_arguments, args=[f"--remote-debugging-port={port}"]
     )
-    wait_for_port(port)
     endpoint_url = f"http://localhost:{port}"
     cdp_browser1 = await browser_type.connect_over_cdp(endpoint_url)
     cdp_browser2 = await browser_type.connect_over_cdp(endpoint_url)
@@ -78,7 +76,6 @@ async def test_conect_over_a_ws_endpoint(
     browser_server = await browser_type.launch(
         **launch_arguments, args=[f"--remote-debugging-port={port}"]
     )
-    wait_for_port(port)
     ws_endpoint = _ws_endpoint_from_url(f"http://localhost:{port}/json/version/")
 
     cdp_browser1 = await browser_type.connect_over_cdp(ws_endpoint)
diff --git a/tests/async/test_defaultbrowsercontext.py b/tests/async/test_defaultbrowsercontext.py
@@ -16,7 +16,6 @@
 import os
 
 import pytest
-from flaky import flaky
 
 from playwright._impl._api_types import Error
 
@@ -280,7 +279,6 @@ async def test_should_accept_user_data_dir(server, tmpdir, launch_persistent):
     assert len(os.listdir(tmpdir)) > 0
 
 
-@flaky
 async def test_should_restore_state_from_userDataDir(
     browser_type, launch_arguments, server, tmp_path_factory
 ):
diff --git a/tests/async/test_download.py b/tests/async/test_download.py
@@ -288,9 +288,6 @@ def handle_download(request):
 
 
 async def test_should_report_new_window_downloads(browser, server):
-    # TODO: - the test fails in headful Chromium as the popup page gets closed along
-    # with the session before download completed event arrives.
-    # - WebKit doesn't close the popup page
     page = await browser.new_page(accept_downloads=True)
     await page.set_content(
         f'<a target=_blank href="{server.PREFIX}/download">download</a>'
diff --git a/tests/async/test_frames.py b/tests/async/test_frames.py
@@ -20,6 +20,7 @@
 async def test_evaluate_handle(page, server):
     await page.goto(server.EMPTY_PAGE)
     main_frame = page.main_frame
+    assert main_frame.page == page
     window_handle = await main_frame.evaluate_handle("window")
     assert window_handle
 
diff --git a/tests/async/test_headful.py b/tests/async/test_headful.py
@@ -14,7 +14,6 @@
 
 
 import pytest
-from flaky import flaky
 
 
 async def test_should_have_default_url_when_launching_browser(
@@ -28,34 +27,6 @@ async def test_should_have_default_url_when_launching_browser(
     await browser_context.close()
 
 
-async def test_headless_should_be_able_to_read_cookies_written_by_headful(
-    browser_type, launch_arguments, server, tmpdir, is_chromium, is_win
-):
-    if is_chromium and is_win:
-        pytest.skip("see https://github.com/microsoft/playwright/issues/717")
-        return
-    # Write a cookie in headful chrome
-    headful_context = await browser_type.launch_persistent_context(
-        tmpdir, **{**launch_arguments, "headless": False}
-    )
-    headful_page = await headful_context.new_page()
-    await headful_page.goto(server.EMPTY_PAGE)
-    await headful_page.evaluate(
-        """() => document.cookie = 'foo=true; expires=Fri, 31 Dec 9999 23:59:59 GMT'"""
-    )
-    await headful_context.close()
-    # Read the cookie from headless chrome
-    headless_context = await browser_type.launch_persistent_context(
-        tmpdir, **{**launch_arguments, "headless": True}
-    )
-    headless_page = await headless_context.new_page()
-    await headless_page.goto(server.EMPTY_PAGE)
-    cookie = await headless_page.evaluate("() => document.cookie")
-    await headless_context.close()
-    # This might throw. See https://github.com/GoogleChrome/puppeteer/issues/2778
-    assert cookie == "foo=true"
-
-
 async def test_should_close_browser_with_beforeunload_page(
     browser_type, launch_arguments, server, tmpdir
 ):
@@ -177,7 +148,6 @@ async def test_should_not_override_viewport_size_when_passed_null(
     await browser.close()
 
 
-@flaky
 async def test_page_bring_to_front_should_work(browser_type, launch_arguments):
     browser = await browser_type.launch(**{**launch_arguments, "headless": False})
     page1 = await browser.new_page()
diff --git a/tests/async/test_network.py b/tests/async/test_network.py
@@ -171,6 +171,7 @@ async def test_request_headers_should_work(
         assert "WebKit" in response.request.headers["user-agent"]
 
 
+# TODO: update once fixed https://github.com/microsoft/playwright/issues/6690
 @pytest.mark.xfail
 async def test_request_headers_should_get_the_same_headers_as_the_server(
     page: Page, server, is_webkit, is_win
diff --git a/tests/async/test_page.py b/tests/async/test_page.py
@@ -128,9 +128,6 @@ async def test_async_stacks_should_work(page, server):
     assert __file__ in exc_info.value.stack
 
 
-# TODO: bring in page.crash events
-
-
 async def test_opener_should_provide_access_to_the_opener_page(page):
     async with page.expect_popup() as popup_info:
         await page.evaluate("window.open('about:blank')"),
@@ -766,7 +763,6 @@ async def test_select_option_should_select_only_first_option(page, server):
     assert await page.evaluate("result.onChange") == ["blue"]
 
 
-@pytest.mark.skip_browser("webkit")  # TODO: investigate
 async def test_select_option_should_not_throw_when_select_causes_navigation(
     page, server
 ):
diff --git a/tests/async/test_tracing.py b/tests/async/test_tracing.py
@@ -28,6 +28,7 @@ async def test_browser_context_output_trace(
     page = await context.new_page()
     await page.goto(server.PREFIX + "/grid.html")
     await context.tracing.stop()
+    await page.wait_for_timeout(1000)
     await context.tracing.export(Path(tmp_path / "traces" / "trace.zip").resolve())
     assert Path(tmp_path / "traces" / "trace.zip").exists()
     assert Path(tmp_path / "traces" / "resources").exists()
diff --git a/tests/async/test_video.py b/tests/async/test_video.py
@@ -28,6 +28,7 @@ async def test_short_video_should_throw(browser, tmpdir, server):
     await page.goto(server.PREFIX + "/grid.html")
     path = await page.video.path()
     assert str(tmpdir) in str(path)
+    await page.wait_for_timeout(1000)
     await page.context.close()
     assert os.path.exists(path)
 
@@ -43,6 +44,7 @@ async def test_short_video_should_throw_persistent_context(
     )
     page = context.pages[0]
     await page.goto(server.PREFIX + "/grid.html")
+    await page.wait_for_timeout(1000)
     await context.close()
 
     path = await page.video.path()
diff --git a/tests/async/test_worker.py b/tests/async/test_worker.py
@@ -16,6 +16,7 @@
 from asyncio.futures import Future
 
 import pytest
+from flaky import flaky
 
 from playwright.async_api import Error, Page, Worker
 
@@ -99,6 +100,7 @@ async def test_workers_should_report_errors(page):
     assert "this is my error" in error_log.message
 
 
+@flaky  # Upstream flaky
 async def test_workers_should_clear_upon_navigation(server, page):
     await page.goto(server.EMPTY_PAGE)
     async with page.expect_event("worker") as event_info:
@@ -114,6 +116,7 @@ async def test_workers_should_clear_upon_navigation(server, page):
     assert len(page.workers) == 0
 
 
+@flaky  # Upstream flaky
 async def test_workers_should_clear_upon_cross_process_navigation(server, page):
     await page.goto(server.EMPTY_PAGE)
     async with page.expect_event("worker") as event_info:
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -239,7 +239,11 @@ def kill(self):
         if self.process.poll() is not None:
             return
         if sys.platform == "win32":
-            subprocess.check_call(["taskkill", "/F", "/PID", str(self.process.pid)])
+            subprocess.check_call(
+                ["taskkill", "/F", "/PID", str(self.process.pid)],
+                stderr=subprocess.DEVNULL,
+                stdout=subprocess.DEVNULL,
+            )
         else:
             self.process.kill()
         self.process.wait()
diff --git a/tests/server.py b/tests/server.py
@@ -18,7 +18,6 @@
 import mimetypes
 import socket
 import threading
-import time
 from contextlib import closing
 from http import HTTPStatus
 from urllib.parse import urlparse
@@ -40,30 +39,6 @@ def find_free_port():
         return s.getsockname()[1]
 
 
-def wait_for_port(port, host="localhost", timeout=5.0):
-    """Wait until a port starts accepting TCP connections.
-    Args:
-        port (int): Port number.
-        host (str): Host address on which the port should exist.
-        timeout (float): In seconds. How long to wait before raising errors.
-    Raises:
-        TimeoutError: The port isn't accepting connection after time specified in `timeout`.
-    Reference: https://gist.github.com/butla/2d9a4c0f35ea47b7452156c96a4e7b12
-    """
-    start_time = time.perf_counter()
-    while True:
-        try:
-            with socket.create_connection((host, port), timeout=timeout):
-                break
-        except OSError as ex:
-            time.sleep(0.01)
-            if time.perf_counter() - start_time >= timeout:
-                raise TimeoutError(
-                    "Waited too long for the port {} on host {} to start accepting "
-                    "connections.".format(port, host)
-                ) from ex
-
-
 class Server:
     protocol = "http"
 
@@ -135,9 +110,7 @@ def process(self):
                     return
                 file_content = None
                 try:
-                    file_content = (
-                        static_path / request.path.decode()[1:]
-                    ).read_bytes()
+                    file_content = (static_path / path[1:]).read_bytes()
                     request.setHeader(b"Content-Type", mimetypes.guess_type(path)[0])
                     request.setHeader(b"Cache-Control", "no-cache, no-store")
                     if path in gzip_routes:
diff --git a/tests/sync/test_browsertype_connect_cdp.py b/tests/sync/test_browsertype_connect_cdp.py
@@ -17,7 +17,7 @@
 import pytest
 
 from playwright.sync_api import BrowserType
-from tests.server import find_free_port, wait_for_port
+from tests.server import find_free_port
 
 pytestmark = pytest.mark.only_browser("chromium")
 
@@ -29,7 +29,6 @@ def test_connect_to_an_existing_cdp_session(
     browser_server = browser_type.launch(
         **launch_arguments, args=[f"--remote-debugging-port={port}"]
     )
-    wait_for_port(port)
     cdp_browser = browser_type.connect_over_cdp(f"http://localhost:{port}")
     assert len(cdp_browser.contexts) == 1
     cdp_browser.close()
diff --git a/tests/sync/test_video.py b/tests/sync/test_video.py
@@ -23,6 +23,7 @@ def test_should_expose_video_path(browser, tmpdir, server):
     path = page.video.path()
     assert repr(page.video) == f"<Video page={page}>"
     assert str(tmpdir) in str(path)
+    page.wait_for_timeout(1000)
     page.context.close()
 
 
@@ -31,6 +32,7 @@ def test_video_should_exist(browser, tmpdir, server):
     page.goto(server.PREFIX + "/grid.html")
     path = page.video.path()
     assert str(tmpdir) in str(path)
+    page.wait_for_timeout(1000)
     page.context.close()
     assert os.path.exists(path)
 
@@ -40,6 +42,7 @@ def test_record_video_to_path(browser, tmpdir, server):
     page.goto(server.PREFIX + "/grid.html")
     path = page.video.path()
     assert str(tmpdir) in str(path)
+    page.wait_for_timeout(1000)
     page.context.close()
     assert os.path.exists(path)
 
@@ -54,6 +57,7 @@ def test_record_video_to_path_persistent(
     page.goto(server.PREFIX + "/grid.html")
     path = page.video.path()
     assert str(tmpdir) in str(path)
+    page.wait_for_timeout(1000)
     context.close()
     assert os.path.exists(path)
 
@@ -67,5 +71,6 @@ def test_record_video_can_get_video_path_immediately(
     page = context.pages[0]
     path = page.video.path()
     assert str(tmpdir) in str(path)
+    page.wait_for_timeout(1000)
     context.close()
     assert os.path.exists(path)

Original file line number	Diff line number	Diff line change
`@@ -338,7 +338,3 @@ def serialize_call_stack(stack_trace: traceback.StackSummary) -> List[Dict]:`
`338`	`338`	`)`
`339`	`339`	`stack.reverse()`
`340`	`340`	`return stack`
`341`		`-`
`342`		`-`
`343`		`-def capture_call_stack() -> List[Dict]:`
`344`		`- return serialize_call_stack(traceback.extract_stack())`