Skip to content

Conversation

@iampopovich
Copy link
Contributor

@iampopovich iampopovich commented Oct 30, 2025

User description

🔗 Related Issues

relates #16432 and #11442

💥 What does this PR do?

This pull request improves the clarity and completeness of docstrings across several Selenium WebDriver classes and methods. The main focus is on documenting the purpose and usage of parameters, especially the value parameter in element locator methods, and providing clearer explanations for device and window management arguments.

Docstring improvements for element location methods:

  • Added documentation for the value parameter in find_element and find_elements methods in WebDriver, WebElement, and ShadowRoot classes, clarifying its role in locator strategies. [1] [2] [3] [4] [5] [6]

Enhancements to window management method docstrings:

  • Improved docstrings for set_window_size and set_window_position methods in WebDriver to document the windowHandle parameter and its default value. [1] [2]

Device and service initialization docstring updates:

  • Updated the ActionChains and PointerActions class docstrings to clarify the optional devices and source parameters and their default behaviors. [1] [2]
  • Enhanced the Service class for IE driver to document the **kwargs parameter for additional customization.

Linting configuration adjustment:

  • Removed the D417 rule from the extend-ignore section in pyproject.toml, allowing enforcement of argument descriptions in __init__ docstrings.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Documentation


Description

  • Add missing parameter documentation to docstrings across multiple files

  • Document value parameter in element locator methods (find_element, find_elements)

  • Document devices and source parameters in ActionChains and PointerActions

  • Document windowHandle parameter in window management methods

  • Enable D417 Ruff rule enforcement for missing argument descriptions in init


Diagram Walkthrough

flowchart LR
  A["Docstring Updates"] --> B["Element Locators"]
  A --> C["Action Chains"]
  A --> D["Window Management"]
  A --> E["Service Classes"]
  F["Enable D417 Rule"] --> G["pyproject.toml"]
  B --> H["WebDriver, WebElement, ShadowRoot"]
  C --> I["ActionChains, PointerActions"]
  D --> J["set_window_size, set_window_position"]
  E --> K["IE Service"]
Loading

File Walkthrough

Relevant files
Documentation
action_chains.py
Document optional devices parameter in ActionChains           

py/selenium/webdriver/common/action_chains.py

  • Added documentation for the devices parameter in ActionChains
    constructor
  • Clarified that devices are optional and defaults will be created if
    not provided
+2/-0     
pointer_actions.py
Enhance PointerActions constructor docstring clarity         

py/selenium/webdriver/common/actions/pointer_actions.py

  • Enhanced PointerActions constructor docstring with summary line
  • Documented source parameter as optional with default behavior
  • Improved formatting and clarity of parameter descriptions
+6/-3     
service.py
Document kwargs parameter in IE Service                                   

py/selenium/webdriver/ie/service.py

  • Added documentation for **kwargs parameter in Service constructor
  • Clarifies that additional keyword arguments can be passed to parent
    Service class
+1/-0     
shadowroot.py
Document value parameter in ShadowRoot locators                   

py/selenium/webdriver/remote/shadowroot.py

  • Added value parameter documentation to find_element method
  • Added value parameter documentation to find_elements method
  • Clarifies the locator value usage with the specified by strategy
+2/-0     
webdriver.py
Document value and windowHandle parameters in WebDriver   

py/selenium/webdriver/remote/webdriver.py

  • Added value parameter documentation to find_element method
  • Added value parameter documentation to find_elements method
  • Added windowHandle parameter documentation to set_window_size method
  • Added windowHandle parameter documentation to set_window_position
    method
+4/-0     
webelement.py
Document value parameter in WebElement locators                   

py/selenium/webdriver/remote/webelement.py

  • Added value parameter documentation to find_element method
  • Added value parameter documentation to find_elements method
  • Clarifies the locator value usage with the specified by strategy
+2/-0     
Configuration changes
pyproject.toml
Enable D417 Ruff rule enforcement                                               

py/pyproject.toml

  • Removed D417 rule from the extend-ignore list in Ruff configuration
  • Enables enforcement of argument descriptions in __init__ docstrings
+0/-1     

@selenium-ci selenium-ci added the C-py Python Bindings label Oct 30, 2025
@iampopovich
Copy link
Contributor Author

@cgoldberg check please

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Oct 30, 2025

PR Compliance Guide 🔍

(Compliance updated until commit 7727420)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logs: The PR adds only docstring and linting changes without introducing logging for critical
actions, which may be acceptable given the PR scope but does not demonstrate audit trail
coverage.

Referred Code
        value: The locator value to use with the specified `by` strategy.

    Returns:
        The first matching WebElement found on the page.

    Example:
        element = driver.find_element(By.ID, 'foo')
    """
    by, value = self.locator_converter.convert(by, value)

    if isinstance(by, RelativeBy):
        elements = self.find_elements(by=by, value=value)
        if not elements:
            raise NoSuchElementException(f"Cannot locate relative element with: {by.root}")
        return elements[0]

    return self.execute(Command.FIND_ELEMENT, {"using": by, "value": value})["value"]

def find_elements(self, by=By.ID, value: Optional[str] = None) -> list[WebElement]:
    """Find elements given a By strategy and locator.



 ... (clipped 7 lines)
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
No error paths: The changes only modify docstrings and do not add or alter error handling for edge cases,
which may be acceptable for a doc-only PR but provides no evidence of robustness.

Referred Code
"""Initialize a new PointerActions instance.

Args:
    source: Optional PointerInput instance. If not provided, a default
        mouse PointerInput will be created.
    duration: Override the default 250 msecs of DEFAULT_MOVE_DURATION
        in the source.
"""
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
No input checks: Added docstrings for parameters like value without introducing or demonstrating input
validation, which may be acceptable for doc updates but does not evidence secure handling.

