[py] retry safaridriver startup in tests by titusfortner · Pull Request #17615 · SeleniumHQ/selenium

titusfortner · 2026-06-02T22:24:30Z

🔗 Related Issues

When Safari 26.5 was released ~May 20, the Python Safari job started consistently failing across multiple runs. When I ran the Ruby Safari tests, they were also failing, so I guarded everything.
Except things seem to be passing now, so I'm not sure what was going on previously. Some of those failures were connection related, so this PR addresses those.

💥 What does this PR do?

reverts the guards added to those 2 commits
implements a polling solution in python to match ruby's. We frequently get cascading errors when a safaridriver hasn't quite exited when we try to start a new one.

🔧 Implementation Notes

The Python retry (Driver._start_local_driver) attempts webdriver.<Driver>(...) up to DRIVER_START_RETRIES (3) times with a DRIVER_START_INTERVAL (1s) pause, logging a warning (with the error) on each retry. The Service is recreated from self.service on each attempt so a retry grabs a fresh free_port() and a clean subprocess rather than reusing a wedged one; self.service already resolves the correct Service class per driver.
Ruby has the equivalent pairing-retry from [rb] add safari tests #16768

🤖 AI assistance

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

💡 Additional Considerations

If the cascade reappears in CI, the retry warnings (surfaced via rerun-with-debug) will show the start-up errors and confirm whether a stability fix beyond retries is still needed.

🔄 Types of changes

Bug fix (backwards compatible)

qodo-code-review · 2026-06-02T22:24:55Z

Review Summary by Qodo

Implement SafariDriver startup retry mechanism and remove Safari 26.5 test guards

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Implement retry mechanism for SafariDriver startup to handle connection errors
  - Python: Added _start_local_driver() method with 3 retry attempts and 1s interval
  - Ruby: Moved test guards from describe blocks to individual test cases
• Remove Safari 26.5 regression test guards across Python test suite
  - Removed @pytest.mark.xfail_safari decorators from 60+ test functions
• Improve driver initialization robustness by recreating Service on each retry attempt

Diagram

flowchart LR
  A["SafariDriver Startup"] --> B["Retry Loop<br/>3 attempts"]
  B --> C["Create Service<br/>Get free port"]
  C --> D["Initialize Driver"]
  D -->|Success| E["Return Driver"]
  D -->|WebDriverException| F["Log Warning"]
  F --> G["Sleep 1s"]
  G -->|Retry Available| B
  G -->|Max Retries| H["Raise Exception"]

File Changes

1. py/conftest.py ✨ Enhancement +26/-3

Add retry logic for local driver initialization

py/conftest.py

2. py/test/selenium/webdriver/common/alerts_tests.py 🐞 Bug fix +0/-17

Remove Safari 26.5 regression test guards

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

3. py/test/selenium/webdriver/common/click_scrolling_tests.py 🐞 Bug fix +0/-6

Remove Safari 26.5 regression test guards

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

View more (22)

4. py/test/selenium/webdriver/common/click_tests.py 🐞 Bug fix +0/-2

Remove Safari 26.5 regression test guards

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

5. py/test/selenium/webdriver/common/correct_event_firing_tests.py 🐞 Bug fix +0/-1

Remove Safari 26.5 regression test guards

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

6. py/test/selenium/webdriver/common/element_attribute_tests.py 🐞 Bug fix +0/-2

Remove Safari 26.5 regression test guards

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

7. py/test/selenium/webdriver/common/executing_async_javascript_tests.py 🐞 Bug fix +0/-1

Remove Safari 26.5 regression test guards

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

8. py/test/selenium/webdriver/common/form_handling_tests.py 🐞 Bug fix +0/-8

Remove Safari 26.5 regression test guards

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

9. py/test/selenium/webdriver/common/frame_switching_tests.py 🐞 Bug fix +0/-8

Remove Safari 26.5 regression test guards

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

10. py/test/selenium/webdriver/common/implicit_waits_tests.py 🐞 Bug fix +0/-2

Remove Safari 26.5 regression test guards

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

11. py/test/selenium/webdriver/common/interactions_tests.py 🐞 Bug fix +0/-4

Remove Safari 26.5 regression test guards

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

12. py/test/selenium/webdriver/common/interactions_with_device_tests.py 🐞 Bug fix +0/-4

Remove Safari 26.5 regression test guards

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

13. py/test/selenium/webdriver/common/opacity_tests.py 🐞 Bug fix +0/-1

Remove Safari 26.5 regression test guards

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

14. py/test/selenium/webdriver/common/typing_tests.py 🐞 Bug fix +0/-6

Remove Safari 26.5 regression test guards

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

15. py/test/selenium/webdriver/common/w3c_interaction_tests.py 🐞 Bug fix +0/-4

Remove Safari 26.5 regression test guards

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

16. py/test/selenium/webdriver/common/webdriverwait_tests.py 🐞 Bug fix +0/-5

Remove Safari 26.5 regression test guards

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

17. py/test/selenium/webdriver/common/window_switching_tests.py 🐞 Bug fix +0/-3

Remove Safari 26.5 regression test guards

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

18. py/test/selenium/webdriver/support/event_firing_webdriver_tests.py 🐞 Bug fix +0/-1

Remove Safari 26.5 regression test guards

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

19. rb/spec/integration/selenium/webdriver/action_builder_spec.rb 🐞 Bug fix +14/-14

Move Safari guards from describe to individual tests

rb/spec/integration/selenium/webdriver/action_builder_spec.rb

20. rb/spec/integration/selenium/webdriver/driver_spec.rb 🐞 Bug fix +1/-1

Remove Safari 26.5 regression test guards

rb/spec/integration/selenium/webdriver/driver_spec.rb

21. rb/spec/integration/selenium/webdriver/element_spec.rb 🐞 Bug fix +6/-6

Remove Safari 26.5 regression test guards

rb/spec/integration/selenium/webdriver/element_spec.rb

22. rb/spec/integration/selenium/webdriver/navigation_spec.rb 🐞 Bug fix +2/-2

Remove Safari 26.5 regression test guards

rb/spec/integration/selenium/webdriver/navigation_spec.rb

23. rb/spec/integration/selenium/webdriver/target_locator_spec.rb 🐞 Bug fix +13/-15

Remove Safari 26.5 regression test guards

rb/spec/integration/selenium/webdriver/target_locator_spec.rb

24. rb/spec/integration/selenium/webdriver/timeout_spec.rb 🐞 Bug fix +2/-3

Remove Safari 26.5 regression test guards

rb/spec/integration/selenium/webdriver/timeout_spec.rb

25. rb/spec/integration/selenium/webdriver/window_spec.rb 🐞 Bug fix +2/-4

Remove Safari 26.5 regression test guards

rb/spec/integration/selenium/webdriver/window_spec.rb

selenium-ci · 2026-06-02T22:24:55Z

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 · 2026-06-02T22:24:56Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (2) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0)

1. ~~Retry catches too narrowly~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

Driver._start_local_driver retries only on WebDriverException, so any other exception type raised
during session creation (e.g., from the HTTP layer) will bypass the retry and fail immediately. This
undermines the intended stabilization for intermittent startup/connectivity failures.

Code

py/conftest.py[R471-475]

Evidence

The new code’s except WebDriverException is narrower than the set of exceptions that can propagate
during session creation: the HTTP request path has no local wrapping at the call site, and
RemoteWebDriver.start_session explicitly re-raises whatever exception occurred. Therefore,
non-WebDriverException failures will not be retried by _start_local_driver.

py/conftest.py[467-484]
py/selenium/webdriver/remote/remote_connection.py[427-438]
py/selenium/webdriver/remote/webdriver.py[388-403]

Agent prompt

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

## Issue description
`Driver._start_local_driver` only catches `WebDriverException`, but driver startup can raise other exception types during the initial NEW_SESSION request path (the HTTP request call site is not wrapped here, and `RemoteWebDriver.start_session` re-raises the original exception type). As a result, retries may not trigger for common startup-connect failures.

