Skip to content

[py] mark Safari tests broken by SafariDriver 26.5 as xfail#17560

Merged
titusfortner merged 3 commits into
trunkfrom
safaridriver_py
May 24, 2026
Merged

[py] mark Safari tests broken by SafariDriver 26.5 as xfail#17560
titusfortner merged 3 commits into
trunkfrom
safaridriver_py

Conversation

@titusfortner
Copy link
Copy Markdown
Member

@titusfortner titusfortner commented May 23, 2026

The macos-15-arm64 runner image bumped Safari/SafariDriver from 26.3 to 26.5, which regressed alert handling (no such alert 404s) and synthetic click delivery across alerts, click, form, frame switching, interactions, typing, waits, and window switching tests.

💥 What does this PR do?

Mark the 72 affected tests xfail_safari so CI surfaces them as XFAIL rather than blocking merges; they will report XPASS once Apple ships a fix.

🔧 Implementation Notes

We could mark these as xfail strict to see when they start passing again

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): claude
    • What was generated:
    • I reviewed all AI output and can explain the change

@selenium-ci selenium-ci added C-py Python Bindings B-support Issue or PR related to support classes labels May 23, 2026
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Mark Safari tests broken by SafariDriver 26.5 regression as xfail

🧪 Tests

Grey Divider

Walkthroughs

Description
• Mark 72 Safari tests as xfail due to SafariDriver 26.5 regression
• Regression affects alert handling, click delivery, and interactions
• Tests span alerts, clicking, forms, frames, typing, waits, windows
• CI will report XFAIL instead of blocking merges until Apple fixes
Diagram
flowchart LR
  A["SafariDriver 26.5 Regression"] -->|affects| B["Alert Handling"]
  A -->|affects| C["Click Delivery"]
  A -->|affects| D["Form Interactions"]
  B -->|mark xfail| E["72 Tests"]
  C -->|mark xfail| E
  D -->|mark xfail| E
  E -->|result| F["XFAIL Status in CI"]

Loading

File Changes

1. py/test/selenium/webdriver/common/alerts_tests.py 🧪 Tests +17/-0

Mark 18 alert handling tests as xfail

py/test/selenium/webdriver/common/alerts_tests.py


2. py/test/selenium/webdriver/common/click_scrolling_tests.py 🧪 Tests +6/-0

Mark 8 click scrolling tests as xfail

py/test/selenium/webdriver/common/click_scrolling_tests.py


3. py/test/selenium/webdriver/common/click_tests.py 🧪 Tests +2/-0

Mark 2 click tests as xfail

py/test/selenium/webdriver/common/click_tests.py


View more (14)
4. py/test/selenium/webdriver/common/correct_event_firing_tests.py 🧪 Tests +1/-0

Mark 1 event firing test as xfail

py/test/selenium/webdriver/common/correct_event_firing_tests.py


5. py/test/selenium/webdriver/common/element_attribute_tests.py 🧪 Tests +2/-0

Mark 2 element attribute tests as xfail

py/test/selenium/webdriver/common/element_attribute_tests.py


6. py/test/selenium/webdriver/common/executing_async_javascript_tests.py 🧪 Tests +1/-0

Mark 1 async JavaScript test as xfail

py/test/selenium/webdriver/common/executing_async_javascript_tests.py


7. py/test/selenium/webdriver/common/form_handling_tests.py 🧪 Tests +8/-0

Mark 9 form handling tests as xfail

py/test/selenium/webdriver/common/form_handling_tests.py


8. py/test/selenium/webdriver/common/frame_switching_tests.py 🧪 Tests +8/-0

Mark 10 frame switching tests as xfail

py/test/selenium/webdriver/common/frame_switching_tests.py


9. py/test/selenium/webdriver/common/implicit_waits_tests.py 🧪 Tests +2/-0

Mark 2 implicit wait tests as xfail

py/test/selenium/webdriver/common/implicit_waits_tests.py


10. py/test/selenium/webdriver/common/interactions_tests.py 🧪 Tests +4/-0

Mark 5 interaction tests as xfail

py/test/selenium/webdriver/common/interactions_tests.py


11. py/test/selenium/webdriver/common/interactions_with_device_tests.py 🧪 Tests +4/-0

Mark 5 device interaction tests as xfail

