♻️ refactor: Cleaned up code and added tests.

Author: cleitonleonel

pydoll/browser/base.py

@@ -256,26 +256,15 @@ async def get_window_id(self):
 
         if response.get('error'):
             pages = await self.get_targets()
-            valid_page = next(
-                (page for page in pages if page.get('type') == 'page' and page.get('attached')),
-                None
+            target_id = await self._get_valid_target_id(pages)
+            response = await self._execute_command(
+                BrowserCommands.get_window_id_by_target(target_id)
             )
 
-            if not valid_page:
-                raise RuntimeError("No valid attached browser page found.")
+        if window_id := response.get('result', {}).get('windowId'):
+            return window_id
 
-            target_id = valid_page.get('targetId')
-            if not target_id:
-                raise RuntimeError("Valid page found but missing 'targetId'.")
-
-            command = BrowserCommands.get_window_id_by_target(target_id)
-            response = await self._execute_command(command)
-
-        try:
-            return response['result']['windowId']
-        except KeyError:
-            error_message = response.get('error', {}).get('message', "Unknown error")
-            raise RuntimeError(f"Failed to get window ID: {error_message}")
+        raise RuntimeError(response.get('error', {}))
 
     async def set_window_bounds(self, bounds: dict):
         """
@@ -548,7 +537,7 @@ def _is_valid_page(page: dict) -> bool:
             'url', ''
         )
 
-    async def _get_valid_page(self, pages) -> str:
+    async def _get_valid_page(self, pages: list) -> str:
         """
         Gets the ID of a valid page or creates a new one.
 
@@ -570,6 +559,31 @@ async def _get_valid_page(self, pages) -> str:
 
         return await self.new_page()
 
+    @staticmethod
+    async def _get_valid_target_id(pages: list) -> str:
+        """
+        Retrieves the target ID of a valid attached browser page.
+
+        Returns:
+            str: The target ID of a valid page.
+
+        """
+
+        valid_page = next(
+            (page for page in pages
+             if page.get('type') == 'page' and page.get('attached')),
+            None
+        )
+
+        if not valid_page:
+            raise RuntimeError("No valid attached browser page found.")
+
+        target_id = valid_page.get('targetId')
+        if not target_id:
+            raise RuntimeError("Valid page found but missing 'targetId'.")
+
+        return target_id
+
     async def _is_browser_running(self, timeout: int = 10) -> bool:
         """
         Checks if the browser process is currently running.

pydoll/browser/page.py

@@ -32,7 +32,6 @@ def __init__(self, connection_port: int, page_id: str):
         self._network_events_enabled = False
         self._fetch_events_enabled = False
         self._dom_events_enabled = False
-        self._page_id = page_id
 
     @property
     def page_events_enabled(self) -> bool:

tests/test_browser_window.py

@@ -0,0 +1,108 @@
+import pytest
+import pytest_asyncio
+from unittest.mock import AsyncMock, MagicMock, patch
+from pydoll.browser.base import Browser
+from pydoll.commands.browser import BrowserCommands
+
+
+class ConcreteBrowser(Browser):
+    def _get_default_binary_location(self) -> str:
+        return '/fake/path/to/browser'
+
+
+@pytest_asyncio.fixture
+async def mock_browser():
+    with patch.multiple(
+            Browser,
+            _get_default_binary_location=MagicMock(
+                return_value='/fake/path/to/browser'
+            ),
+    ), patch(
+        'pydoll.connection.connection.ConnectionHandler',
+        autospec=True,
+    ) as mock_conn_handler:
+        browser = ConcreteBrowser()
+        browser._connection_handler = mock_conn_handler.return_value
+        browser._connection_handler.execute_command = AsyncMock()
+        browser._connection_handler.register_callback = AsyncMock()
+
+        browser._pages = ['page1']
+
+        yield browser
+
+
+@pytest.mark.asyncio
+async def test_get_window_id_success(mock_browser):
+    mock_browser._connection_handler.execute_command.return_value = {
+        'result': {'windowId': 123}
+    }
+    result = await mock_browser.get_window_id()
+    assert result == 123
+
+
+@pytest.mark.asyncio
+async def test_get_window_id_with_error_and_retry(mock_browser):
+    mock_browser._execute_command = AsyncMock(side_effect=[
+        {'error': 'some error'},
+        {'result': {
+            'targetInfos': [{
+                'type': 'page',
+                'attached': True,
+                'targetId': 'target1'
+            }]
+        }},
+        {'result': {'windowId': 123}}
+    ])
+
+    result = await mock_browser.get_window_id()
+    assert result == 123
+
+
+@pytest.mark.asyncio
+async def test_get_window_id_failure(mock_browser):
+    mock_browser._connection_handler.execute_command.return_value = {
+        'error': 'some error'
+    }
+    mock_browser.get_targets = AsyncMock(return_value=[])
+    with pytest.raises(RuntimeError):
+        await mock_browser.get_window_id()
+
+
+@pytest.mark.asyncio
+async def test_get_valid_target_id_success(mock_browser):
+    pages = [{
+        'type': 'page',
+        'attached': True,
+        'targetId': 'target1'
+    }]
+    result = await mock_browser._get_valid_target_id(pages)
+    assert result == 'target1'
+
+
+@pytest.mark.asyncio
+async def test_get_valid_target_id_no_valid_page(mock_browser):
+    pages = []
+    with pytest.raises(RuntimeError, match="No valid attached browser page found."):
+        await mock_browser._get_valid_target_id(pages)
+
+
+@pytest.mark.asyncio
+async def test_get_valid_target_id_missing_target_id(mock_browser):
+    pages = [{
+        'type': 'page',
+        'attached': True,
+        'targetId': None
+    }]
+    with pytest.raises(RuntimeError, match="Valid page found but missing 'targetId'."):
+        await mock_browser._get_valid_target_id(pages)
+
+
+@pytest.mark.asyncio
+async def test_get_window_id_by_target(mock_browser):
+    expected_command = {
+        'method': 'Browser.getWindowForTarget',
+        'params': {
+            'targetId': 'target1',
+        },
+    }
+    assert BrowserCommands.get_window_id_by_target('target1') == expected_command