## Issue Context
`RemoteConnection._request` directly calls `urllib3` without catching exceptions, and `RemoteWebDriver.start_session` catches generic `Exception` only to stop the service and then re-raises the same exception.

## Fix Focus Areas
- py/conftest.py[467-484]

### Suggested approach
- Expand the retry handler to catch broader exceptions for local startup (e.g., `Exception` or a targeted tuple that includes `WebDriverException` plus networking errors), while still re-raising immediately on the final attempt.
- Keep the warning log including exception type/message to preserve debuggability.

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

2. Retries amplify startup timeout 🐞 Bug ➹ Performance

Description

Driver._start_local_driver retries full WebDriver construction up to 3 times, which can multiply
Service.start()'s built-in connect wait and turn a permanent startup failure into a 100s+ stall.
This can slow CI feedback and may trigger timeout/fault-handler behavior during driver startup
failures.

Code

py/conftest.py[R468-485]

Evidence

The new retry loop in py/conftest.py can re-run WebDriver construction up to 3 times. Selenium
Service.start() already includes a loop that can wait up to 70 iterations with increasing sleeps
capped at 0.5s (worst-case ~35s) before raising; multiplying this by 3 attempts can exceed 100s. The
repo’s pytest config also sets faulthandler_timeout = 60, increasing the likelihood of noisy
timeout dumps during these extended waits.

py/conftest.py[459-485]
py/selenium/webdriver/common/service.py[99-120]
py/pyproject.toml[76-80]

Agent prompt

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

### Issue description
`Driver._start_local_driver` retries full driver creation (`getattr(webdriver, self.driver_class)(**kwargs)`) up to 3 times. Each attempt may already spend significant time inside Selenium `Service.start()` waiting for connectability, so the retry can multiply that wait and stall a run for 100s+ on non-transient failures.

### Issue Context
This retry is intended to address intermittent SafariDriver startup/connectivity issues, but as implemented it applies to all local drivers and retries even when the underlying failure is permanent.

### Fix Focus Areas
- Consider scoping the retry to Safari only (or only to known-transient failure modes), to avoid multiplying waits for unrelated/permanent failures.
- Alternatively (or additionally), make retries configurable via pytest options/env vars and/or reduce retry count/interval.
- Consider failing fast on clearly permanent errors (e.g., missing/invalid executable path) while still retrying on connectability/HTTP-layer startup errors.

- py/conftest.py[459-485]
- py/selenium/webdriver/common/service.py[99-120]
- py/pyproject.toml[76-80]

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

3. Retry loop skips cleanup 📘 Rule violation ☼ Reliability

Description

On startup failure, Driver._start_local_driver retries after logging and sleeping but does not
perform exception-safe, bounded cleanup of the Service created for the failed attempt. This can
leak orphaned driver subprocesses/ports across retries and cause cascading port/connect instability
in resource-heavy test runs, especially when Service.start() spawns a process and then raises on
its connectability check without stopping it.

Code

py/conftest.py[R467-483]

Evidence

Rule 13 calls for exception-safe, bounded cleanup in resource-heavy paths, but the retry handler in
py/conftest.py catches WebDriverException and immediately logs and sleeps without tearing down
the per-attempt service. This matters because selenium.webdriver.common.service.Service.start()
starts a subprocess and then runs a connect loop that can time out and raise WebDriverException
without calling stop(), and some drivers (e.g., Safari) start their service before entering their
own session-creation try/except; therefore, without explicit caller-side cleanup, a failed attempt
can leave the spawned subprocess running and retries can accumulate orphaned processes and follow-on
port/connect failures.

py/conftest.py[467-483]
py/conftest.py[458-484]
py/conftest.py[430-436]
py/selenium/webdriver/common/service.py[99-120]
py/selenium/webdriver/safari/webdriver.py[42-60]
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
The retry path in `Driver._start_local_driver` retries driver creation after `WebDriverException` but does not stop/cleanup any `Service` started during a failed attempt. Because `Service.start()` can spawn a subprocess and then raise (e.g., failing its connectability check) without stopping it, repeated retries can leak orphaned driver processes/ports and trigger cascading connection/port failures.

