Skip to content

fix: preserve default locator strategy in WebDriver type hints - #1298

Merged
KazuCocoa merged 4 commits into
appium:masterfrom
MattL1989:fix/find-default-locator-types
Sep 10, 2026
Merged

fix: preserve default locator strategy in WebDriver type hints#1298
KazuCocoa merged 4 commits into
appium:masterfrom
MattL1989:fix/find-default-locator-types

Conversation

@MattL1989

Copy link
Copy Markdown
Contributor

The type-only find_element and find_elements declarations require by, although their inherited Selenium implementations default it to By.ID. This makes mypy reject valid calls such as driver.find_element(value='target') with a missing-argument error.

Add the inherited defaults and a consumer-level mypy regression covering omitted and explicit locator strategies while checking Appium WebElement return types. Runtime method dispatch is unchanged. This addresses a separate signature mismatch adjacent to #1147.

Validation on Windows with Python 3.12.14, Selenium 4.48.0 and mypy 2.3.1, using the repository's frozen development dependencies:

  • The new regression fails before the fix with two missing-by errors and passes afterward.
  • python -m pytest test/unit/: 187 passed (dependency deprecation and mocked-thread exception warnings emitted).
  • python -m mypy appium: 320 source files pass.
  • python -m ruff check . and python -m ruff format --check .: pass.

Prepared with AI assistance. No device-dependent functional tests were run; the correction affects static type declarations only.

Could you also confirm whether this correction qualifies for a contributor payment under the Appium compensation scheme, and the applicable tier and expected payment timing? I am evaluating a small paid contribution trial and would like to confirm those details before taking on further work.

@linux-foundation-easycla

linux-foundation-easycla Bot commented Sep 8, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

Comment thread appium/webdriver/webdriver.py Outdated
SessionNotCreatedException,
WebDriverException,
)
from selenium.webdriver.common.by import By

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I assume this import can be inside if TYPE_CHECKING:

if TYPE_CHECKING:
    from selenium.webdriver.common.by import By

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Valid, and safe. I checked the head commit: By appears only at line 23 (the import) and lines 285/288 and both stubs sit inside the class-level if TYPE_CHECKING: block, so By.ID is never evaluated at runtime.

@KazuCocoa

KazuCocoa commented Sep 9, 2026

Copy link
Copy Markdown
Member

The amount would be adjusted, but basically the policy will work if it meed S and larger one. It will be in a couple of months usually.

Comment thread test/unit/webdriver/type_hints_test.py Outdated
Address review feedback on appium#1298. The `By` import is only referenced by
the type-checking-only `find_element`/`find_elements` stubs, so guard it
with `if TYPE_CHECKING:` rather than importing it at runtime. Remove the
mypy-based regression test as requested.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@MattL1989

Copy link
Copy Markdown
Contributor Author

Both points addressed in 26824aaBy import moved under if TYPE_CHECKING: and the test removed. Re-ran ruff, mypy and the unit suite locally (186 passed). Thanks also for clarifying the compensation tiering and timing.

@KazuCocoa
KazuCocoa merged commit 0bbe39c into appium:master Sep 10, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants