From 5f51cb7dc1774e24cb6e36b8ca0263c18f09db0e Mon Sep 17 00:00:00 2001
From: Pavel Feldman <pavel.feldman@gmail.com>
Date: Fri, 25 Jun 2021 11:51:03 -0700
Subject: [PATCH] fix(cdp): fix cdp session, add tests

---
 playwright/_impl/_cdp_session.py |  6 +--
 tests/async/test_cdp_session.py  | 16 ++++---
 tests/sync/test_cdp_session.py   | 78 ++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 11 deletions(-)
 create mode 100644 tests/sync/test_cdp_session.py

diff --git a/playwright/_impl/_cdp_session.py b/playwright/_impl/_cdp_session.py
index 3e78ad89d..a6af32b90 100644
--- a/playwright/_impl/_cdp_session.py
+++ b/playwright/_impl/_cdp_session.py
@@ -16,7 +16,6 @@
 
 from playwright._impl._connection import ChannelOwner
 from playwright._impl._helper import locals_to_params
-from playwright._impl._js_handle import parse_result
 
 
 class CDPSession(ChannelOwner):
@@ -27,11 +26,10 @@ def __init__(
         self._channel.on("event", lambda params: self._on_event(params))
 
     def _on_event(self, params: Any) -> None:
-        self.emit(params["method"], parse_result(params["params"]))
+        self.emit(params["method"], params["params"])
 
     async def send(self, method: str, params: Dict = None) -> Dict:
-        result = await self._channel.send("send", locals_to_params(locals()))
-        return parse_result(result)
+        return await self._channel.send("send", locals_to_params(locals()))
 
     async def detach(self) -> None:
         await self._channel.send("detach")
diff --git a/tests/async/test_cdp_session.py b/tests/async/test_cdp_session.py
index 6f0bfda68..14ee65db3 100644
--- a/tests/async/test_cdp_session.py
+++ b/tests/async/test_cdp_session.py
@@ -12,8 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import asyncio
-
 import pytest
 
 from playwright.async_api import Error
@@ -22,17 +20,21 @@
 @pytest.mark.only_browser("chromium")
 async def test_should_work(page):
     client = await page.context.new_cdp_session(page)
-
-    await asyncio.gather(
-        client.send("Runtime.enable"),
-        client.send("Runtime.evaluate", {"expression": "window.foo = 'bar'"}),
+    events = []
+    client.on("Runtime.consoleAPICalled", lambda params: events.append(params))
+    await client.send("Runtime.enable")
+    result = await client.send(
+        "Runtime.evaluate",
+        {"expression": "window.foo = 'bar'; console.log('log'); 'result'"},
     )
+    assert result == {"result": {"type": "string", "value": "result"}}
     foo = await page.evaluate("() => window.foo")
     assert foo == "bar"
+    assert events[0]["args"][0]["value"] == "log"
 
 
 @pytest.mark.only_browser("chromium")
-async def test_should_send_events(page, server):
+async def test_should_receive_events(page, server):
     client = await page.context.new_cdp_session(page)
     await client.send("Network.enable")
     events = []
diff --git a/tests/sync/test_cdp_session.py b/tests/sync/test_cdp_session.py
new file mode 100644
index 000000000..deae8d0da
--- /dev/null
+++ b/tests/sync/test_cdp_session.py
@@ -0,0 +1,78 @@
+# 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 pytest
+
+from playwright.sync_api import Error
+
+
+@pytest.mark.only_browser("chromium")
+def test_should_work(page):
+    client = page.context.new_cdp_session(page)
+    events = []
+    client.on("Runtime.consoleAPICalled", lambda params: events.append(params))
+    client.send("Runtime.enable")
+    result = client.send(
+        "Runtime.evaluate",
+        {"expression": "window.foo = 'bar'; console.log('log'); 'result'"},
+    )
+    assert result == {"result": {"type": "string", "value": "result"}}
+    foo = page.evaluate("() => window.foo")
+    assert foo == "bar"
+    assert events[0]["args"][0]["value"] == "log"
+
+
+@pytest.mark.only_browser("chromium")
+def test_should_receive_events(page, server):
+    client = page.context.new_cdp_session(page)
+    client.send("Network.enable")
+    events = []
+    client.on("Network.requestWillBeSent", lambda event: events.append(event))
+    page.goto(server.EMPTY_PAGE)
+    assert len(events) == 1
+
+
+@pytest.mark.only_browser("chromium")
+def test_should_be_able_to_detach_session(page):
+    client = page.context.new_cdp_session(page)
+    client.send("Runtime.enable")
+    eval_response = client.send(
+        "Runtime.evaluate", {"expression": "1 + 2", "returnByValue": True}
+    )
+    assert eval_response["result"]["value"] == 3
+    client.detach()
+    with pytest.raises(Error) as exc_info:
+        client.send("Runtime.evaluate", {"expression": "3 + 1", "returnByValue": True})
+    assert "Target page, context or browser has been closed" in exc_info.value.message
+
+
+@pytest.mark.only_browser("chromium")
+def test_should_not_break_page_close(browser):
+    context = browser.new_context()
+    page = context.new_page()
+    session = page.context.new_cdp_session(page)
+    session.detach()
+    page.close()
+    context.close()
+
+
+@pytest.mark.only_browser("chromium")
+def test_should_detach_when_page_closes(browser):
+    context = browser.new_context()
+    page = context.new_page()
+    session = context.new_cdp_session(page)
+    page.close()
+    with pytest.raises(Error):
+        session.detach()
+    context.close()