chore: prevent save_as from hanging in sync mode (#1474)

Resolves #1462.

Docs: microsoft/playwright#16120

Author: rwoll

playwright/_impl/_video.py

@@ -55,6 +55,10 @@ async def path(self) -> pathlib.Path:
         return artifact.absolute_path
 
     async def save_as(self, path: Union[str, pathlib.Path]) -> None:
+        if self._page._connection._is_sync and not self._page._is_closed:
+            raise Error(
+                "Page is not yet closed. Close the page prior to calling save_as"
+            )
         artifact = await self._artifact_future
         if not artifact:
             raise Error("Page did not produce any video frames")

tests/async/test_video.py

@@ -49,3 +49,16 @@ async def test_short_video_should_throw_persistent_context(
 
     path = await page.video.path()
     assert str(tmpdir) in str(path)
+
+
+async def test_should_not_error_if_page_not_closed_before_save_as(
+    browser, tmpdir, server
+):
+    page = await browser.new_page(record_video_dir=tmpdir)
+    await page.goto(server.PREFIX + "/grid.html")
+    out_path = tmpdir / "some-video.webm"
+    saved = page.video.save_as(out_path)
+    await page.close()
+    await saved
+    await page.context.close()
+    assert os.path.exists(out_path)

tests/sync/test_video.py

@@ -16,7 +16,9 @@
 from pathlib import Path
 from typing import Dict
 
-from playwright.sync_api import Browser, BrowserType
+import pytest
+
+from playwright.sync_api import Browser, BrowserType, Error
 from tests.server import Server
 
 
@@ -91,3 +93,21 @@ def test_record_video_can_get_video_path_immediately(
     page.wait_for_timeout(1000)
     context.close()
     assert os.path.exists(path)
+
+
+def test_should_error_if_page_not_closed_before_save_as(
+    browser: Browser, tmpdir: Path, server: Server
+) -> None:
+    page = browser.new_page(record_video_dir=tmpdir)
+    page.goto(server.PREFIX + "/grid.html")
+    out_path = tmpdir / "some-video.webm"
+    with pytest.raises(Error) as err:
+        video = page.video
+        assert video
+        video.save_as(out_path)
+    assert "Page is not yet closed. Close the page prior to calling save_as" in str(err)
+    assert not os.path.exists(out_path)
+    page.context.close()
+
+    video.save_as(out_path)
+    assert os.path.exists(out_path)