Skip to content

Conversation

@ShauryaDusht
Copy link
Contributor

@ShauryaDusht ShauryaDusht commented Jun 8, 2025

User description

🔗 Related Issues

Closes #14881

💥 What does this PR do?

Adds missing RelativeBy type annotation support to:

  • find_element and find_elements
  • expected_conditions.py (conditions now support RelativeBy locators)

Also adds unit tests to validate these changes.

🔧 Implementation Notes

  • Updated type hints from Tuple[str, str] to Union[Tuple[ByType, str], Tuple[RelativeBy, None]] where needed.

💡 Additional Considerations

N/A

🔄 Types of changes

  • Bug fixes
  • Tests coverage

Files Changed/Added

  • selenium/webdriver/remote/shadowroot.py
  • selenium/webdriver/remote/webdriver.py
  • selenium/webdriver/remote/webelement.py
  • selenium/webdriver/support/expected_conditions.py
  • test/unit/selenium/webdriver/support/test_expected_conditions_relative_by.py
  • test/unit/selenium/webdriver/remote/test_relative_by_annotations.py

PR Type

Bug fix, Enhancement, Tests


Description

  • Add RelativeBy type annotation support to find_element/find_elements

    • Update signatures for WebDriver, WebElement, and ShadowRoot
  • Update expected conditions to accept RelativeBy locators

    • Refactor type hints in expected_conditions.py
  • Add unit tests for RelativeBy type annotation support

    • Cover WebDriver, WebElement, ShadowRoot, and expected conditions

Changes walkthrough 📝

