Skip to content

[py] Fix remaining mypy errors and enable CI for type checking#16837

Merged
iampopovich merged 1 commit intoSeleniumHQ:trunkfrom
cgoldberg:py-finish-type-hints
Jan 3, 2026
Merged

[py] Fix remaining mypy errors and enable CI for type checking#16837
iampopovich merged 1 commit intoSeleniumHQ:trunkfrom
cgoldberg:py-finish-type-hints

Conversation

@cgoldberg
Copy link
Member

@cgoldberg cgoldberg commented Jan 3, 2026

User description

🔗 Related Issues

Fixes #15697
Supersedes #16827

💥 What does this PR do?

This PR fixes the rest of the type annotation errors across the Python bindings. You can now run mypy (tox -e typecheck) and there are zero errors with the current rule set.

This PR also:

  • moves typing stubs for third party packages into actual package dependencies so they are available to users
  • updates the typing job in ci-py to fail if any type errors are encountered in PR's or in trunk

🔧 Implementation Notes

There are still some additional mypy rules we can enable in pyproject.toml to make type checking stricter and mandatory.

This PR also adds some instance variables on some Service classes to make them similar to the others, and cleans up docstrings and removes a deprecation messages that shouldn't be there.

🔄 Types of changes

  • Cleanup
  • Type Checking
  • Bug fix (backwards compatible)

PR Type

Bug fix, Tests


Description

  • Fix type annotation errors across Python bindings for mypy compliance

  • Add type stubs as dependencies and enable strict mypy rules

  • Store options and service as instance variables in WebDriver classes

  • Add null checks for global devtools module and command executor validation

  • Clean up docstrings and remove incorrect deprecation warnings


Diagram Walkthrough

flowchart LR
  A["Type Annotations"] -->|Fix errors| B["Mypy Compliance"]
  C["Type Stubs"] -->|Add as deps| D["Available to Users"]
  E["WebDriver Classes"] -->|Store as instance vars| F["Consistent Pattern"]
  G["Global Modules"] -->|Add null checks| H["Runtime Safety"]
  I["CI Configuration"] -->|Enable strict checks| J["Type Checking Enforced"]
Loading

File Walkthrough

Relevant files
Bug fix
16 files
remote_connection.py
Remove optional union from ignore_proxy parameter               
+1/-1     
webdriver.py
Update docstring and store options as instance variable   
+1/-2     
remote_connection.py
Remove optional union from ignore_proxy parameter               
+1/-1     
webdriver.py
Make parameters required and store options as instance variable
+9/-10   
remote_connection.py
Remove optional union from ignore_proxy parameter               
+1/-1     
webdriver.py
Store service and options as instance variables                   
+7/-10   
remote_connection.py
Remove optional union and add type ignore comment               
+2/-2     
webdriver.py
Store options as instance variable and update docstrings 
+9/-10   
webdriver.py
Store options as instance variable and clean docstrings   
+6/-7     
remote_connection.py
Fix type annotations and add validation for client config
+18/-17 
websocket_connection.py
Remove type ignore comment from websocket import                 
+1/-1     
remote_connection.py
Remove optional union from ignore_proxy parameter               
+1/-1     
webdriver.py
Reorder parameters and store options as instance variable
+8/-10   
event_firing_webdriver.py
Add type ignore comment for dynamic method assignment       
+2/-1     
webdriver.py
Add type annotations and exception handling                           
+10/-7   
webdriver.py
Add type annotations and exception handling                           
+10/-7   
Error handling
3 files
cdp.py
Add null checks for global devtools module                             
+6/-0     
webdriver.py
Add safety check before accessing service attribute           
+2/-2     
webdriver.py
Add null checks for CDP version and command executor validation
+12/-1   
Configuration changes
2 files
ci-python.yml
Make type checking fail on errors in CI                                   
+1/-1     
tox.ini
Update typecheck environment to use requirements                 
+3/-1     
Dependencies
4 files
BUILD.bazel
Add type stub packages as dependencies                                     
+5/-2     
pyproject.toml
Move type stubs to main dependencies and enable strict mypy rules
+8/-12   
requirements.txt
Add type stub packages to requirements                                     
+3/-0     
requirements_lock.txt
Update locked versions with type stub dependencies             
+24/-0   

@selenium-ci selenium-ci added C-py Python Bindings B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related B-support Issue or PR related to support classes labels Jan 3, 2026
@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

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

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

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

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

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

Learn more about managing compliance generic rules or creating your own custom rules

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

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

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

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

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

Status:
Audit logging absent: The diff adds new connection initialization/validation behavior but does not show any
corresponding audit logging for potentially critical actions, so audit-trail compliance
cannot be verified from the PR changes alone.

Referred Code
def __init__(
    self,
    remote_server_addr: str | None = None,
    keep_alive: bool = True,
    ignore_proxy: bool = False,
    ignore_certificates: bool | None = False,
    init_args_for_pool_manager: dict | None = None,
    client_config: ClientConfig | None = None,
):
    if client_config:
        self._client_config = client_config
    elif remote_server_addr:
        self._client_config = ClientConfig(
            remote_server_addr=remote_server_addr,
            keep_alive=keep_alive,
            ignore_certificates=ignore_certificates,
            init_args_for_pool_manager=init_args_for_pool_manager,
        )
    else:
        raise WebDriverException("Must provide either 'remote_server_addr' or 'client_config'")

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@iampopovich iampopovich merged commit 80b13e8 into SeleniumHQ:trunk Jan 3, 2026
29 checks passed
@cgoldberg cgoldberg deleted the py-finish-type-hints branch January 5, 2026 15:29
@cdce8p
Copy link

cdce8p commented Jan 19, 2026

  • moves typing stubs for third party packages into actual package dependencies so they are available to users

Hi @cgoldberg,

I'm trying to understand why you made types-certifi and types-urllib3 runtime dependencies. Which users will benefit from that move? The way I've understood stubs packages (so far) has been that they should only really be added as test / dev dependencies. The Python packaging system doesn't allow downstream consumers to overwrite dependencies. So adding a stubs package here will force any packages which requires selenium to also install these even if they don't really need to. In fact in Home Assistant we even have a CI check to make sure requirements don't install stubs packages as this is such a common mistake and just leads to dependency conflicts. That's how I noticed this change here as our CI checks are currently failing because of it.

Technically this even applies to typing_extensions though it's so ubiquitous in the Python ecosystem that it's not really an issue.

I'd recommend to move types-certifi and types-urllib3 back to the test dependencies.

--
Saw that someone just created an issue for it #16943.

@cgoldberg
Copy link
Member Author

Yea, that's fair. I'll move them to dev dependencies.

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

Labels

B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related B-support Issue or PR related to support classes C-py Python Bindings Review effort 3/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: [py] Many type annotation errors need fixing

4 participants