py/test/selenium/webdriver/common/interactions_with_device_tests.py


12. py/test/selenium/webdriver/common/opacity_tests.py 🧪 Tests +1/-0

Mark 1 opacity test as xfail

py/test/selenium/webdriver/common/opacity_tests.py


13. py/test/selenium/webdriver/common/typing_tests.py 🧪 Tests +3/-0

Mark 4 typing tests as xfail

py/test/selenium/webdriver/common/typing_tests.py


14. py/test/selenium/webdriver/common/w3c_interaction_tests.py 🧪 Tests +4/-0

Mark 5 W3C interaction tests as xfail

py/test/selenium/webdriver/common/w3c_interaction_tests.py


15. py/test/selenium/webdriver/common/webdriverwait_tests.py 🧪 Tests +5/-0

Mark 5 WebDriverWait tests as xfail

py/test/selenium/webdriver/common/webdriverwait_tests.py


16. py/test/selenium/webdriver/common/window_switching_tests.py 🧪 Tests +3/-0

Mark 4 window switching tests as xfail

py/test/selenium/webdriver/common/window_switching_tests.py


17. py/test/selenium/webdriver/support/event_firing_webdriver_tests.py 🧪 Tests +1/-0

Mark 1 event firing webdriver test as xfail

py/test/selenium/webdriver/support/event_firing_webdriver_tests.py


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 23, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (2)

Grey Divider


Remediation recommended

1. Mass xfail_safari decorator churn 📘 Rule violation ⚙ Maintainability
Description
This PR adds the same @pytest.mark.xfail_safari(reason="SafariDriver 26.5 regression") decorator
across many separate test modules, creating a large mechanical diff that is harder to review,
revert, and maintain. This increases blast radius versus implementing a centralized, reversible
Safari-26.5 suppression mechanism.
Code

py/test/selenium/webdriver/common/alerts_tests.py[41]

Evidence
PR Compliance ID 3 requires avoiding broad mechanical churn. The diff shows repeated additions of
the same xfail_safari decorator across multiple independent test modules (e.g., alerts, click
scrolling, webdriver waits), indicating a cross-module mechanical change rather than a more
centralized, easily reversible approach.

AGENTS.md: Avoid repo-wide refactors or mass formatting changes; prefer small, reversible diffs
py/test/selenium/webdriver/common/alerts_tests.py[41-41]
py/test/selenium/webdriver/common/click_scrolling_tests.py[26-26]
py/test/selenium/webdriver/common/webdriverwait_tests.py[48-48]
py/test/selenium/webdriver/common/window_switching_tests.py[46-46]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR introduces broad mechanical churn by adding `@pytest.mark.xfail_safari(reason="SafariDriver 26.5 regression")` to many individual tests across many files.

## Issue Context
Compliance guidance prefers small, reversible diffs and avoiding mass mechanical edits when an equivalent centralized mechanism exists.

## Fix Focus Areas
- py/test/selenium/webdriver/safari/conftest.py[18-37]
- py/test/selenium/webdriver/common/alerts_tests.py[41-41]
- py/test/selenium/webdriver/common/click_scrolling_tests.py[26-26]
- py/test/selenium/webdriver/common/webdriverwait_tests.py[48-48]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Untracked Safari XFAIL reason 🐞 Bug ⚙ Maintainability
Description
The newly-added xfail_safari decorators use a generic reason string without a bug/issue reference,
which makes it difficult to audit why these tests are suppressed and when they should be removed.
This reduces long-term maintainability as Safari regressions evolve across versions/runners.
Code

py/test/selenium/webdriver/common/alerts_tests.py[41]

Evidence
The new Safari XFAIL reason is generic, while nearby and repo-wide XFAIL markers frequently include
concrete bug URLs/IDs, which improves traceability and cleanup discipline.

py/test/selenium/webdriver/common/alerts_tests.py[139-159]
py/test/selenium/webdriver/common/clear_tests.py[31-33]
py/test/selenium/webdriver/common/click_scrolling_tests.py[117-120]
py/test/selenium/webdriver/common/typing_tests.py[204-205]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
New `@pytest.mark.xfail_safari(reason="SafariDriver 26.5 regression")` markers don’t include a tracking reference (e.g., WebKit bug / Apple issue / internal ticket). This makes it hard to know what upstream issue they correspond to and when to remove them.

### Issue Context
The codebase already commonly embeds concrete links/IDs in `reason=` for XFAIL markers (e.g., chromedriver bugs, W3C issues, Mozilla bugzilla).

### Fix Focus Areas
- py/test/selenium/webdriver/common/alerts_tests.py[41-41]

### Suggested fix
Update the `reason=` string(s) to include the upstream tracking reference, e.g.:
- `reason="https://bugs.webkit.org/show_bug.cgi?id=XXXXX - SafariDriver 26.5 regression"`

Apply consistently to the other newly-added `xfail_safari` markers in this PR as well.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. xfail_safari lacks version condition 📘 Rule violation ☼ Reliability
Description
The new @pytest.mark.xfail_safari(reason="SafariDriver 26.5 regression") marks are unconditional
for all Safari runs even though the reason indicates the failures are SafariDriver-26.5-specific, so
they are not gated on the actual runtime/browser version or capability. This can permanently hide
failures (and mask recovery) after the upstream regression is fixed or the runner image changes
unless someone manually removes the markers.
Code

py/test/selenium/webdriver/common/alerts_tests.py[41]

Evidence
Rule 13 requires gating tests based on runtime/framework/browser feature support, but the PR adds
xfail_safari markers without any condition in the test files. In py/conftest.py, the harness
explicitly supports a condition kwarg for xfail_* markers and defaults it to True when
omitted, which means an xfail_safari marker that only provides a reason will always trigger for
Safari; combined with the version-specific reason string ("SafariDriver 26.5 regression"), this
demonstrates the xfails are currently unconditional rather than scoped to the affected SafariDriver
version.

py/test/selenium/webdriver/common/alerts_tests.py[41-41]
py/conftest.py[454-473]
py/conftest.py[460-466]
py/test/selenium/webdriver/common/click_tests.py[30-39]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New `@pytest.mark.xfail_safari(reason="SafariDriver 26.5 regression")` annotations are applied without a `condition=`, which makes them apply to *all* Safari runs even though the failures are described as specific to SafariDriver `26.5`. Update these markers (or the harness behavior) so they only `xfail` when the affected Safari/SafariDriver version (or capability/CI image) is detected, and automatically stop xfail’ing once the environment is no longer on SafariDriver 26.5.

## Issue Context
`py/conftest.py` supports a `condition` kwarg on `xfail_*` markers and defaults it to `True` when omitted, so any `xfail_safari` marker that only supplies a reason will always trigger on Safari. The PR adds multiple such unconditional markers with a version-specific reason string, which can mask recovery after the regression is fixed or the runner image changes.

## Fix Focus Areas
- py/test/selenium/webdriver/common/alerts_tests.py[41-46]
- py/test/selenium/webdriver/common/click_tests.py[30-39]
- py/conftest.py[431-473]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (1)
4. XFAIL prevents XPASS 🐞 Bug ◔ Observability
Description
xfail_safari is enforced by calling pytest.xfail() in the shared driver fixture, which aborts
during fixture setup so the test body never runs. With the newly-added decorators, these Safari
tests can never produce an XPASS signal when Apple fixes the regression.
Code

py/test/selenium/webdriver/common/alerts_tests.py[41]

Evidence
The new decorators add xfail_safari to many tests. The shared driver fixture checks for
xfail_<driver> markers and directly calls pytest.xfail(**kwargs), which terminates the test
during fixture setup (before the test body executes), eliminating the possibility of XPASS
reporting.

py/test/selenium/webdriver/common/alerts_tests.py[41-46]
py/conftest.py[431-473]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Tests marked with `@pytest.mark.xfail_safari` are currently xfailed by calling `pytest.xfail(**kwargs)` inside the `driver` fixture. This prevents the test body from executing, which means these tests cannot ever surface an XPASS when SafariDriver is fixed.

## Issue Context
The PR’s intent mentions "they will report XPASS once Apple ships a fix", but the current mechanism stops execution before assertions/actions occur.

## Fix Focus Areas
- py/conftest.py[431-473]

