From 58b405fe1c4a4f4a411ae34c23a7021670ac5403 Mon Sep 17 00:00:00 2001 From: Pavel Date: Wed, 28 Oct 2020 00:07:38 -0700 Subject: [PATCH] api: use bytes, not string|bytes for file input buffers --- playwright/async_api.py | 8 +-- playwright/element_handle.py | 30 ++++++----- playwright/helper.py | 8 ++- playwright/sync_api.py | 8 +-- scripts/expected_api_mismatch.txt | 8 +-- tests/async/test_click.py | 23 +++------ tests/async/test_input.py | 8 +-- tests/async/test_page.py | 2 +- tests/async/test_worker.py | 83 +++++++++++++++---------------- tests/sync/test_input.py | 23 +++++++++ 10 files changed, 113 insertions(+), 88 deletions(-) create mode 100644 tests/sync/test_input.py diff --git a/playwright/async_api.py b/playwright/async_api.py index c6a7bb97a..d9741c913 100644 --- a/playwright/async_api.py +++ b/playwright/async_api.py @@ -1186,7 +1186,7 @@ async def setInputFiles( Parameters ---------- - files : Union[str, pathlib.Path, {"name": str, "mimeType": str, "buffer": Union[bytes, str]}, List[str], List[pathlib.Path], List[{"name": str, "mimeType": str, "buffer": Union[bytes, str]}]] + files : Union[str, pathlib.Path, {"name": str, "mimeType": str, "buffer": bytes}, List[str], List[pathlib.Path], List[{"name": str, "mimeType": str, "buffer": bytes}]] timeout : Optional[int] Maximum time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the browserContext.setDefaultTimeout(timeout) or page.setDefaultTimeout(timeout) methods. noWaitAfter : Optional[bool] @@ -1662,7 +1662,7 @@ async def setFiles( Parameters ---------- - files : Union[str, {"name": str, "mimeType": str, "buffer": Union[bytes, str]}, List[str], List[{"name": str, "mimeType": str, "buffer": Union[bytes, str]}]] + files : Union[str, {"name": str, "mimeType": str, "buffer": bytes}, List[str], List[{"name": str, "mimeType": str, "buffer": bytes}]] timeout : Optional[int] Maximum time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the browserContext.setDefaultTimeout(timeout) or page.setDefaultTimeout(timeout) methods. noWaitAfter : Optional[bool] @@ -2588,7 +2588,7 @@ async def setInputFiles( ---------- selector : str A selector to search for element to click. If there are multiple elements satisfying the selector, the first will be clicked. See working with selectors for more details. - files : Union[str, pathlib.Path, {"name": str, "mimeType": str, "buffer": Union[bytes, str]}, List[str], List[pathlib.Path], List[{"name": str, "mimeType": str, "buffer": Union[bytes, str]}]] + files : Union[str, pathlib.Path, {"name": str, "mimeType": str, "buffer": bytes}, List[str], List[pathlib.Path], List[{"name": str, "mimeType": str, "buffer": bytes}]] timeout : Optional[int] Maximum time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the browserContext.setDefaultTimeout(timeout) or page.setDefaultTimeout(timeout) methods. noWaitAfter : Optional[bool] @@ -4648,7 +4648,7 @@ async def setInputFiles( ---------- selector : str A selector to search for element to click. If there are multiple elements satisfying the selector, the first will be clicked. See working with selectors for more details. - files : Union[str, {"name": str, "mimeType": str, "buffer": Union[bytes, str]}, List[str], List[{"name": str, "mimeType": str, "buffer": Union[bytes, str]}]] + files : Union[str, {"name": str, "mimeType": str, "buffer": bytes}, List[str], List[{"name": str, "mimeType": str, "buffer": bytes}]] timeout : Optional[int] Maximum time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the browserContext.setDefaultTimeout(timeout) or page.setDefaultTimeout(timeout) methods. noWaitAfter : Optional[bool] diff --git a/playwright/element_handle.py b/playwright/element_handle.py index 681a323f8..b73d62481 100644 --- a/playwright/element_handle.py +++ b/playwright/element_handle.py @@ -27,6 +27,7 @@ MouseButton, MousePosition, SelectOption, + SetFilePayload, locals_to_params, ) from playwright.js_handle import ( @@ -291,22 +292,27 @@ def convert_select_option_values(arg: ValuesToSelect) -> Any: def normalize_file_payloads( files: Union[str, Path, FilePayload, List[str], List[Path], List[FilePayload]] -) -> List[FilePayload]: +) -> List[SetFilePayload]: file_list = files if isinstance(files, list) else [files] - file_payloads: List[FilePayload] = [] + file_payloads: List[SetFilePayload] = [] for item in file_list: if isinstance(item, str) or isinstance(item, Path): with open(item, mode="rb") as fd: - file: FilePayload = { - "name": os.path.basename(item), - "mimeType": mimetypes.guess_type(str(Path(item)))[0] - or "application/octet-stream", - "buffer": base64.b64encode(fd.read()).decode(), - } - file_payloads.append(file) + file_payloads.append( + { + "name": os.path.basename(item), + "mimeType": mimetypes.guess_type(str(Path(item)))[0] + or "application/octet-stream", + "buffer": base64.b64encode(fd.read()).decode(), + } + ) else: - if isinstance(item["buffer"], bytes): - item["buffer"] = base64.b64encode(item["buffer"]).decode() - file_payloads.append(item) + file_payloads.append( + { + "name": item["name"], + "mimeType": item["mimeType"], + "buffer": base64.b64encode(item["buffer"]).decode(), + } + ) return file_payloads diff --git a/playwright/helper.py b/playwright/helper.py index a3d46df98..d26d8fe67 100644 --- a/playwright/helper.py +++ b/playwright/helper.py @@ -59,7 +59,13 @@ class MousePosition(TypedDict): class FilePayload(TypedDict): name: str mimeType: str - buffer: Union[bytes, str] + buffer: bytes + + +class SetFilePayload(TypedDict): + name: str + mimeType: str + buffer: str class SelectOption(TypedDict): diff --git a/playwright/sync_api.py b/playwright/sync_api.py index f191f00d2..189e5465d 100644 --- a/playwright/sync_api.py +++ b/playwright/sync_api.py @@ -1224,7 +1224,7 @@ def setInputFiles( Parameters ---------- - files : Union[str, pathlib.Path, {"name": str, "mimeType": str, "buffer": Union[bytes, str]}, List[str], List[pathlib.Path], List[{"name": str, "mimeType": str, "buffer": Union[bytes, str]}]] + files : Union[str, pathlib.Path, {"name": str, "mimeType": str, "buffer": bytes}, List[str], List[pathlib.Path], List[{"name": str, "mimeType": str, "buffer": bytes}]] timeout : Optional[int] Maximum time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the browserContext.setDefaultTimeout(timeout) or page.setDefaultTimeout(timeout) methods. noWaitAfter : Optional[bool] @@ -1718,7 +1718,7 @@ def setFiles( Parameters ---------- - files : Union[str, {"name": str, "mimeType": str, "buffer": Union[bytes, str]}, List[str], List[{"name": str, "mimeType": str, "buffer": Union[bytes, str]}]] + files : Union[str, {"name": str, "mimeType": str, "buffer": bytes}, List[str], List[{"name": str, "mimeType": str, "buffer": bytes}]] timeout : Optional[int] Maximum time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the browserContext.setDefaultTimeout(timeout) or page.setDefaultTimeout(timeout) methods. noWaitAfter : Optional[bool] @@ -2683,7 +2683,7 @@ def setInputFiles( ---------- selector : str A selector to search for element to click. If there are multiple elements satisfying the selector, the first will be clicked. See working with selectors for more details. - files : Union[str, pathlib.Path, {"name": str, "mimeType": str, "buffer": Union[bytes, str]}, List[str], List[pathlib.Path], List[{"name": str, "mimeType": str, "buffer": Union[bytes, str]}]] + files : Union[str, pathlib.Path, {"name": str, "mimeType": str, "buffer": bytes}, List[str], List[pathlib.Path], List[{"name": str, "mimeType": str, "buffer": bytes}]] timeout : Optional[int] Maximum time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the browserContext.setDefaultTimeout(timeout) or page.setDefaultTimeout(timeout) methods. noWaitAfter : Optional[bool] @@ -4835,7 +4835,7 @@ def setInputFiles( ---------- selector : str A selector to search for element to click. If there are multiple elements satisfying the selector, the first will be clicked. See working with selectors for more details. - files : Union[str, {"name": str, "mimeType": str, "buffer": Union[bytes, str]}, List[str], List[{"name": str, "mimeType": str, "buffer": Union[bytes, str]}]] + files : Union[str, {"name": str, "mimeType": str, "buffer": bytes}, List[str], List[{"name": str, "mimeType": str, "buffer": bytes}]] timeout : Optional[int] Maximum time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the browserContext.setDefaultTimeout(timeout) or page.setDefaultTimeout(timeout) methods. noWaitAfter : Optional[bool] diff --git a/scripts/expected_api_mismatch.txt b/scripts/expected_api_mismatch.txt index 1bed1ea0a..8e49efe7f 100644 --- a/scripts/expected_api_mismatch.txt +++ b/scripts/expected_api_mismatch.txt @@ -74,10 +74,10 @@ Parameter not documented: BrowserContext.addInitScript(path=) Parameter not documented: Page.addInitScript(path=) # File payload -Parameter type mismatch in FileChooser.setFiles(files=): documented as Union[str, List[str], Dict, List[Dict]], code has Union[str, {"name": str, "mimeType": str, "buffer": Union[bytes, str]}, List[str], List[{"name": str, "mimeType": str, "buffer": Union[bytes, str]}]] -Parameter type mismatch in Page.setInputFiles(files=): documented as Union[str, List[str], Dict, List[Dict]], code has Union[str, {"name": str, "mimeType": str, "buffer": Union[bytes, str]}, List[str], List[{"name": str, "mimeType": str, "buffer": Union[bytes, str]}]] -Parameter type mismatch in ElementHandle.setInputFiles(files=): documented as Union[str, List[str], Dict, List[Dict]], code has Union[str, pathlib.Path, {"name": str, "mimeType": str, "buffer": Union[bytes, str]}, List[str], List[pathlib.Path], List[{"name": str, "mimeType": str, "buffer": Union[bytes, str]}]] -Parameter type mismatch in Frame.setInputFiles(files=): documented as Union[str, List[str], Dict, List[Dict]], code has Union[str, pathlib.Path, {"name": str, "mimeType": str, "buffer": Union[bytes, str]}, List[str], List[pathlib.Path], List[{"name": str, "mimeType": str, "buffer": Union[bytes, str]}]] +Parameter type mismatch in FileChooser.setFiles(files=): documented as Union[str, List[str], Dict, List[Dict]], code has Union[str, {"name": str, "mimeType": str, "buffer": bytes}, List[str], List[{"name": str, "mimeType": str, "buffer": bytes}]] +Parameter type mismatch in Page.setInputFiles(files=): documented as Union[str, List[str], Dict, List[Dict]], code has Union[str, {"name": str, "mimeType": str, "buffer": bytes}, List[str], List[{"name": str, "mimeType": str, "buffer": bytes}]] +Parameter type mismatch in ElementHandle.setInputFiles(files=): documented as Union[str, List[str], Dict, List[Dict]], code has Union[str, pathlib.Path, {"name": str, "mimeType": str, "buffer": bytes}, List[str], List[pathlib.Path], List[{"name": str, "mimeType": str, "buffer": bytes}]] +Parameter type mismatch in Frame.setInputFiles(files=): documented as Union[str, List[str], Dict, List[Dict]], code has Union[str, pathlib.Path, {"name": str, "mimeType": str, "buffer": bytes}, List[str], List[pathlib.Path], List[{"name": str, "mimeType": str, "buffer": bytes}]] # Select option Parameter type mismatch in ElementHandle.selectOption(values=): documented as Union[str, ElementHandle, List[str], Dict, List[ElementHandle], List[Dict], NoneType], code has Union[str, ElementHandle, {"value": Optional[str], "label": Optional[str], "index": Optional[str]}, List[str], List[ElementHandle], List[{"value": Optional[str], "label": Optional[str], "index": Optional[str]}], NoneType] diff --git a/tests/async/test_click.py b/tests/async/test_click.py index 1895e8034..c5341b6b5 100644 --- a/tests/async/test_click.py +++ b/tests/async/test_click.py @@ -809,13 +809,10 @@ async def test_fail_when_element_detaches_after_animation(page, server): promise = asyncio.create_task(handle.click()) await asyncio.sleep(0) # execute scheduled tasks, but don't await them await page.evaluate("stopButton(true)") - error = None - try: - error = await promise - except Error as e: - error = e + with pytest.raises(Error) as exc_info: + await promise assert await page.evaluate("window.clicked") is None - assert "Element is not attached to the DOM" in error.message + assert "Element is not attached to the DOM" in exc_info.value.message async def test_retry_when_element_detaches_after_animation(page, server): @@ -950,16 +947,12 @@ async def test_click_the_button_when_window_inner_width_is_corrupted(page, serve async def test_timeout_when_click_opens_alert(page, server): - dialog_promise = asyncio.create_task(page.waitForEvent("dialog")) - await asyncio.sleep(0) # execute scheduled tasks, but don't await them await page.setContent('
Click me
') - error = None - try: - await page.click("div", timeout=3000) - except TimeoutError as e: - error = e - assert "Timeout 3000ms exceeded" in error.message - dialog = await dialog_promise + async with page.expect_event("dialog") as dialog_info: + with pytest.raises(Error) as exc_info: + await page.click("div", timeout=3000) + assert "Timeout 3000ms exceeded" in exc_info.value.message + dialog = await dialog_info.value await dialog.dismiss() diff --git a/tests/async/test_input.py b/tests/async/test_input.py index 490d9fbbc..b07eb552f 100644 --- a/tests/async/test_input.py +++ b/tests/async/test_input.py @@ -77,10 +77,10 @@ async def test_should_emit_event(page: Page, server): async def test_should_work_when_file_input_is_attached_to_DOM(page: Page, server): await page.setContent("") - file_chooser = asyncio.create_task(page.waitForEvent("filechooser")) - await asyncio.sleep(0) # execute scheduled tasks, but don't await them - await page.click("input") - assert await file_chooser + async with page.expect_event("filechooser") as fc_info: + await page.click("input") + file_chooser = await fc_info.value + assert file_chooser async def test_should_work_when_file_input_is_not_attached_to_DOM(page, server): diff --git a/tests/async/test_page.py b/tests/async/test_page.py index 84d1ffbf6..1f52c66f8 100644 --- a/tests/async/test_page.py +++ b/tests/async/test_page.py @@ -224,7 +224,7 @@ async def test_wait_for_request_should_work_with_url_match(page, server): assert request.url == server.PREFIX + "/digits/1.png" -async def test_wait_for_event_should_fail_with_error_upon_disconnect(page, server): +async def test_wait_for_event_should_fail_with_error_upon_disconnect(page): future = asyncio.create_task(page.waitForEvent("download")) await asyncio.sleep(0) # execute scheduled tasks, but don't await them await page.close() diff --git a/tests/async/test_worker.py b/tests/async/test_worker.py index c75889a9b..be5b91a8d 100644 --- a/tests/async/test_worker.py +++ b/tests/async/test_worker.py @@ -38,12 +38,12 @@ async def test_workers_page_workers(page, server): async def test_workers_should_emit_created_and_destroyed_events(page: Page): - worker_createdpromise = asyncio.create_task(page.waitForEvent("worker")) - await asyncio.sleep(0) # execute scheduled tasks, but don't await them - worker_obj = await page.evaluateHandle( - "() => new Worker(URL.createObjectURL(new Blob(['1'], {type: 'application/javascript'})))" - ) - worker = await worker_createdpromise + worker_obj = None + async with page.expect_event("worker") as event_info: + worker_obj = await page.evaluateHandle( + "() => new Worker(URL.createObjectURL(new Blob(['1'], {type: 'application/javascript'})))" + ) + worker = await event_info.value worker_this_obj = await worker.evaluateHandle("() => this") worker_destroyed_promise: Future[Worker] = asyncio.Future() worker.once("close", lambda w: worker_destroyed_promise.set_result(w)) @@ -78,12 +78,11 @@ async def test_workers_should_have_JSHandles_for_console_logs(page): async def test_workers_should_evaluate(page): - worker_createdpromise = asyncio.create_task(page.waitForEvent("worker")) - await asyncio.sleep(0) # execute scheduled tasks, but don't await them - await page.evaluate( - "() => new Worker(URL.createObjectURL(new Blob(['console.log(1)'], {type: 'application/javascript'})))" - ) - worker = await worker_createdpromise + async with page.expect_event("worker") as event_info: + await page.evaluate( + "() => new Worker(URL.createObjectURL(new Blob(['console.log(1)'], {type: 'application/javascript'})))" + ) + worker = await event_info.value assert await worker.evaluate("1+1") == 2 @@ -105,12 +104,11 @@ async def test_workers_should_report_errors(page): async def test_workers_should_clear_upon_navigation(server, page): await page.goto(server.EMPTY_PAGE) - worker_createdpromise = asyncio.create_task(page.waitForEvent("worker")) - await asyncio.sleep(0) # execute scheduled tasks, but don't await them - await page.evaluate( - '() => new Worker(URL.createObjectURL(new Blob(["console.log(1)"], {type: "application/javascript"})))' - ) - worker = await worker_createdpromise + async with page.expect_event("worker") as event_info: + await page.evaluate( + '() => new Worker(URL.createObjectURL(new Blob(["console.log(1)"], {type: "application/javascript"})))' + ) + worker = await event_info.value assert len(page.workers) == 1 destroyed = [] worker.once("close", lambda _: destroyed.append(True)) @@ -121,12 +119,11 @@ async def test_workers_should_clear_upon_navigation(server, page): async def test_workers_should_clear_upon_cross_process_navigation(server, page): await page.goto(server.EMPTY_PAGE) - worker_createdpromise = asyncio.create_task(page.waitForEvent("worker")) - await asyncio.sleep(0) # execute scheduled tasks, but don't await them - await page.evaluate( - "() => new Worker(URL.createObjectURL(new Blob(['console.log(1)'], {type: 'application/javascript'})))" - ) - worker = await worker_createdpromise + async with page.expect_event("worker") as event_info: + await page.evaluate( + "() => new Worker(URL.createObjectURL(new Blob(['console.log(1)'], {type: 'application/javascript'})))" + ) + worker = await event_info.value assert len(page.workers) == 1 destroyed = [] worker.once("close", lambda _: destroyed.append(True)) @@ -141,14 +138,14 @@ async def test_workers_should_report_network_activity(page, server): page.goto(server.PREFIX + "/worker/worker.html"), ) url = server.PREFIX + "/one-style.css" - request_promise = asyncio.create_task(page.waitForRequest(url)) - response_promise = asyncio.create_task(page.waitForResponse(url)) - await asyncio.sleep(0) # execute scheduled tasks, but don't await them - await worker.evaluate( - "url => fetch(url).then(response => response.text()).then(console.log)", url - ) - request = await request_promise - response = await response_promise + async with page.expect_request(url) as request_info, page.expect_response( + url + ) as response_info: + await worker.evaluate( + "url => fetch(url).then(response => response.text()).then(console.log)", url + ) + request = await request_info.value + response = await response_info.value assert request.url == url assert response.request == request assert response.ok @@ -158,17 +155,17 @@ async def test_workers_should_report_network_activity_on_worker_creation(page, s # Chromium needs waitForDebugger enabled for this one. await page.goto(server.EMPTY_PAGE) url = server.PREFIX + "/one-style.css" - request_promise = asyncio.create_task(page.waitForRequest(url)) - response_promise = asyncio.create_task(page.waitForResponse(url)) - await asyncio.sleep(0) # execute scheduled tasks, but don't await them - await page.evaluate( - """url => new Worker(URL.createObjectURL(new Blob([` - fetch("${url}").then(response => response.text()).then(console.log); - `], {type: 'application/javascript'})))""", - url, - ) - request = await request_promise - response = await response_promise + async with page.expect_request(url) as request_info, page.expect_response( + url + ) as response_info: + await page.evaluate( + """url => new Worker(URL.createObjectURL(new Blob([` + fetch("${url}").then(response => response.text()).then(console.log); + `], {type: 'application/javascript'})))""", + url, + ) + request = await request_info.value + response = await response_info.value assert request.url == url assert response.request == request assert response.ok diff --git a/tests/sync/test_input.py b/tests/sync/test_input.py new file mode 100644 index 000000000..a45919139 --- /dev/null +++ b/tests/sync/test_input.py @@ -0,0 +1,23 @@ +# 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. + + +def test_expect_file_chooser(page, server): + page.setContent("") + with page.expect_file_chooser() as fc_info: + page.click('input[type="file"]') + fc = fc_info.value + fc.setFiles( + {"name": "test.txt", "mimeType": "text/plain", "buffer": b"Hello World"} + )