From 356b8f596bcfdddabddea8171bc1ed17bbb46bb8 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Tue, 12 Apr 2022 10:36:18 -0700 Subject: [PATCH 01/13] chore: roll Playwright to 1.21.0 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index d598e5d69..5d8e764b5 100644 --- a/setup.py +++ b/setup.py @@ -30,7 +30,7 @@ InWheel = None from wheel.bdist_wheel import bdist_wheel as BDistWheelCommand -driver_version = "1.21.0-beta-1649712128000" +driver_version = "1.21.0" def extractall(zip: zipfile.ZipFile, path: str) -> None: From f08111f5107c20970e9a0e5c3f25968f1415f6e0 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Tue, 12 Apr 2022 12:29:59 -0700 Subject: [PATCH 02/13] wip: support large file uploads --- playwright/_impl/_element_handle.py | 17 +++- playwright/_impl/_file_chooser.py | 28 ------- playwright/_impl/_frame.py | 14 +++- playwright/_impl/_object_factory.py | 9 +++ playwright/_impl/_page.py | 1 + playwright/_impl/_set_input_files_helpers.py | 81 ++++++++++++++++++++ playwright/_impl/_writeable_stream.py | 32 ++++++++ tests/assets/input/fileupload.html | 4 +- tests/async/test_input.py | 42 ++++++++++ 9 files changed, 192 insertions(+), 36 deletions(-) create mode 100644 playwright/_impl/_set_input_files_helpers.py create mode 100644 playwright/_impl/_writeable_stream.py diff --git a/playwright/_impl/_element_handle.py b/playwright/_impl/_element_handle.py index 23aec8994..8014890c7 100644 --- a/playwright/_impl/_element_handle.py +++ b/playwright/_impl/_element_handle.py @@ -19,8 +19,8 @@ from playwright._impl._api_structures import FilePayload, FloatRect, Position from playwright._impl._connection import ChannelOwner, from_nullable_channel -from playwright._impl._file_chooser import normalize_file_payloads from playwright._impl._helper import ( + Error, KeyboardModifier, MouseButton, async_writefile, @@ -33,6 +33,7 @@ parse_result, serialize_argument, ) +from playwright._impl._set_input_files_helpers import convert_input_files if sys.version_info >= (3, 8): # pragma: no cover from typing import Literal @@ -190,8 +191,18 @@ async def set_input_files( noWaitAfter: bool = None, ) -> None: params = locals_to_params(locals()) - params["files"] = await normalize_file_payloads(files) - await self._channel.send("setInputFiles", params) + frame = await self.owner_frame() + if not frame: + raise Error("Cannot set input files to detached element") + converted = await convert_input_files(files, frame.page.context) + if converted.get("files") is not None: + params["files"] = converted.get("files") + await self._channel.send("setInputFiles", params) + else: + await self._channel.send( + "setInputFilePaths", + locals_to_params({**params, **converted, "files": None}), + ) async def focus(self) -> None: await self._channel.send("focus") diff --git a/playwright/_impl/_file_chooser.py b/playwright/_impl/_file_chooser.py index 7add73e07..a15050fc0 100644 --- a/playwright/_impl/_file_chooser.py +++ b/playwright/_impl/_file_chooser.py @@ -12,13 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -import base64 -import os from pathlib import Path from typing import TYPE_CHECKING, List, Union from playwright._impl._api_structures import FilePayload -from playwright._impl._helper import async_readfile if TYPE_CHECKING: # pragma: no cover from playwright._impl._element_handle import ElementHandle @@ -56,28 +53,3 @@ async def set_files( noWaitAfter: bool = None, ) -> None: await self._element_handle.set_input_files(files, timeout, noWaitAfter) - - -async def normalize_file_payloads( - files: Union[str, Path, FilePayload, List[Union[str, Path]], List[FilePayload]] -) -> List: - file_list = files if isinstance(files, list) else [files] - file_payloads: List = [] - for item in file_list: - if isinstance(item, (str, Path)): - file_payloads.append( - { - "name": os.path.basename(item), - "buffer": base64.b64encode(await async_readfile(item)).decode(), - } - ) - else: - file_payloads.append( - { - "name": item["name"], - "mimeType": item["mimeType"], - "buffer": base64.b64encode(item["buffer"]).decode(), - } - ) - - return file_payloads diff --git a/playwright/_impl/_frame.py b/playwright/_impl/_frame.py index 9d6b272ce..99541ef1c 100644 --- a/playwright/_impl/_frame.py +++ b/playwright/_impl/_frame.py @@ -18,6 +18,7 @@ from typing import TYPE_CHECKING, Any, Dict, List, Optional, Pattern, Set, Union, cast from pyee import EventEmitter +from requests import delete from playwright._impl._api_structures import FilePayload, Position from playwright._impl._api_types import Error @@ -28,7 +29,6 @@ ) from playwright._impl._element_handle import ElementHandle, convert_select_option_values from playwright._impl._event_context_manager import EventContextManagerImpl -from playwright._impl._file_chooser import normalize_file_payloads from playwright._impl._helper import ( DocumentLoadState, FrameNavigatedEvent, @@ -48,6 +48,7 @@ ) from playwright._impl._locator import FrameLocator, Locator from playwright._impl._network import Response +from playwright._impl._set_input_files_helpers import convert_input_files from playwright._impl._wait_helper import WaitHelper if sys.version_info >= (3, 8): # pragma: no cover @@ -598,8 +599,15 @@ async def set_input_files( noWaitAfter: bool = None, ) -> None: params = locals_to_params(locals()) - params["files"] = await normalize_file_payloads(files) - await self._channel.send("setInputFiles", params) + converted = await convert_input_files(files, self.page.context) + if converted.get("files") is not None: + params["files"] = converted.get("files") + await self._channel.send("setInputFiles", params) + else: + await self._channel.send( + "setInputFilePaths", + locals_to_params({**params, **converted, "files": None}), + ) async def type( self, diff --git a/playwright/_impl/_object_factory.py b/playwright/_impl/_object_factory.py index a926d71ca..de95b0625 100644 --- a/playwright/_impl/_object_factory.py +++ b/playwright/_impl/_object_factory.py @@ -33,6 +33,7 @@ from playwright._impl._selectors import SelectorsOwner from playwright._impl._stream import Stream from playwright._impl._tracing import Tracing +from playwright._impl._writeable_stream import WriteableStream class DummyObject(ChannelOwner): @@ -89,6 +90,14 @@ def create_remote_object( return WebSocket(parent, type, guid, initializer) if type == "Worker": return Worker(parent, type, guid, initializer) + if type == "WriteableStream": + print("WRITEABLE STREAM") + print("WRITEABLE STREAM") + print("WRITEABLE STREAM") + print("WRITEABLE STREAM") + print("WRITEABLE STREAM") + print("WRITEABLE STREAM") + return WriteableStream(parent, type, guid, initializer) if type == "Selectors": return SelectorsOwner(parent, type, guid, initializer) return DummyObject(parent, type, guid, initializer) diff --git a/playwright/_impl/_page.py b/playwright/_impl/_page.py index 98916923a..5867c8b25 100644 --- a/playwright/_impl/_page.py +++ b/playwright/_impl/_page.py @@ -83,6 +83,7 @@ serialize_argument, ) from playwright._impl._network import Request, Response, Route, serialize_headers +from playwright._impl._set_input_files_helpers import convert_input_files from playwright._impl._video import Video from playwright._impl._wait_helper import WaitHelper diff --git a/playwright/_impl/_set_input_files_helpers.py b/playwright/_impl/_set_input_files_helpers.py new file mode 100644 index 000000000..0c11fecd4 --- /dev/null +++ b/playwright/_impl/_set_input_files_helpers.py @@ -0,0 +1,81 @@ +import base64 +import os +import shutil +from pathlib import Path +from posixpath import basename +from typing import TYPE_CHECKING, List, Union + +from playwright._impl._helper import Error, async_readfile +from playwright._impl._writeable_stream import WriteableStream + +if TYPE_CHECKING: # pragma: no cover + from playwright._impl._browser_context import BrowserContext + +from playwright._impl._api_structures import FilePayload + +SIZE_LIMIT_IN_BYTES = 50 * 1024 * 1024 + + +async def convert_input_files( + files: Union[str, Path, FilePayload, List[Union[str, Path]], List[FilePayload]], + context: "BrowserContext", +): + files = files if isinstance(files, list) else [files] + + has_large_buffer = list( + filter( + lambda f: not isinstance(f, (str, Path)) + and len(f.get("buffer", "")) > SIZE_LIMIT_IN_BYTES, + files, + ) + ) + if has_large_buffer: + raise Error( + "Cannot set buffer larger than 50Mb, please write it to a file and pass its path instead." + ) + + has_large_file = list( + map( + lambda f: os.stat(f).st_size, + filter(lambda f: isinstance(f, (str, Path)), files), + ) + ) + if has_large_file: + if context._channel._connection.is_remote: + streams = [] + for file in files: + stream = await context._channel.send( + "createTempFile", {"name": os.path.basename(file)} + ) + with open(file, "rb") as f: + shutil.copyfileobj(f, stream) + streams.append(stream) + return {"streams": streams} + return {"localPaths": list(map(lambda f: Path(f).absolute().resolve(), files))} + + return {"files": await normalize_file_payloads(files)} + + +async def normalize_file_payloads( + files: Union[str, Path, FilePayload, List[Union[str, Path]], List[FilePayload]] +) -> List: + file_list = files if isinstance(files, list) else [files] + file_payloads: List = [] + for item in file_list: + if isinstance(item, (str, Path)): + file_payloads.append( + { + "name": os.path.basename(item), + "buffer": base64.b64encode(await async_readfile(item)).decode(), + } + ) + else: + file_payloads.append( + { + "name": item["name"], + "mimeType": item["mimeType"], + "buffer": base64.b64encode(item["buffer"]).decode(), + } + ) + + return file_payloads diff --git a/playwright/_impl/_writeable_stream.py b/playwright/_impl/_writeable_stream.py new file mode 100644 index 000000000..3cb17ec84 --- /dev/null +++ b/playwright/_impl/_writeable_stream.py @@ -0,0 +1,32 @@ +# Copyright (c) Microsoft Corporation. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import base64 +from typing import Dict + +from playwright._impl._connection import ChannelOwner + + +class WriteableStream(ChannelOwner): + def __init__( + self, parent: ChannelOwner, type: str, guid: str, initializer: Dict + ) -> None: + super().__init__(parent, type, guid, initializer) + + # FIXME: These need to be async and await the channel send + def write(self, chunk: bytes): + self._channel.send("write", {"binary": base64.b64encode(chunk)}) + + def close(self): + pass diff --git a/tests/assets/input/fileupload.html b/tests/assets/input/fileupload.html index 57e90d50a..771dc4c68 100644 --- a/tests/assets/input/fileupload.html +++ b/tests/assets/input/fileupload.html @@ -4,8 +4,8 @@ File upload test -
- + +
diff --git a/tests/async/test_input.py b/tests/async/test_input.py index 3ff92ebe6..aaa62daac 100644 --- a/tests/async/test_input.py +++ b/tests/async/test_input.py @@ -14,6 +14,8 @@ import asyncio import os +from pprint import pprint +from shutil import ExecError import pytest @@ -284,3 +286,43 @@ async def _listen_for_wheel_events(page: Page, selector: str) -> None: """, selector, ) + + +# FIXME: skip if not CR, mark slow +async def test_should_upload_large_file(page, server, tmp_path): + await page.goto(server.PREFIX + "/input/fileupload.html") + large_file_path = tmp_path / "200MB.zip" + with large_file_path.open("w") as f: + data = "A" * 1024 + for i in range(0, 5 * 1024): + f.write(data) + input = page.locator('input[type="file"]') + events = await input.evaluate_handle( + """ + e => { + const events = []; + e.addEventListener('input', () => events.push('input')); + e.addEventListener('change', () => events.push('change')); + return events; + } + """ + ) + + await input.set_input_files(large_file_path) + assert await input.evaluate("e => e.files[0].name") == "200MB.zip" + assert await events.evaluate("e => e") == ["input", "change"] + + server_request = None + + def handler(request): + server_request = request + request.finish() + + server.set_route("/upload", handler) + + await page.click("input[type=submit]") + + # FIXME: add assertions + # expect(file1.originalFilename).toBe('200MB.zip'); + # expect(file1.size).toBe(200 * 1024 * 1024); + # await Promise.all([uploadFile, file1.filepath].map(fs.promises.unlink)); From 9c2aa044bf8b9193882c7220960a849d344e8434 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Tue, 12 Apr 2022 15:58:26 -0700 Subject: [PATCH 03/13] remove debug code --- playwright/_impl/_frame.py | 1 - playwright/_impl/_object_factory.py | 6 ------ 2 files changed, 7 deletions(-) diff --git a/playwright/_impl/_frame.py b/playwright/_impl/_frame.py index 99541ef1c..6a9820911 100644 --- a/playwright/_impl/_frame.py +++ b/playwright/_impl/_frame.py @@ -18,7 +18,6 @@ from typing import TYPE_CHECKING, Any, Dict, List, Optional, Pattern, Set, Union, cast from pyee import EventEmitter -from requests import delete from playwright._impl._api_structures import FilePayload, Position from playwright._impl._api_types import Error diff --git a/playwright/_impl/_object_factory.py b/playwright/_impl/_object_factory.py index de95b0625..53987efd4 100644 --- a/playwright/_impl/_object_factory.py +++ b/playwright/_impl/_object_factory.py @@ -91,12 +91,6 @@ def create_remote_object( if type == "Worker": return Worker(parent, type, guid, initializer) if type == "WriteableStream": - print("WRITEABLE STREAM") - print("WRITEABLE STREAM") - print("WRITEABLE STREAM") - print("WRITEABLE STREAM") - print("WRITEABLE STREAM") - print("WRITEABLE STREAM") return WriteableStream(parent, type, guid, initializer) if type == "Selectors": return SelectorsOwner(parent, type, guid, initializer) From 6df445f40c4e200b998e59d2d77ce945b498bc70 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Tue, 12 Apr 2022 21:55:18 -0700 Subject: [PATCH 04/13] add test, fix WriteableStream --- playwright/_impl/_set_input_files_helpers.py | 5 ++-- playwright/_impl/_writeable_stream.py | 24 ++++++++++++++---- tests/async/test_input.py | 26 +++++++++++--------- 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/playwright/_impl/_set_input_files_helpers.py b/playwright/_impl/_set_input_files_helpers.py index 0c11fecd4..748aa4152 100644 --- a/playwright/_impl/_set_input_files_helpers.py +++ b/playwright/_impl/_set_input_files_helpers.py @@ -44,11 +44,10 @@ async def convert_input_files( if context._channel._connection.is_remote: streams = [] for file in files: - stream = await context._channel.send( + stream: WriteableStream = await context._channel.send( "createTempFile", {"name": os.path.basename(file)} ) - with open(file, "rb") as f: - shutil.copyfileobj(f, stream) + await WriteableStream.copy(file, stream) streams.append(stream) return {"streams": streams} return {"localPaths": list(map(lambda f: Path(f).absolute().resolve(), files))} diff --git a/playwright/_impl/_writeable_stream.py b/playwright/_impl/_writeable_stream.py index 3cb17ec84..a14384a7f 100644 --- a/playwright/_impl/_writeable_stream.py +++ b/playwright/_impl/_writeable_stream.py @@ -13,8 +13,13 @@ # limitations under the License. import base64 +import os from typing import Dict +# COPY_BUFSIZE is taken from shutil.py in the standard library +_WINDOWS = os.name == 'nt' +COPY_BUFSIZE = 1024 * 1024 if _WINDOWS else 64 * 1024 + from playwright._impl._connection import ChannelOwner @@ -24,9 +29,18 @@ def __init__( ) -> None: super().__init__(parent, type, guid, initializer) - # FIXME: These need to be async and await the channel send - def write(self, chunk: bytes): - self._channel.send("write", {"binary": base64.b64encode(chunk)}) + @staticmethod + async def copy(path: os.PathLike, stream: 'WriteableStream'): + with open(path, 'rb') as f: + while True: + data = f.read(COPY_BUFSIZE) + if not data: + break + await stream.write(data) + await stream.close() + + async def write(self, chunk: bytes): + await self._channel.send("write", {"binary": base64.b64encode(chunk)}) - def close(self): - pass + async def close(self): + await self._channel.send('close') diff --git a/tests/async/test_input.py b/tests/async/test_input.py index aaa62daac..fb829a2de 100644 --- a/tests/async/test_input.py +++ b/tests/async/test_input.py @@ -16,7 +16,7 @@ import os from pprint import pprint from shutil import ExecError - +import re import pytest from playwright._impl._path_utils import get_file_dirname @@ -292,9 +292,9 @@ async def _listen_for_wheel_events(page: Page, selector: str) -> None: async def test_should_upload_large_file(page, server, tmp_path): await page.goto(server.PREFIX + "/input/fileupload.html") large_file_path = tmp_path / "200MB.zip" - with large_file_path.open("w") as f: - data = "A" * 1024 - for i in range(0, 5 * 1024): + data = b'A' * 1024 + with large_file_path.open("wb") as f: + for i in range(0, 200 * 1024 * 1024, len(data)): f.write(data) input = page.locator('input[type="file"]') events = await input.evaluate_handle( @@ -312,17 +312,19 @@ async def test_should_upload_large_file(page, server, tmp_path): assert await input.evaluate("e => e.files[0].name") == "200MB.zip" assert await events.evaluate("e => e") == ["input", "change"] - server_request = None - + file_upload = asyncio.Future() def handler(request): - server_request = request + file_upload.set_result(request) request.finish() server.set_route("/upload", handler) await page.click("input[type=submit]") - - # FIXME: add assertions - # expect(file1.originalFilename).toBe('200MB.zip'); - # expect(file1.size).toBe(200 * 1024 * 1024); - # await Promise.all([uploadFile, file1.filepath].map(fs.promises.unlink)); + request = await file_upload + contents = request.args[b'file1'][0] + assert len(contents) == 200 * 1024 * 1024 + assert contents[:1024] == data + assert contents[len(contents)-1024:] == data + match = re.search(rb'^.*Content-Disposition: form-data; name="(?P.*)"; filename="(?P.*)".*$', request.post_body, re.MULTILINE) + assert match.group('name') == b'file1' + assert match.group('filename') == b'200MB.zip' From 6007f810b71a44ecfa43804c3e52a89aba947fed Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Tue, 12 Apr 2022 22:06:53 -0700 Subject: [PATCH 05/13] add remote test --- playwright/_impl/_writeable_stream.py | 10 +---- tests/async/test_browsertype_connect.py | 53 +++++++++++++++++++++++++ tests/async/test_input.py | 1 - 3 files changed, 55 insertions(+), 9 deletions(-) diff --git a/playwright/_impl/_writeable_stream.py b/playwright/_impl/_writeable_stream.py index a14384a7f..45a248fca 100644 --- a/playwright/_impl/_writeable_stream.py +++ b/playwright/_impl/_writeable_stream.py @@ -36,11 +36,5 @@ async def copy(path: os.PathLike, stream: 'WriteableStream'): data = f.read(COPY_BUFSIZE) if not data: break - await stream.write(data) - await stream.close() - - async def write(self, chunk: bytes): - await self._channel.send("write", {"binary": base64.b64encode(chunk)}) - - async def close(self): - await self._channel.send('close') + await stream.send("write", {"binary": base64.b64encode(data).decode()}) + await stream.send('close') diff --git a/tests/async/test_browsertype_connect.py b/tests/async/test_browsertype_connect.py index 73ab81c0b..9efc402e1 100644 --- a/tests/async/test_browsertype_connect.py +++ b/tests/async/test_browsertype_connect.py @@ -14,6 +14,8 @@ from typing import Callable +import asyncio +import re import pytest from playwright.async_api import BrowserType, Error, Playwright, Route @@ -241,3 +243,54 @@ async def handle_request(route: Route) -> None: assert await response.json() == {"foo": "bar"} remote.kill() + +# FIXME: skip if not CR, mark slow +async def test_should_upload_large_file( + browser_type: BrowserType, + launch_server: Callable[[], RemoteServer], + playwright: Playwright, + server: Server, + tmp_path): + remote = launch_server() + + browser = await browser_type.connect(remote.ws_endpoint) + context = await browser.new_context() + page = await context.new_page() + + await page.goto(server.PREFIX + "/input/fileupload.html") + large_file_path = tmp_path / "200MB.zip" + data = b'A' * 1024 + with large_file_path.open("wb") as f: + for i in range(0, 200 * 1024 * 1024, len(data)): + f.write(data) + input = page.locator('input[type="file"]') + events = await input.evaluate_handle( + """ + e => { + const events = []; + e.addEventListener('input', () => events.push('input')); + e.addEventListener('change', () => events.push('change')); + return events; + } + """ + ) + + await input.set_input_files(large_file_path) + assert await input.evaluate("e => e.files[0].name") == "200MB.zip" + assert await events.evaluate("e => e") == ["input", "change"] + + file_upload = asyncio.Future() + def handler(request): + file_upload.set_result(request) + request.finish() + server.set_route("/upload", handler) + + await page.click("input[type=submit]") + request = await file_upload + contents = request.args[b'file1'][0] + assert len(contents) == 200 * 1024 * 1024 + assert contents[:1024] == data + assert contents[len(contents)-1024:] == data + match = re.search(rb'^.*Content-Disposition: form-data; name="(?P.*)"; filename="(?P.*)".*$', request.post_body, re.MULTILINE) + assert match.group('name') == b'file1' + assert match.group('filename') == b'200MB.zip' diff --git a/tests/async/test_input.py b/tests/async/test_input.py index fb829a2de..f309130b9 100644 --- a/tests/async/test_input.py +++ b/tests/async/test_input.py @@ -316,7 +316,6 @@ async def test_should_upload_large_file(page, server, tmp_path): def handler(request): file_upload.set_result(request) request.finish() - server.set_route("/upload", handler) await page.click("input[type=submit]") From b2324480be5436d986310f4099d2f17a677a4143 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Tue, 12 Apr 2022 22:13:40 -0700 Subject: [PATCH 06/13] s/Writeable/Writable --- playwright/_impl/_object_factory.py | 6 +-- playwright/_impl/_set_input_files_helpers.py | 6 +-- playwright/_impl/_writeable_stream.py | 40 -------------------- 3 files changed, 6 insertions(+), 46 deletions(-) delete mode 100644 playwright/_impl/_writeable_stream.py diff --git a/playwright/_impl/_object_factory.py b/playwright/_impl/_object_factory.py index 53987efd4..e04d1adec 100644 --- a/playwright/_impl/_object_factory.py +++ b/playwright/_impl/_object_factory.py @@ -33,7 +33,7 @@ from playwright._impl._selectors import SelectorsOwner from playwright._impl._stream import Stream from playwright._impl._tracing import Tracing -from playwright._impl._writeable_stream import WriteableStream +from playwright._impl._writable_stream import WritableStream class DummyObject(ChannelOwner): @@ -90,8 +90,8 @@ def create_remote_object( return WebSocket(parent, type, guid, initializer) if type == "Worker": return Worker(parent, type, guid, initializer) - if type == "WriteableStream": - return WriteableStream(parent, type, guid, initializer) + if type == "WritableStream": + return WritableStream(parent, type, guid, initializer) if type == "Selectors": return SelectorsOwner(parent, type, guid, initializer) return DummyObject(parent, type, guid, initializer) diff --git a/playwright/_impl/_set_input_files_helpers.py b/playwright/_impl/_set_input_files_helpers.py index 748aa4152..9309e2d7b 100644 --- a/playwright/_impl/_set_input_files_helpers.py +++ b/playwright/_impl/_set_input_files_helpers.py @@ -6,7 +6,7 @@ from typing import TYPE_CHECKING, List, Union from playwright._impl._helper import Error, async_readfile -from playwright._impl._writeable_stream import WriteableStream +from playwright._impl._writable_stream import WritableStream if TYPE_CHECKING: # pragma: no cover from playwright._impl._browser_context import BrowserContext @@ -44,10 +44,10 @@ async def convert_input_files( if context._channel._connection.is_remote: streams = [] for file in files: - stream: WriteableStream = await context._channel.send( + stream: WritableStream = await context._channel.send( "createTempFile", {"name": os.path.basename(file)} ) - await WriteableStream.copy(file, stream) + await WritableStream.copy(file, stream) streams.append(stream) return {"streams": streams} return {"localPaths": list(map(lambda f: Path(f).absolute().resolve(), files))} diff --git a/playwright/_impl/_writeable_stream.py b/playwright/_impl/_writeable_stream.py deleted file mode 100644 index 45a248fca..000000000 --- a/playwright/_impl/_writeable_stream.py +++ /dev/null @@ -1,40 +0,0 @@ -# Copyright (c) Microsoft Corporation. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import base64 -import os -from typing import Dict - -# COPY_BUFSIZE is taken from shutil.py in the standard library -_WINDOWS = os.name == 'nt' -COPY_BUFSIZE = 1024 * 1024 if _WINDOWS else 64 * 1024 - -from playwright._impl._connection import ChannelOwner - - -class WriteableStream(ChannelOwner): - def __init__( - self, parent: ChannelOwner, type: str, guid: str, initializer: Dict - ) -> None: - super().__init__(parent, type, guid, initializer) - - @staticmethod - async def copy(path: os.PathLike, stream: 'WriteableStream'): - with open(path, 'rb') as f: - while True: - data = f.read(COPY_BUFSIZE) - if not data: - break - await stream.send("write", {"binary": base64.b64encode(data).decode()}) - await stream.send('close') From 8015d37529e0951ad4f5ddb1002fd760c8da6056 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Tue, 12 Apr 2022 22:22:27 -0700 Subject: [PATCH 07/13] mark tests CR only --- tests/async/test_browsertype_connect.py | 2 +- tests/async/test_input.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/async/test_browsertype_connect.py b/tests/async/test_browsertype_connect.py index 9efc402e1..86aeccd7c 100644 --- a/tests/async/test_browsertype_connect.py +++ b/tests/async/test_browsertype_connect.py @@ -244,7 +244,7 @@ async def handle_request(route: Route) -> None: remote.kill() -# FIXME: skip if not CR, mark slow +@pytest.mark.only_browser("chromium") async def test_should_upload_large_file( browser_type: BrowserType, launch_server: Callable[[], RemoteServer], diff --git a/tests/async/test_input.py b/tests/async/test_input.py index f309130b9..a0d33edb6 100644 --- a/tests/async/test_input.py +++ b/tests/async/test_input.py @@ -288,7 +288,7 @@ async def _listen_for_wheel_events(page: Page, selector: str) -> None: ) -# FIXME: skip if not CR, mark slow +@pytest.mark.only_browser("chromium") async def test_should_upload_large_file(page, server, tmp_path): await page.goto(server.PREFIX + "/input/fileupload.html") large_file_path = tmp_path / "200MB.zip" From 60c3fdcfcc59e03856d8bfc17a71524b2791d014 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Tue, 12 Apr 2022 22:25:34 -0700 Subject: [PATCH 08/13] add WritableStream MVP --- playwright/_impl/_writable_stream.py | 40 ++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 playwright/_impl/_writable_stream.py diff --git a/playwright/_impl/_writable_stream.py b/playwright/_impl/_writable_stream.py new file mode 100644 index 000000000..b18e1b3ad --- /dev/null +++ b/playwright/_impl/_writable_stream.py @@ -0,0 +1,40 @@ +# Copyright (c) Microsoft Corporation. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import base64 +import os +from typing import Dict + +# COPY_BUFSIZE is taken from shutil.py in the standard library +_WINDOWS = os.name == 'nt' +COPY_BUFSIZE = 1024 * 1024 if _WINDOWS else 64 * 1024 + +from playwright._impl._connection import ChannelOwner + + +class WritableStream(ChannelOwner): + def __init__( + self, parent: ChannelOwner, type: str, guid: str, initializer: Dict + ) -> None: + super().__init__(parent, type, guid, initializer) + + @staticmethod + async def copy(path: os.PathLike, stream: 'WritableStream'): + with open(path, 'rb') as f: + while True: + data = f.read(COPY_BUFSIZE) + if not data: + break + await stream.send("write", {"binary": base64.b64encode(data).decode()}) + await stream.send('close') From c0aebdb89ed80066d7a597e2b3a7dd482a228f39 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Wed, 13 Apr 2022 10:20:51 -0700 Subject: [PATCH 09/13] address review feedback --- playwright/_impl/_element_handle.py | 7 ++- playwright/_impl/_frame.py | 7 ++- playwright/_impl/_page.py | 1 - playwright/_impl/_set_input_files_helpers.py | 59 +++++++++++++------- playwright/_impl/_writable_stream.py | 15 ++--- tests/async/test_browsertype_connect.py | 36 ++++++------ tests/async/test_input.py | 31 +++++----- 7 files changed, 90 insertions(+), 66 deletions(-) diff --git a/playwright/_impl/_element_handle.py b/playwright/_impl/_element_handle.py index 8014890c7..ee0e73bb5 100644 --- a/playwright/_impl/_element_handle.py +++ b/playwright/_impl/_element_handle.py @@ -195,9 +195,10 @@ async def set_input_files( if not frame: raise Error("Cannot set input files to detached element") converted = await convert_input_files(files, frame.page.context) - if converted.get("files") is not None: - params["files"] = converted.get("files") - await self._channel.send("setInputFiles", params) + if converted["files"]: + await self._channel.send( + "setInputFiles", {**params, "files": converted["files"]} + ) else: await self._channel.send( "setInputFilePaths", diff --git a/playwright/_impl/_frame.py b/playwright/_impl/_frame.py index 6a9820911..db798071d 100644 --- a/playwright/_impl/_frame.py +++ b/playwright/_impl/_frame.py @@ -599,9 +599,10 @@ async def set_input_files( ) -> None: params = locals_to_params(locals()) converted = await convert_input_files(files, self.page.context) - if converted.get("files") is not None: - params["files"] = converted.get("files") - await self._channel.send("setInputFiles", params) + if converted["files"]: + await self._channel.send( + "setInputFiles", {**params, "files": converted["files"]} + ) else: await self._channel.send( "setInputFilePaths", diff --git a/playwright/_impl/_page.py b/playwright/_impl/_page.py index 5867c8b25..98916923a 100644 --- a/playwright/_impl/_page.py +++ b/playwright/_impl/_page.py @@ -83,7 +83,6 @@ serialize_argument, ) from playwright._impl._network import Request, Response, Route, serialize_headers -from playwright._impl._set_input_files_helpers import convert_input_files from playwright._impl._video import Video from playwright._impl._wait_helper import WaitHelper diff --git a/playwright/_impl/_set_input_files_helpers.py b/playwright/_impl/_set_input_files_helpers.py index 9309e2d7b..c433bbc98 100644 --- a/playwright/_impl/_set_input_files_helpers.py +++ b/playwright/_impl/_set_input_files_helpers.py @@ -1,9 +1,13 @@ import base64 import os -import shutil +import sys from pathlib import Path -from posixpath import basename -from typing import TYPE_CHECKING, List, Union +from typing import TYPE_CHECKING, List, Optional, Union + +if sys.version_info >= (3, 8): # pragma: no cover + from typing import TypedDict +else: # pragma: no cover + from typing_extensions import TypedDict from playwright._impl._helper import Error, async_readfile from playwright._impl._writable_stream import WritableStream @@ -16,46 +20,59 @@ SIZE_LIMIT_IN_BYTES = 50 * 1024 * 1024 +class InputFilesList(TypedDict): + streams: Optional[List[WritableStream]] + localPaths: Optional[List[str]] + files: Optional[List[FilePayload]] + + async def convert_input_files( files: Union[str, Path, FilePayload, List[Union[str, Path]], List[FilePayload]], context: "BrowserContext", -): - files = files if isinstance(files, list) else [files] +) -> InputFilesList: + file_list = files if isinstance(files, list) else [files] - has_large_buffer = list( - filter( - lambda f: not isinstance(f, (str, Path)) - and len(f.get("buffer", "")) > SIZE_LIMIT_IN_BYTES, - files, - ) + has_large_buffer = any( + [ + not isinstance(f, (str, Path)) + and len(f.get("buffer", "")) > SIZE_LIMIT_IN_BYTES + for f in file_list + ] ) if has_large_buffer: raise Error( "Cannot set buffer larger than 50Mb, please write it to a file and pass its path instead." ) - has_large_file = list( - map( - lambda f: os.stat(f).st_size, - filter(lambda f: isinstance(f, (str, Path)), files), - ) + has_large_file = any( + [ + isinstance(f, (str, Path)) and os.stat(f).st_size > SIZE_LIMIT_IN_BYTES + for f in file_list + ] ) if has_large_file: if context._channel._connection.is_remote: streams = [] - for file in files: + for file in file_list: + assert isinstance(file, (str, Path)) stream: WritableStream = await context._channel.send( "createTempFile", {"name": os.path.basename(file)} ) await WritableStream.copy(file, stream) streams.append(stream) - return {"streams": streams} - return {"localPaths": list(map(lambda f: Path(f).absolute().resolve(), files))} + return InputFilesList(streams=streams, localPaths=None, files=None) + local_paths = [] + for p in file_list: + assert isinstance(p, (str, Path)) + local_paths.append(str(Path(p).absolute().resolve())) + return InputFilesList(streams=None, localPaths=local_paths, files=None) - return {"files": await normalize_file_payloads(files)} + return InputFilesList( + streams=None, localPaths=None, files=await _normalize_file_payloads(files) + ) -async def normalize_file_payloads( +async def _normalize_file_payloads( files: Union[str, Path, FilePayload, List[Union[str, Path]], List[FilePayload]] ) -> List: file_list = files if isinstance(files, list) else [files] diff --git a/playwright/_impl/_writable_stream.py b/playwright/_impl/_writable_stream.py index b18e1b3ad..fa5c90af4 100644 --- a/playwright/_impl/_writable_stream.py +++ b/playwright/_impl/_writable_stream.py @@ -14,14 +14,15 @@ import base64 import os -from typing import Dict +from pathlib import Path +from typing import Dict, Union + +from playwright._impl._connection import ChannelOwner # COPY_BUFSIZE is taken from shutil.py in the standard library -_WINDOWS = os.name == 'nt' +_WINDOWS = os.name == "nt" COPY_BUFSIZE = 1024 * 1024 if _WINDOWS else 64 * 1024 -from playwright._impl._connection import ChannelOwner - class WritableStream(ChannelOwner): def __init__( @@ -30,11 +31,11 @@ def __init__( super().__init__(parent, type, guid, initializer) @staticmethod - async def copy(path: os.PathLike, stream: 'WritableStream'): - with open(path, 'rb') as f: + async def copy(path: Union[str, Path], stream: "WritableStream") -> None: + with open(path, "rb") as f: while True: data = f.read(COPY_BUFSIZE) if not data: break await stream.send("write", {"binary": base64.b64encode(data).decode()}) - await stream.send('close') + await stream.send("close") diff --git a/tests/async/test_browsertype_connect.py b/tests/async/test_browsertype_connect.py index 86aeccd7c..9dce38780 100644 --- a/tests/async/test_browsertype_connect.py +++ b/tests/async/test_browsertype_connect.py @@ -12,10 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import Callable - import asyncio import re +from typing import Callable + import pytest from playwright.async_api import BrowserType, Error, Playwright, Route @@ -244,13 +244,15 @@ async def handle_request(route: Route) -> None: remote.kill() + @pytest.mark.only_browser("chromium") async def test_should_upload_large_file( browser_type: BrowserType, launch_server: Callable[[], RemoteServer], playwright: Playwright, server: Server, - tmp_path): + tmp_path, +): remote = launch_server() browser = await browser_type.connect(remote.ws_endpoint) @@ -259,7 +261,7 @@ async def test_should_upload_large_file( await page.goto(server.PREFIX + "/input/fileupload.html") large_file_path = tmp_path / "200MB.zip" - data = b'A' * 1024 + data = b"A" * 1024 with large_file_path.open("wb") as f: for i in range(0, 200 * 1024 * 1024, len(data)): f.write(data) @@ -279,18 +281,20 @@ async def test_should_upload_large_file( assert await input.evaluate("e => e.files[0].name") == "200MB.zip" assert await events.evaluate("e => e") == ["input", "change"] - file_upload = asyncio.Future() - def handler(request): - file_upload.set_result(request) - request.finish() - server.set_route("/upload", handler) + [request, _] = await asyncio.gather( + server.wait_for_request("/upload"), + page.click("input[type=submit]"), + ) - await page.click("input[type=submit]") - request = await file_upload - contents = request.args[b'file1'][0] + contents = request.args[b"file1"][0] assert len(contents) == 200 * 1024 * 1024 assert contents[:1024] == data - assert contents[len(contents)-1024:] == data - match = re.search(rb'^.*Content-Disposition: form-data; name="(?P.*)"; filename="(?P.*)".*$', request.post_body, re.MULTILINE) - assert match.group('name') == b'file1' - assert match.group('filename') == b'200MB.zip' + # flake8: noqa: E203 + assert contents[len(contents) - 1024 :] == data + match = re.search( + rb'^.*Content-Disposition: form-data; name="(?P.*)"; filename="(?P.*)".*$', + request.post_body, + re.MULTILINE, + ) + assert match.group("name") == b"file1" + assert match.group("filename") == b"200MB.zip" diff --git a/tests/async/test_input.py b/tests/async/test_input.py index a0d33edb6..77e0ca1c8 100644 --- a/tests/async/test_input.py +++ b/tests/async/test_input.py @@ -14,9 +14,8 @@ import asyncio import os -from pprint import pprint -from shutil import ExecError import re + import pytest from playwright._impl._path_utils import get_file_dirname @@ -292,7 +291,7 @@ async def _listen_for_wheel_events(page: Page, selector: str) -> None: async def test_should_upload_large_file(page, server, tmp_path): await page.goto(server.PREFIX + "/input/fileupload.html") large_file_path = tmp_path / "200MB.zip" - data = b'A' * 1024 + data = b"A" * 1024 with large_file_path.open("wb") as f: for i in range(0, 200 * 1024 * 1024, len(data)): f.write(data) @@ -312,18 +311,20 @@ async def test_should_upload_large_file(page, server, tmp_path): assert await input.evaluate("e => e.files[0].name") == "200MB.zip" assert await events.evaluate("e => e") == ["input", "change"] - file_upload = asyncio.Future() - def handler(request): - file_upload.set_result(request) - request.finish() - server.set_route("/upload", handler) + [request, _] = await asyncio.gather( + server.wait_for_request("/upload"), + page.click("input[type=submit]"), + ) - await page.click("input[type=submit]") - request = await file_upload - contents = request.args[b'file1'][0] + contents = request.args[b"file1"][0] assert len(contents) == 200 * 1024 * 1024 assert contents[:1024] == data - assert contents[len(contents)-1024:] == data - match = re.search(rb'^.*Content-Disposition: form-data; name="(?P.*)"; filename="(?P.*)".*$', request.post_body, re.MULTILINE) - assert match.group('name') == b'file1' - assert match.group('filename') == b'200MB.zip' + # flake8: noqa: E203 + assert contents[len(contents) - 1024 :] == data + match = re.search( + rb'^.*Content-Disposition: form-data; name="(?P.*)"; filename="(?P.*)".*$', + request.post_body, + re.MULTILINE, + ) + assert match.group("name") == b"file1" + assert match.group("filename") == b"200MB.zip" From 4ff19c6c6f7013feb3d3b3c8cf027dd38fa0d9f0 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Wed, 13 Apr 2022 11:35:40 -0700 Subject: [PATCH 10/13] more pythonic list comprehension --- playwright/_impl/_set_input_files_helpers.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/playwright/_impl/_set_input_files_helpers.py b/playwright/_impl/_set_input_files_helpers.py index c433bbc98..e42762145 100644 --- a/playwright/_impl/_set_input_files_helpers.py +++ b/playwright/_impl/_set_input_files_helpers.py @@ -34,9 +34,9 @@ async def convert_input_files( has_large_buffer = any( [ - not isinstance(f, (str, Path)) - and len(f.get("buffer", "")) > SIZE_LIMIT_IN_BYTES + len(f.get("buffer", "")) > SIZE_LIMIT_IN_BYTES for f in file_list + if not isinstance(f, (str, Path)) ] ) if has_large_buffer: @@ -46,8 +46,9 @@ async def convert_input_files( has_large_file = any( [ - isinstance(f, (str, Path)) and os.stat(f).st_size > SIZE_LIMIT_IN_BYTES + os.stat(f).st_size > SIZE_LIMIT_IN_BYTES for f in file_list + if isinstance(f, (str, Path)) ] ) if has_large_file: From aed76b5e754c3d872305e330b3cc333f39127116 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Wed, 13 Apr 2022 11:47:42 -0700 Subject: [PATCH 11/13] move copy to WriteableStream class --- playwright/_impl/_set_input_files_helpers.py | 13 ++++++++----- playwright/_impl/_writable_stream.py | 9 +++++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/playwright/_impl/_set_input_files_helpers.py b/playwright/_impl/_set_input_files_helpers.py index e42762145..a03a41e91 100644 --- a/playwright/_impl/_set_input_files_helpers.py +++ b/playwright/_impl/_set_input_files_helpers.py @@ -9,6 +9,7 @@ else: # pragma: no cover from typing_extensions import TypedDict +from playwright._impl._connection import Channel, from_channel from playwright._impl._helper import Error, async_readfile from playwright._impl._writable_stream import WritableStream @@ -21,7 +22,7 @@ class InputFilesList(TypedDict): - streams: Optional[List[WritableStream]] + streams: Optional[List[Channel]] localPaths: Optional[List[str]] files: Optional[List[FilePayload]] @@ -56,11 +57,13 @@ async def convert_input_files( streams = [] for file in file_list: assert isinstance(file, (str, Path)) - stream: WritableStream = await context._channel.send( - "createTempFile", {"name": os.path.basename(file)} + stream: WritableStream = from_channel( + await context._channel.send( + "createTempFile", {"name": os.path.basename(file)} + ) ) - await WritableStream.copy(file, stream) - streams.append(stream) + await stream.copy(file) + streams.append(stream._channel) return InputFilesList(streams=streams, localPaths=None, files=None) local_paths = [] for p in file_list: diff --git a/playwright/_impl/_writable_stream.py b/playwright/_impl/_writable_stream.py index fa5c90af4..702adf153 100644 --- a/playwright/_impl/_writable_stream.py +++ b/playwright/_impl/_writable_stream.py @@ -30,12 +30,13 @@ def __init__( ) -> None: super().__init__(parent, type, guid, initializer) - @staticmethod - async def copy(path: Union[str, Path], stream: "WritableStream") -> None: + async def copy(self, path: Union[str, Path]) -> None: with open(path, "rb") as f: while True: data = f.read(COPY_BUFSIZE) if not data: break - await stream.send("write", {"binary": base64.b64encode(data).decode()}) - await stream.send("close") + await self._channel.send( + "write", {"binary": base64.b64encode(data).decode()} + ) + await self._channel.send("close") From da8acd256399b42d8744b0de4a7ed462c89ab200 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Wed, 13 Apr 2022 11:57:37 -0700 Subject: [PATCH 12/13] add new init script test --- tests/async/test_add_init_script.py | 7 +++++++ tests/sync/test_add_init_script.py | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/tests/async/test_add_init_script.py b/tests/async/test_add_init_script.py index 7317e7523..515891a88 100644 --- a/tests/async/test_add_init_script.py +++ b/tests/async/test_add_init_script.py @@ -74,3 +74,10 @@ async def test_add_init_script_support_multiple_scripts(page): await page.goto("data:text/html,") assert await page.evaluate("window.script1") == 1 assert await page.evaluate("window.script2") == 2 + + +async def test_should_work_with_trailing_comments(page): + await page.add_init_script("// comment") + await page.add_init_script("window.secret = 42;") + await page.goto("data:text/html,") + assert await page.evaluate("secret") == 42 diff --git a/tests/sync/test_add_init_script.py b/tests/sync/test_add_init_script.py index 5fc626a18..039862063 100644 --- a/tests/sync/test_add_init_script.py +++ b/tests/sync/test_add_init_script.py @@ -78,3 +78,10 @@ def test_add_init_script_support_multiple_scripts(page: Page) -> None: page.goto("data:text/html,") assert page.evaluate("window.script1") == 1 assert page.evaluate("window.script2") == 2 + + +def test_should_work_with_trailing_comments(page: Page) -> None: + page.add_init_script("// comment") + page.add_init_script("window.secret = 42;") + page.goto("data:text/html,") + assert page.evaluate("secret") == 42 From 95a2d3e7da53dab0570a886a6ea9f861a8aae175 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Wed, 13 Apr 2022 12:16:47 -0700 Subject: [PATCH 13/13] fix test_should_be_able_to_reset_selected_files_with_empty_file_list --- playwright/_impl/_element_handle.py | 2 +- playwright/_impl/_frame.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/playwright/_impl/_element_handle.py b/playwright/_impl/_element_handle.py index ee0e73bb5..5803413dc 100644 --- a/playwright/_impl/_element_handle.py +++ b/playwright/_impl/_element_handle.py @@ -195,7 +195,7 @@ async def set_input_files( if not frame: raise Error("Cannot set input files to detached element") converted = await convert_input_files(files, frame.page.context) - if converted["files"]: + if converted["files"] is not None: await self._channel.send( "setInputFiles", {**params, "files": converted["files"]} ) diff --git a/playwright/_impl/_frame.py b/playwright/_impl/_frame.py index db798071d..471ea7dcb 100644 --- a/playwright/_impl/_frame.py +++ b/playwright/_impl/_frame.py @@ -599,7 +599,7 @@ async def set_input_files( ) -> None: params = locals_to_params(locals()) converted = await convert_input_files(files, self.page.context) - if converted["files"]: + if converted["files"] is not None: await self._channel.send( "setInputFiles", {**params, "files": converted["files"]} )