[Dor-bl]

Selenium's default LocatorConverter rewrites id, name and class name into css selector (e.g. [id="foo"]), which Appium native contexts don't support. AppiumLocatorConverter exists to prevent that, but no unit test checked the request body the driver actually sends for these strategies.

This adds one parametrized test in test/unit/webdriver/webdriver_test.py that calls driver.find_element(by=..., value=...) and then element.find_element(...), and asserts the request body is exactly {'using': by, 'value': ...}.

It's a driver-level alternative to #28. Unlike #28, it also fails if the locator_converter=AppiumLocatorConverter() wiring in webdriver.py is removed.

Verification

• python -m pytest test/unit: 201 passed
• With locator_converter=AppiumLocatorConverter() commented out in appium/webdriver/webdriver.py, the new test fails, as expected
• ruff check / ruff format --check pass

🤖 Generated with Claude Code

https://claude.ai/code/session_012HCUdSWNJMojXCjBoxhg41

---

Generated by Claude Code

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Chores
  • Release publishing is now limited to the upstream project repository, preventing forks from publishing releases through this workflow.
• Tests
  • Added coverage confirming that element searches by ID, name, and class name preserve their locator details in requests, including searches within a parent element.

The release job now runs only in the upstream repository. Parameterized WebDriver tests check that element lookup requests preserve locator strategies and values.

Changes: Release workflow guard

Layer / File(s)	Summary
Restrict release job to upstream .github/workflows/publish.yml	The release job runs only when github.repository equals appium/python-client. Comments describe the upstream-only release policy and fork behavior.

Locator request tests

Layer / File(s)	Summary
Verify locator values in lookup requests test/unit/webdriver/webdriver_test.py	Parameterized tests check that top-level and child-element lookups preserve the supplied strategy and value for ID, NAME, and CLASS_NAME.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Possibly related PRs

• Dor-bl/python-client#19: This PR also restricts the release job to the upstream repository.
• Dor-bl/python-client#29: This PR also adds the same upstream-only release guard.

Suggested reviewers: kazucocoa

Merge Risk: Minimal · up to 93da6

No merge-blocking issue is identified in the release guard or locator tests.

Security architecture risk: Minimal · up to 93da6

The release restriction reduces exposure by skipping the entire release job outside the upstream repository. Upstream permissions and publication behavior remain unchanged, and the locator assertions add no production behavior. No material security risk was found introduced or worsened by these changes.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The affected authority concerns repository versioning and GitHub/PyPI publication. The change removes this workflow’s execution path in non-upstream repositories; it does not increase the authority or publication destinations of upstream runs.

Security Findings and Attack Paths

• observed — The routed locator test uses fixed locator values, an HTTP-mocking decorator, mocked session creation, and registered parent/child lookup responses. These additions exercise existing driver behavior rather than adding an attacker-facing request path.

Trust Boundaries and Controls

• observed — Repository identity is checked at the release-job boundary using the GitHub repository context. The condition covers both manual dispatch and master-branch push triggers and precedes token-consuming versioning and publication steps.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title names a test-only change that adds a parametrized driver-level test asserting that By.ID, By.NAME, and By.CLASS_NAME locators reach the HTTP request body unconverted. The changeset's test mo…
Description check		The description explains that Selenium's default LocatorConverter rewrites id, name, and class name locators into CSS selectors, and that the new parametrized test in webdriver_test.py asserts the req…
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names a test-only change that adds a parametrized driver-level test asserting that By.ID, By.NAME, and By.CLASS_NAME locators reach the HTTP request body unconverted. The changeset's test module adds exactly this test. The title follows the conventional 'test:' prefix and is specific, so the primary change is clear.

Full details: Description check

Explanation

The description explains that Selenium's default LocatorConverter rewrites id, name, and class name locators into CSS selectors, and that the new parametrized test in webdriver_test.py asserts the request body keeps the original strategy. This matches the changeset. It also mentions the release workflow fork guard only indirectly through the objectives, so it does not cover every file, which the lenient check does not require.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 91cd6a4c-b6ba-477f-b03d-a2dd1526951a

Commits

Reviewing files that changed from the base of the PR and between 3392743 and 93da620.

Files selected for processing: 2

• .github/workflows/publish.yml
• test/unit/webdriver/webdriver_test.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.