Relevant files
Enhancement
4 files
shadowroot.py
Add RelativeBy type annotation to find_element(s)               
+13/-5   
webdriver.py
Add RelativeBy type annotation to WebDriver find_element(s)
+157/-65
webelement.py
Add RelativeBy type annotation to WebElement find_element(s)
+37/-12 
expected_conditions.py
Support RelativeBy in expected_conditions locator arguments
+50/-25 
Tests
3 files
test_relative_by_annotations.py
Add unit tests for RelativeBy annotation support (remote)
+66/-0   
test_expected_conditions_relative_by.py
Add tests for expected_conditions with RelativeBy locators
+58/-0   
test_relative_by_annotations.py
Add unit tests for RelativeBy annotation support (core)   
+66/-0   

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @selenium-ci selenium-ci added C-py Python Bindings B-support Issue or PR related to support classes labels Jun 8, 2025
    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 8, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Missing Import

    The code adds a reference to Permissions class in the permissions property but the import for this class is removed. This will cause a NameError when the permissions property is accessed.

    if not self._websocket_connection:
        self._start_bidi()
    
    if self._permissions is None:
        self._permissions = Permissions(self._websocket_connection)
    Duplicate Test File

    This test file appears to be an exact duplicate of py/test/unit/selenium/webdriver/remote/test_relative_by_annotations.py, which could cause confusion and redundant test execution.

    from unittest.mock import MagicMock, Mock
    
    import pytest
    from selenium.webdriver.common.by import By
    from selenium.webdriver.remote.shadowroot import ShadowRoot
    from selenium.webdriver.remote.webdriver import WebDriver
    from selenium.webdriver.remote.webelement import WebElement
    from selenium.webdriver.support.relative_locator import RelativeBy, locate_with
    
    
    class TestRelativeByAnnotations:
        """Test that RelativeBy is properly accepted in type annotations"""
    
        def test_webdriver_find_element_accepts_relative_by(self):
            """Test WebDriver.find_element accepts RelativeBy"""
            driver = Mock(spec=WebDriver)
            relative_by = locate_with(By.TAG_NAME, "div").above({By.ID: "footer"})
    
            # This should not raise type checking errors
            driver.find_element(by=relative_by)
            driver.find_element(relative_by)
    
        def test_webdriver_find_elements_accepts_relative_by(self):
            """Test WebDriver.find_elements accepts RelativeBy"""
            driver = Mock(spec=WebDriver)
            relative_by = locate_with(By.TAG_NAME, "div").below({By.ID: "header"})
    
            # This should not raise type checking errors
            driver.find_elements(by=relative_by)
            driver.find_elements(relative_by)
    
        def test_webelement_find_element_accepts_relative_by(self):
            """Test WebElement.find_element accepts RelativeBy"""
            element = Mock(spec=WebElement)
            relative_by = locate_with(By.TAG_NAME, "span").near({By.CLASS_NAME: "button"})
    
            # This should not raise type checking errors
            element.find_element(by=relative_by)
            element.find_element(relative_by)
    
        def test_webelement_find_elements_accepts_relative_by(self):
            """Test WebElement.find_elements accepts RelativeBy"""
            element = Mock(spec=WebElement)
            relative_by = locate_with(By.TAG_NAME, "input").to_left_of({By.ID: "submit"})
    
            # This should not raise type checking errors
            element.find_elements(by=relative_by)
            element.find_elements(relative_by)
    
        def test_shadowroot_find_element_accepts_relative_by(self):
            """Test ShadowRoot.find_element accepts RelativeBy"""
            shadow_root = Mock(spec=ShadowRoot)
            relative_by = locate_with(By.TAG_NAME, "button").to_right_of({By.ID: "cancel"})
    
            # This should not raise type checking errors
            shadow_root.find_element(by=relative_by)
            shadow_root.find_element(relative_by)
    
        def test_shadowroot_find_elements_accepts_relative_by(self):
            """Test ShadowRoot.find_elements accepts RelativeBy"""
            shadow_root = Mock(spec=ShadowRoot)
            relative_by = locate_with(By.CSS_SELECTOR, ".item").above({By.ID: "footer"})
    
            # This should not raise type checking errors
            shadow_root.find_elements(by=relative_by)
            shadow_root.find_elements(relative_by)

    @selenium-ci
    Copy link
    Member

    Thank you, @ShauryaDusht for this code suggestion.

    The support packages contain example code that many users find helpful, but they do not necessarily represent
    the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

    We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
    If you have any questions, please contact us

    @selenium-ci selenium-ci closed this Jun 8, 2025
    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 8, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix missing attribute reference

    The permissions property references self._permissions which is initialized to
    None but never defined in the class. The import for Permissions was also
    removed. This will cause an AttributeError when accessing this property.

    py/selenium/webdriver/remote/webdriver.py [1405-1421]

     @property
     def permissions(self):
         """Returns a permissions module object for BiDi permissions commands.
         Returns:
         --------
         Permissions: an object containing access to BiDi permissions commands.
         Examples:
         ---------
         >>> from selenium.webdriver.common.bidi.permissions import PermissionDescriptor, PermissionState
         >>> descriptor = PermissionDescriptor("geolocation")
         >>> driver.permissions.set_permission(descriptor, PermissionState.GRANTED, "https://example.com")
         """
         if not self._websocket_connection:
             self._start_bidi()
     
    -    if self._permissions is None:
    +    if not hasattr(self, "_permissions") or self._permissions is None:
    +        from selenium.webdriver.common.bidi.permissions import Permissions
             self._permissions = Permissions(self._websocket_connection)
     
    +    return self._permissions
    +

    [To ensure code accuracy, apply this suggestion manually]

    Suggestion importance[1-10]: 9

    __

    Why: This identifies a critical bug where self._permissions initialization was removed from __init__ and the Permissions import was removed, but the permissions property still references both, which will cause AttributeError and NameError at runtime.

    High
    Fix incorrect type annotation

    The find_element method doesn't handle RelativeBy locators but the type
    annotation suggests it does. Either implement RelativeBy support or update the
    type annotation to reflect the actual supported types.

    py/selenium/webdriver/remote/shadowroot.py [50-55]

    -def find_element(self, by: "Union[ByType, RelativeBy]" = By.ID, value: str = None):
    +def find_element(self, by: "ByType" = By.ID, value: str = None):
         """Find an element inside a shadow root given a By strategy and
         locator.
     
         Parameters:
         -----------
    • Apply / Chat
    Suggestion importance[1-10]: 8

    __

    Why: The suggestion correctly identifies that the ShadowRoot.find_element method claims to accept RelativeBy in its type annotation but lacks the implementation to handle it, unlike the main WebDriver class which has special handling for RelativeBy instances.

    Medium
    • More

    @cgoldberg
    Copy link
    Member

    @ShauryaDusht please stop submitting duplicate PR's. I'm leaving them all closed because I have no idea what you are actually trying to submit.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    B-support Issue or PR related to support classes C-py Python Bindings Review effort 2/5

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    [🐛 Bug]: Missing support for RelativeBy in annotations

    3 participants