[rb] support WebDriver BiDi on Safari Preview and move #bidi onto Driver#17729
[rb] support WebDriver BiDi on Safari Preview and move #bidi onto Driver#17729titusfortner wants to merge 5 commits into
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 e032a44 |
|
Code review by qodo was updated up to the latest commit 584be91 |
584be91 to
fe95e14
Compare
|
Code review by qodo was updated up to the latest commit fe95e14 |
|
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.
🔗 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