Referred Code
    value: The locator value to use with the specified `by` strategy.

Returns:
    The first matching `WebElement` found on the page.

Example:
    >>> element = driver.find_element(By.ID, "foo")
"""
if by == By.ID:
    by = By.CSS_SELECTOR
    value = f'[id="{value}"]'
elif by == By.CLASS_NAME:
    if value and any(char.isspace() for char in value.strip()):
        raise InvalidSelectorException("Compound class names are not allowed.")
    by = By.CSS_SELECTOR
    value = f".{value}"
elif by == By.NAME:
    by = By.CSS_SELECTOR
    value = f'[name="{value}"]'

return self._execute(Command.FIND_ELEMENT_FROM_SHADOW_ROOT, {"using": by, "value": value})["value"]


 ... (clipped 17 lines)
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit 7727420
Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

@qodo-merge-pro
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Handle potential None value for locator

Add a check in the find_element method to handle cases where the locator value
is None, preventing the generation of incorrect selectors by raising an
exception.

py/selenium/webdriver/remote/shadowroot.py [47-82]

 def find_element(self, by: str = By.ID, value: str = None):
     """Find an element inside a shadow root given a By strategy and
     locator.
 
     Args:
         by: The locating strategy to use. Default is `By.ID`. Supported values include:
             - By.ID: Locate by element ID.
             - By.NAME: Locate by the `name` attribute.
             - By.XPATH: Locate by an XPath expression.
             - By.CSS_SELECTOR: Locate by a CSS selector.
             - By.CLASS_NAME: Locate by the `class` attribute.
             - By.TAG_NAME: Locate by the tag name (e.g., "input", "button").
             - By.LINK_TEXT: Locate a link element by its exact text.
             - By.PARTIAL_LINK_TEXT: Locate a link element by partial text match.
             - RelativeBy: Locate elements relative to a specified root element.
         value: The locator value to use with the specified `by` strategy.
 
     Returns:
         The first matching `WebElement` found on the page.
 
     Example:
         >>> element = driver.find_element(By.ID, "foo")
     """
+    if value is None:
+        raise InvalidArgumentException("Locator value cannot be None")
     if by == By.ID:
         by = By.CSS_SELECTOR
         value = f'[id="{value}"]'
     elif by == By.CLASS_NAME:
-        if value and any(char.isspace() for char in value.strip()):
+        if any(char.isspace() for char in value.strip()):
             raise InvalidSelectorException("Compound class names are not allowed.")
         by = By.CSS_SELECTOR
         value = f".{value}"
     elif by == By.NAME:
         by = By.CSS_SELECTOR
         value = f'[name="{value}"]'
 
     return self._execute(Command.FIND_ELEMENT_FROM_SHADOW_ROOT, {"using": by, "value": value})["value"]

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a bug where passing None as the value argument leads to incorrect locator generation, and the proposed fix of raising an exception is a robust way to handle this invalid input.

Medium
Learned
best practice
Validate window size inputs

Validate that width and height are positive finite numbers before casting, and
raise ValueError with a clear message if invalid.

py/selenium/webdriver/remote/webdriver.py [942-954]

 def set_window_size(self, width, height, windowHandle: str = "current") -> None:
-    """Sets the width and height of the current window. (window.resizeTo)
+    """Sets the width and height of the current window. (window.resizeTo)"""
+    self._check_if_window_handle_is_current(windowHandle)
+    try:
+        w = int(width)
+        h = int(height)
+    except (TypeError, ValueError) as e:
+        raise ValueError(f"Invalid window size: width={width}, height={height}") from e
+    if w <= 0 or h <= 0:
+        raise ValueError(f"Window size must be positive: width={w}, height={h}")
+    self.set_window_rect(width=w, height=h)
 
-    Args:
-        width: The width in pixels to set the window to.
-        height: The height in pixels to set the window to.
-        windowHandle: The handle of the window to resize. Default is "current".
-
-    Example:
-        >>> driver.set_window_size(800, 600)
-    """
-    self._check_if_window_handle_is_current(windowHandle)
-    self.set_window_rect(width=int(width), height=int(height))
-

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Guard external API and I/O parameters with validation to surface clear errors.

Low
Clarify duration parameter semantics

Update the docstring to clarify the unit and constraints of duration and whether
non-positive values are allowed or clamped.

py/selenium/webdriver/common/actions/pointer_actions.py [27-35]

 def __init__(self, source: Optional[PointerInput] = None, duration: int = 250):
     """Initialize a new PointerActions instance.
 
     Args:
-        source: Optional PointerInput instance. If not provided, a default
-            mouse PointerInput will be created.
-        duration: Override the default 250 msecs of DEFAULT_MOVE_DURATION
-            in the source.
+        source: Optional PointerInput instance. If not provided, a default mouse PointerInput will be created.
+        duration: Move duration in milliseconds; must be a positive integer. Defaults to 250 ms.
     """
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Enforce accurate and consistent documentation to match actual behavior and APIs.

Low
  • More

@iampopovich
Copy link
Contributor Author

Just in case, I'll run tests in my pipeline ⏳

@cgoldberg cgoldberg changed the title [py] D417 Ruff warns fix for pydocstyle [py] Fix Ruff D417 warnings in docstrings Oct 31, 2025
Copy link
Member

@cgoldberg cgoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@cgoldberg cgoldberg merged commit bde7f06 into SeleniumHQ:trunk Oct 31, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants