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 type annotations

🔄 Types of changes

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

PR Type

Bug fix


Description

  • Fix mypy type errors

@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
@pallavigitwork
Copy link
Member Author

@cgoldberg kindly let me know if any more change is required?

@pallavigitwork
Copy link
Member Author

@cgoldberg kindly check, if any other changes required?

@pallavigitwork
Copy link
Member Author

Hi @cgoldberg is there any other change required, kindly let me know. thanks.

@cgoldberg cgoldberg changed the title [py] Fix mypy errors in switch_to file and relative locator file [py] Fix mypy errors in by file and exceptions file Oct 17, 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 now. Thanks for making those changes.

@cgoldberg cgoldberg merged commit d05680d into SeleniumHQ:trunk Oct 17, 2025
22 checks passed
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