## Suggested fix
In `py/conftest.py` (inside the `driver` fixture), replace the `pytest.xfail(**kwargs)` call with adding a real pytest xfail marker to the node, so the test still runs:
- Build `kwargs` from the `xfail_*` marker as today.
- If `run=False`, keep the current skip behavior.
- Otherwise do something like `request.node.add_marker(pytest.mark.xfail(**kwargs))` and continue to `yield selenium_driver.driver`.
This preserves XFAIL reporting while allowing XPASS detection when the underlying issue is resolved.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit a6c7697

Results up to commit dda8362


🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0)


Remediation recommended
1. xfail_safari lacks version condition 📘 Rule violation ☼ Reliability
Description
The new @pytest.mark.xfail_safari(reason="SafariDriver 26.5 regression") marks are unconditional
for all Safari runs even though the reason indicates the failures are SafariDriver-26.5-specific, so
they are not gated on the actual runtime/browser version or capability. This can permanently hide
failures (and mask recovery) after the upstream regression is fixed or the runner image changes
unless someone manually removes the markers.
Code

py/test/selenium/webdriver/common/alerts_tests.py[41]

Evidence
Rule 13 requires gating tests based on runtime/framework/browser feature support, but the PR adds
xfail_safari markers without any condition in the test files. In py/conftest.py, the harness
explicitly supports a condition kwarg for xfail_* markers and defaults it to True when
omitted, which means an xfail_safari marker that only provides a reason will always trigger for
Safari; combined with the version-specific reason string ("SafariDriver 26.5 regression"), this
demonstrates the xfails are currently unconditional rather than scoped to the affected SafariDriver
version.

py/test/selenium/webdriver/common/alerts_tests.py[41-41]
py/conftest.py[454-473]
py/conftest.py[460-466]
py/test/selenium/webdriver/common/click_tests.py[30-39]
Best Practice: Learned patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New `@pytest.mark.xfail_safari(reason="SafariDriver 26.5 regression")` annotations are applied without a `condition=`, which makes them apply to *all* Safari runs even though the failures are described as specific to SafariDriver `26.5`. Update these markers (or the harness behavior) so they only `xfail` when the affected Safari/SafariDriver version (or capability/CI image) is detected, and automatically stop xfail’ing once the environment is no longer on SafariDriver 26.5.

## Issue Context
`py/conftest.py` supports a `condition` kwarg on `xfail_*` markers and defaults it to `True` when omitted, so any `xfail_safari` marker that only supplies a reason will always trigger on Safari. The PR adds multiple such unconditional markers with a version-specific reason string, which can mask recovery after the regression is fixed or the runner image changes.

## Fix Focus Areas
- py/test/selenium/webdriver/common/alerts_tests.py[41-46]
- py/test/selenium/webdriver/common/click_tests.py[30-39]
- py/conftest.py[431-473]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. XFAIL prevents XPASS 🐞 Bug ◔ Observability
Description
xfail_safari is enforced by calling pytest.xfail() in the shared driver fixture, which aborts
during fixture setup so the test body never runs. With the newly-added decorators, these Safari
tests can never produce an XPASS signal when Apple fixes the regression.
Code

py/test/selenium/webdriver/common/alerts_tests.py[41]

Evidence
The new decorators add xfail_safari to many tests. The shared driver fixture checks for
xfail_<driver> markers and directly calls pytest.xfail(**kwargs), which terminates the test
during fixture setup (before the test body executes), eliminating the possibility of XPASS
reporting.

py/test/selenium/webdriver/common/alerts_tests.py[41-46]
py/conftest.py[431-473]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Tests marked with `@pytest.mark.xfail_safari` are currently xfailed by calling `pytest.xfail(**kwargs)` inside the `driver` fixture. This prevents the test body from executing, which means these tests cannot ever surface an XPASS when SafariDriver is fixed.

## Issue Context
The PR’s intent mentions "they will report XPASS once Apple ships a fix", but the current mechanism stops execution before assertions/actions occur.

## Fix Focus Areas
- py/conftest.py[431-473]

## Suggested fix
In `py/conftest.py` (inside the `driver` fixture), replace the `pytest.xfail(**kwargs)` call with adding a real pytest xfail marker to the node, so the test still runs:
- Build `kwargs` from the `xfail_*` marker as today.
- If `run=False`, keep the current skip behavior.
- Otherwise do something like `request.node.add_marker(pytest.mark.xfail(**kwargs))` and continue to `yield selenium_driver.driver`.
This preserves XFAIL reporting while allowing XPASS detection when the underlying issue is resolved.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 9e63a61


🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)


Remediation recommended
1. Untracked Safari XFAIL reason 🐞 Bug ⚙ Maintainability
Description
The newly-added xfail_safari decorators use a generic reason string without a bug/issue reference,
which makes it difficult to audit why these tests are suppressed and when they should be removed.
This reduces long-term maintainability as Safari regressions evolve across versions/runners.
Code

py/test/selenium/webdriver/common/alerts_tests.py[41]

Evidence
The new Safari XFAIL reason is generic, while nearby and repo-wide XFAIL markers frequently include
concrete bug URLs/IDs, which improves traceability and cleanup discipline.

py/test/selenium/webdriver/common/alerts_tests.py[139-159]
py/test/selenium/webdriver/common/clear_tests.py[31-33]
py/test/selenium/webdriver/common/click_scrolling_tests.py[117-120]
py/test/selenium/webdriver/common/typing_tests.py[204-205]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
New `@pytest.mark.xfail_safari(reason="SafariDriver 26.5 regression")` markers don’t include a tracking reference (e.g., WebKit bug / Apple issue / internal ticket). This makes it hard to know what upstream issue they correspond to and when to remove them.

### Issue Context
The codebase already commonly embeds concrete links/IDs in `reason=` for XFAIL markers (e.g., chromedriver bugs, W3C issues, Mozilla bugzilla).

### Fix Focus Areas
- py/test/selenium/webdriver/common/alerts_tests.py[41-41]

### Suggested fix
Update the `reason=` string(s) to include the upstream tracking reference, e.g.:
- `reason="https://bugs.webkit.org/show_bug.cgi?id=XXXXX - SafariDriver 26.5 regression"`

Apply consistently to the other newly-added `xfail_safari` markers in this PR as well.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 581ab0a


🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0)


Remediation recommended
1. Mass xfail_safari decorator churn 📘 Rule violation ⚙ Maintainability
Description
This PR adds the same @pytest.mark.xfail_safari(reason="SafariDriver 26.5 regression") decorator
across many separate test modules, creating a large mechanical diff that is harder to review,
revert, and maintain. This increases blast radius versus implementing a centralized, reversible
Safari-26.5 suppression mechanism.
Code

py/test/selenium/webdriver/common/alerts_tests.py[41]

Evidence
PR Compliance ID 3 requires avoiding broad mechanical churn. The diff shows repeated additions of
the same xfail_safari decorator across multiple independent test modules (e.g., alerts, click
scrolling, webdriver waits), indicating a cross-module mechanical change rather than a more
centralized, easily reversible approach.

AGENTS.md: Avoid repo-wide refactors or mass formatting changes; prefer small, reversible diffs
py/test/selenium/webdriver/common/alerts_tests.py[41-41]
py/test/selenium/webdriver/common/click_scrolling_tests.py[26-26]
py/test/selenium/webdriver/common/webdriverwait_tests.py[48-48]
py/test/selenium/webdriver/common/window_switching_tests.py[46-46]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR introduces broad mechanical churn by adding `@pytest.mark.xfail_safari(reason="SafariDriver 26.5 regression")` to many individual tests across many files.

## Issue Context
Compliance guidance prefers small, reversible diffs and avoiding mass mechanical edits when an equivalent centralized mechanism exists.

## Fix Focus Areas
- py/test/selenium/webdriver/safari/conftest.py[18-37]
- py/test/selenium/webdriver/common/alerts_tests.py[41-41]
- py/test/selenium/webdriver/common/click_scrolling_tests.py[26-26]
- py/test/selenium/webdriver/common/webdriverwait_tests.py[48-48]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

@selenium-ci
Copy link
Copy Markdown
Member

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

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 23, 2026

Persistent review updated to latest commit 9e63a61

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 24, 2026

Persistent review updated to latest commit f01aec4

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 24, 2026

Persistent review updated to latest commit e1dc860

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 24, 2026

Persistent review updated to latest commit 581ab0a

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 24, 2026

Persistent review updated to latest commit a6c7697

@titusfortner titusfortner merged commit 9d4ef39 into trunk May 24, 2026
40 checks passed
@titusfortner titusfortner deleted the safaridriver_py branch May 24, 2026 11:10
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants