Skip to content

Conversation

pallavigitwork
Copy link
Member

@pallavigitwork pallavigitwork commented Sep 8, 2025

User description

🔗 Related Issues

#15697

💥 What does this PR do?

fixes mypy errors in two files
switch_to
and
relative_locator.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • Bug fix (backwards compatible)

PR Type

Bug fix


Description

  • Fix mypy type errors in switch_to.py and relative_locator.py

  • Convert frame reference to string in exception handling

  • Remove unused import and fix CSS selector string literal


Diagram Walkthrough

flowchart LR
  A["Type Errors"] --> B["String Conversion"]
  A --> C["Import Cleanup"]
  B --> D["NoSuchFrameException Fix"]
  C --> E["CSS Selector Fix"]
Loading

File Walkthrough

Relevant files
Bug fix
switch_to.py
Fix type error in exception handling                                         

py/selenium/webdriver/remote/switch_to.py

  • Convert frame_reference to string when raising NoSuchFrameException
+1/-1     
relative_locator.py
Clean up imports and fix selector reference                           

py/selenium/webdriver/support/relative_locator.py

  • Remove unused By import
  • Replace By.CSS_SELECTOR with string literal "css selector"
+2/-2     

@selenium-ci selenium-ci added C-py Python Bindings B-support Issue or PR related to support classes labels Sep 8, 2025
@selenium-ci
Copy link
Member

Thank you, @pallavigitwork 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.

After reviewing the change, unless it is a critical fix or a feature that is needed for Selenium
to work, we will likely close the PR.

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

Copy link
Contributor

qodo-merge-pro bot commented Sep 8, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

5678 - Partially compliant

Compliant requirements:

Non-compliant requirements:

  • Investigate and resolve "ConnectFailure" on multiple ChromeDriver instances.
  • Ensure subsequent instantiations do not log errors.
  • Provide workaround or code change to prevent connection issues.

Requires further human verification:

  • Reproduce the ConnectFailure on the specified environment and versions.
  • Runtime validation with multiple sequential ChromeDriver launches.

1234 - Partially compliant

Compliant requirements:

Non-compliant requirements:

  • Restore click() behavior to trigger JavaScript in href.
  • Validate behavior in Firefox.

Requires further human verification:

  • Browser testing to confirm click() triggers JS in href across supported versions.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Behavior Change

Converting frame_reference to string in the NoSuchFrameException may alter error messaging/context; confirm this aligns with expected exception semantics and downstream handlers that might rely on the original type or message format.

if isinstance(frame_reference, str):
    try:
        frame_reference = self._driver.find_element(By.ID, frame_reference)
    except NoSuchElementException:
        try:
            frame_reference = self._driver.find_element(By.NAME, frame_reference)
        except NoSuchElementException as exc:
            raise NoSuchFrameException(str(frame_reference)) from exc

self._driver.execute(Command.SWITCH_TO_FRAME, {"id": frame_reference})
Selector Semantics

Replacing By.CSS_SELECTOR with the string "css selector" in RelativeBy construction changes the key type; verify all consumers expect a string key and that enum-to-string conversion is consistent across codebase.

    if not tag_name:
        raise WebDriverException("tag_name can not be null")
    return RelativeBy({"css selector": tag_name})


def locate_with(by: ByType, using: str) -> "RelativeBy":
    """Start searching for relative objects your search criteria with By.
API Compatibility

Removing the By import may be fine, but ensure public API and type hints remain stable; validate that ByType is correct and available across supported Python versions and typing contexts.

from selenium.common.exceptions import WebDriverException
from selenium.webdriver.common.by import ByType
from selenium.webdriver.remote.webelement import WebElement

Copy link
Contributor

qodo-merge-pro bot commented Sep 8, 2025

PR Code Suggestions ✨

No code suggestions found for the PR.

@cgoldberg cgoldberg changed the title fixed mypy errors in switch_to file and relative locator file [py] Fix mypy errors in switch_to file and relative locator file Sep 8, 2025
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.

3 participants