|
| 1 | +--- |
| 2 | +description: Add a new low-level browser operation to both Selenium and Playwright implementations with tests |
| 3 | +--- |
| 4 | + |
| 5 | +## User Input |
| 6 | + |
| 7 | +```text |
| 8 | +$ARGUMENTS |
| 9 | +``` |
| 10 | + |
| 11 | +**Operation Description**: The user input describes the browser operation to add (e.g., "add method to scroll element to center of viewport" or "add method to get element's bounding box"). |
| 12 | + |
| 13 | +## Implementation Steps |
| 14 | + |
| 15 | +1. **Parse the operation description** from user input: |
| 16 | + - Extract the operation name (convert to snake_case method name) |
| 17 | + - Identify parameters needed |
| 18 | + - Determine return type |
| 19 | + - Understand the browser behavior to implement |
| 20 | + |
| 21 | +2. **Add to HasDriverProtocol** (`lib/galaxy/selenium/has_driver_protocol.py`): |
| 22 | + - Add abstract method with proper signature |
| 23 | + - Include comprehensive docstring with: |
| 24 | + * Description of what the operation does |
| 25 | + * Args documentation |
| 26 | + * Returns documentation |
| 27 | + * Usage example if helpful |
| 28 | + - Add `ContextManager` import if method returns a context manager |
| 29 | + |
| 30 | +3. **Implement in HasDriver** (`lib/galaxy/selenium/has_driver.py`): |
| 31 | + - Implement using Selenium WebDriver API |
| 32 | + - Use `self.driver` for page-level operations |
| 33 | + - Use element methods for element-specific operations |
| 34 | + - Handle any Selenium-specific quirks or edge cases |
| 35 | + - Add appropriate error handling |
| 36 | + |
| 37 | +4. **Implement in HasPlaywrightDriver** (`lib/galaxy/selenium/has_playwright_driver.py`): |
| 38 | + - Implement using Playwright API |
| 39 | + - Use `self.page` for page-level operations |
| 40 | + - For element operations, follow the wrapper pattern: |
| 41 | + * Create public method accepting `WebElementProtocol` |
| 42 | + * Use `self._unwrap_element()` to get `ElementHandle` |
| 43 | + * Create private `_method_name()` with `ElementHandle` parameter |
| 44 | + - Use `self._frame_or_page` instead of `self.page` if operation should work within frames |
| 45 | + - Handle any Playwright-specific patterns |
| 46 | + |
| 47 | +5. **Update HasDriverProxy** (`lib/galaxy/selenium/has_driver_proxy.py`): |
| 48 | + - Add delegation method that forwards to `self._driver_impl` |
| 49 | + - Include docstring (can be brief, main docs are in protocol) |
| 50 | + - Ensure type hints match the protocol |
| 51 | + |
| 52 | +6. **Add comprehensive unit tests** (`test/unit/selenium/test_has_driver.py`): |
| 53 | + - Create new test class or add to existing relevant class |
| 54 | + - Test the operation with `has_driver_instance` fixture (parametrized for both backends) |
| 55 | + - Include tests for: |
| 56 | + * Basic functionality |
| 57 | + * Edge cases |
| 58 | + * Error conditions if applicable |
| 59 | + * Different element states if relevant |
| 60 | + - Use `base_url` fixture to load test pages |
| 61 | + - Assert expected behavior for both Selenium and Playwright |
| 62 | + |
| 63 | +7. **Verify implementation**: |
| 64 | + - Run tests: `uv run pytest tests/seleniumtests/test_has_driver.py::<TestClass>::<test_name> -v` |
| 65 | + - Run type checking: `make mypy` |
| 66 | + - Ensure all 3 backend variants pass (selenium, playwright, proxy-selenium) |
| 67 | + |
| 68 | +8. **Update WebElementProtocol if needed**: |
| 69 | + - If the operation is element-specific and should be callable on elements directly |
| 70 | + - Add to `lib/galaxy/selenium/web_element_protocol.py` |
| 71 | + - Implement in `lib/galaxy/selenium/playwright_element.py` for Playwright |
| 72 | + - Selenium's WebElement will implement it natively if it's a standard method |
| 73 | + |
| 74 | +## Example Workflow |
| 75 | + |
| 76 | +For operation "get element's computed background color": |
| 77 | + |
| 78 | +1. **Protocol** (`has_driver_protocol.py`): |
| 79 | +```python |
| 80 | +@abstractmethod |
| 81 | +def get_element_background_color(self, element: WebElementProtocol) -> str: |
| 82 | + """Get the computed background color of an element. |
| 83 | +
|
| 84 | + Args: |
| 85 | + element: The element to get background color from |
| 86 | +
|
| 87 | + Returns: |
| 88 | + The computed background-color CSS value (e.g., "rgb(255, 0, 0)") |
| 89 | + """ |
| 90 | + ... |
| 91 | +``` |
| 92 | + |
| 93 | +2. **Selenium** (`has_driver.py`): |
| 94 | +```python |
| 95 | +def get_element_background_color(self, element: WebElementProtocol) -> str: |
| 96 | + return element.value_of_css_property("background-color") |
| 97 | +``` |
| 98 | + |
| 99 | +3. **Playwright** (`has_playwright_driver.py`): |
| 100 | +```python |
| 101 | +def get_element_background_color(self, element: WebElementProtocol) -> str: |
| 102 | + return element.value_of_css_property("background-color") |
| 103 | +``` |
| 104 | + |
| 105 | +4. **Proxy** (`has_driver_proxy.py`): |
| 106 | +```python |
| 107 | +def get_element_background_color(self, element: WebElementProtocol) -> str: |
| 108 | + """Get the computed background color of an element.""" |
| 109 | + return self._driver_impl.get_element_background_color(element) |
| 110 | +``` |
| 111 | + |
| 112 | +5. **Tests** (`test_has_driver.py`): |
| 113 | +```python |
| 114 | +class TestCSSProperties: |
| 115 | + def test_get_element_background_color(self, has_driver_instance, base_url): |
| 116 | + has_driver_instance.navigate_to(f"{base_url}/basic.html") |
| 117 | + element = has_driver_instance.find_element_by_id("test-div") |
| 118 | + color = has_driver_instance.get_element_background_color(element) |
| 119 | + assert color != "" |
| 120 | + # Color format varies by browser, just check it's a valid value |
| 121 | + assert isinstance(color, str) |
| 122 | +``` |
| 123 | + |
| 124 | +## Notes |
| 125 | + |
| 126 | +- Always maintain API compatibility between Selenium and Playwright implementations |
| 127 | +- Use the wrapper pattern (`_unwrap_element()`) for element operations in Playwright |
| 128 | +- Ensure both implementations have identical behavior from the user's perspective |
| 129 | +- Write thorough tests that verify behavior on both backends |
| 130 | +- Follow existing code patterns and naming conventions |
0 commit comments