## Issue Context
Driver startup typically spawns/owns subprocess resources. In `selenium.webdriver.common.service.Service.start()`, a subprocess is started and a connect loop runs; on timeout it raises `WebDriverException` without calling `stop()`. Several driver implementations call `service.start()` before their own try/except around session creation, so if `_start_local_driver` catches and retries without explicitly stopping the service for that attempt, the subprocess can remain running across attempts.

Suggested approach:
- Create an explicit `service` instance for each attempt (for all local drivers, not only when `--executable` is provided), pass it via `kwargs["service"]`, and on exception call `service.stop()` before sleeping/retrying.
- Ensure `kwargs` does not retain a stopped service between attempts (either overwrite each time or `del kwargs["service"]` after stopping).

## Fix Focus Areas
- py/conftest.py[458-484]

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

4. _start_local_driver lacks tests 📘 Rule violation ☼ Reliability

Description

The PR introduces new retry-and-sleep startup behavior in Driver._start_local_driver but does not
add any targeted test coverage to validate retry counts, sleep behavior, or the final failure path.
This risks regressions and flaky CI behavior if the retry logic changes or breaks silently.

Code

py/conftest.py[R303-308]

Evidence
Rule 6 requires adding/updating tests alongside behavioral changes when feasible. The diff adds new
retry constants and a retry loop with time.sleep and logging in Driver._start_local_driver, but
no corresponding test additions are present to assert the new behavior.
AGENTS.md: Add or update tests for implemented changes; prefer small unit tests and avoid mocks
py/conftest.py[303-483]

Agent prompt

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

## Issue description
`Driver._start_local_driver` adds retry logic that is not directly validated by tests (retry count, sleep interval, and raise-on-final-attempt behavior).

## Issue Context
This is test harness infrastructure used broadly by the Python binding tests; an unintended change to retry semantics can create widespread flakiness.

## Fix Focus Areas
- py/conftest.py[303-483]
- py/test/unit/selenium/webdriver/common/[add new test file validating retry behavior]

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

…tion

qodo-code-review · 2026-06-02T23:17:45Z

Code review by qodo was updated up to the latest commit 1b2d4c9

[py][rb] retry SafariDriver startup and remove Safari 26.5 test guards

3bb41ba

selenium-ci added C-py Python Bindings C-rb Ruby Bindings B-support Issue or PR related to support classes labels Jun 2, 2026

titusfortner changed the title ~~[py][rb] retry SafariDriver startup and remove Safari 26.5 test guards~~ [py] retry SafariDriver startup in tests Jun 2, 2026

titusfortner changed the title ~~[py] retry SafariDriver startup in tests~~ [py] retry safaridriver startup in tests Jun 2, 2026

titusfortner requested a review from cgoldberg June 2, 2026 22:28

qodo-code-review Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread py/conftest.py

[py] retry driver start on connection errors, not just WebDriverExcep…

1b2d4c9

…tion

titusfortner merged commit 702c97b into trunk Jun 3, 2026
53 checks passed

titusfortner deleted the safari_26_5 branch June 3, 2026 00:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[py] retry safaridriver startup in tests#17615

[py] retry safaridriver startup in tests#17615
titusfortner merged 2 commits into
trunkfrom
safari_26_5

titusfortner commented Jun 2, 2026

Uh oh!

qodo-code-review Bot commented Jun 2, 2026

Uh oh!

selenium-ci commented Jun 2, 2026

Uh oh!

qodo-code-review Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

qodo-code-review Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

titusfortner commented Jun 2, 2026

🔗 Related Issues

💥 What does this PR do?

🔧 Implementation Notes

🤖 AI assistance

💡 Additional Considerations

🔄 Types of changes

Uh oh!

qodo-code-review Bot commented Jun 2, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

selenium-ci commented Jun 2, 2026

Uh oh!

qodo-code-review Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

qodo-code-review Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

qodo-code-review Bot commented Jun 2, 2026 •

edited

Loading

qodo-code-review Bot commented Jun 2, 2026 •

edited

Loading