[rb] support WebDriver BiDi on Safari Preview and move #bidi onto Driver - #17729
Conversation
PR Summary by Qodo[rb] Add WebDriver BiDi support for Safari Preview; move #bidi to base Driver
AI Description
Diagram
High-Level Assessment
Files changed (20)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
19 rules 1.
|
|
Code review by qodo was updated up to the latest commit 1eac820 |
|
Code review by qodo was updated up to the latest commit 3aad87f |
|
Code review by qodo was updated up to the latest commit 0bcc302 |
There was a problem hiding this comment.
Pull request overview
Adds Safari Technology Preview coverage for WebDriver BiDi in the Ruby bindings by enabling Safari’s vendor BiDi capability, moving #bidi onto the base Driver, and wiring Bazel/RSpec so BiDi specs can run (with Safari-specific pendings where domains are unsupported).
Changes:
- Enable Safari BiDi by setting both
webSocketUrlandsafari:experimentalWebSocketUrl, with an “experimental” warning. - Expose
Driver#bididirectly (and validate returnedwebSocketUrlfor remote BiDi sessions). - Update Bazel/RSpec integration to generate/run BiDi variants and mark known Safari gaps as pending.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rb/spec/unit/selenium/webdriver/safari/options_spec.rb | Adds unit coverage for Safari BiDi capability enablement and warning behavior. |
| rb/spec/unit/selenium/webdriver/remote/bidi_bridge_spec.rb | New unit tests for clearer failures when remote webSocketUrl is invalid. |
| rb/spec/tests.bzl | Adds a bidi flag and Safari Preview BiDi metadata for generating BiDi test targets. |
| rb/spec/integration/selenium/webdriver/spec_support/test_environment.rb | Enables web_socket_url automatically for Safari Preview when WEBDRIVER_BIDI is set. |
| rb/spec/integration/selenium/webdriver/network_spec.rb | Marks Network specs pending on Safari where the BiDi network domain is unsupported. |
| rb/spec/integration/selenium/webdriver/navigation_spec.rb | Marks BiDi traverseHistory-dependent navigation test pending on Safari. |
| rb/spec/integration/selenium/webdriver/BUILD.bazel | Switches BiDi wiring from a tag to the new bidi = True flag. |
| rb/spec/integration/selenium/webdriver/bidi/script_spec.rb | Marks console log-entry BiDi tests pending on Safari due to missing event delivery. |
| rb/spec/integration/selenium/webdriver/bidi/network_spec.rb | Marks BiDi network-domain tests pending on Safari and updates combined pending conditions. |
| rb/spec/integration/selenium/webdriver/bidi/browsing_context_spec.rb | Expands pending coverage for Safari-specific BiDi behavior differences. |
| rb/spec/integration/selenium/webdriver/bidi/browser_spec.rb | Marks user-context and client-window BiDi tests pending on Safari. |
| rb/spec/integration/selenium/webdriver/bidi_spec.rb | Marks session.status-dependent tests pending on Safari due to differing readiness reporting. |
| rb/sig/lib/selenium/webdriver/remote/bidi_bridge.rbs | Adds RBS signature for validated_socket_url. |
| rb/sig/lib/selenium/webdriver/common/driver.rbs | Adds RBS signature for Driver#bidi. |
| rb/sig/lib/selenium/webdriver/common/driver_extensions/has_bidi.rbs | Removes the HasBiDi extension signature. |
| rb/lib/selenium/webdriver/safari/options.rb | Implements Safari-specific BiDi enablement (vendor capability + warning). |
| rb/lib/selenium/webdriver/remote/bidi_bridge.rb | Validates remote webSocketUrl and raises a clearer error for invalid values. |
| rb/lib/selenium/webdriver/firefox/driver.rb | Removes HasBiDi from Firefox driver extension list now that Driver#bidi exists. |
| rb/lib/selenium/webdriver/common/options.rb | Auto-calls enable_bidi! during options initialization when web_socket_url is set. |
| rb/lib/selenium/webdriver/common/driver.rb | Adds Driver#bidi delegating to the underlying bridge. |
| rb/lib/selenium/webdriver/common/driver_extensions/has_bidi.rb | Removes the HasBiDi extension implementation. |
| rb/lib/selenium/webdriver/common.rb | Drops the require for the removed HasBiDi extension file. |
| rb/lib/selenium/webdriver/chromium/driver.rb | Removes HasBiDi from Chromium driver extension list now that Driver#bidi exists. |
|
Code review by qodo was updated up to the latest commit 5b27335 |
|
Code review by qodo was updated up to the latest commit 8f5d12e |
|
Code review by qodo was updated up to the latest commit d2787e5 |
|
Code review by qodo was updated up to the latest commit 417ee5a |
|
Code review by qodo was updated up to the latest commit b781119 |
|
Code review by qodo was updated up to the latest commit 8303927 |
|
Code review by qodo was updated up to the latest commit 5aa169f |
diemol
left a comment
There was a problem hiding this comment.
If we are sure that removing HasBiDi won't break anything because in theory is not being used, then we should go for it.
5aa169f to
def6acd
Compare
|
Code review by qodo was updated up to the latest commit def6acd |
🔗 Related Issues
💥 What does this PR do?
#bidion baseDriver; improved error messages for bidi not supported🔧 Implementation Notes
webSocketUrland the vendorsafari:experimentalWebSocketUrlcapabilities are set, soSafari::Options#enable_bidi!now sets both.DriverExtensions::HasBiDiis removed outright (no deprecation shim). It's a public constant, so flagging in case a shim is preferred.web_socket_url: trueusesenable_bidi!to support different subclass behavior.🤖 AI assistance
💡 Additional Considerations
🔄 Types of changes