[rb] add ClientConfig for HTTP client customization - #17699
Conversation
PR Summary by QodoAdd ClientConfig to centralize Ruby HTTP client customization Description
Diagram
High-Level Assessment
Files changed (37)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
19 rules 1. Redirect leaks extra headers
|
|
Code review by qodo was updated up to the latest commit 8f5d153 |
There was a problem hiding this comment.
Pull request overview
Adds a Ruby ClientConfig object to centralize HTTP client settings (timeouts, redirects, proxy, headers, user-agent, server URL), and refactors driver/bridge construction so the Bridge only consumes a configured http_client.
Changes:
- Introduces
Selenium::WebDriver::ClientConfigwith defaults (including read timeout 120s) and environment proxy support. - Updates local + remote driver constructors to accept
client_config:(mutually exclusive withhttp_client:) and to pass only an HTTP client into the Bridge. - Updates unit tests and RBS signatures for the new constructor shapes and config behavior.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| rb/spec/unit/selenium/webdriver/safari/service_spec.rb | Updates expected error message when :url is provided for a local driver. |
| rb/spec/unit/selenium/webdriver/remote/http/default_spec.rb | Updates timeout defaults and adds coverage for ClientConfig + redirect limits. |
| rb/spec/unit/selenium/webdriver/remote/http/common_spec.rb | Shifts header/user-agent expectations to ClientConfig defaults. |
| rb/spec/unit/selenium/webdriver/remote/features_spec.rb | Updates Bridge construction to rely on http_client.server_url. |
| rb/spec/unit/selenium/webdriver/remote/driver_spec.rb | Adds coverage for client_config in remote driver initialization and conflicts. |
| rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb | Updates Bridge initialization expectations (now http_client: only). |
| rb/spec/unit/selenium/webdriver/ie/service_spec.rb | Updates expected error message when :url is provided for a local driver. |
| rb/spec/unit/selenium/webdriver/firefox/service_spec.rb | Updates expected error message when :url is provided for a local driver. |
| rb/spec/unit/selenium/webdriver/edge/service_spec.rb | Updates expected error message when :url is provided for a local driver. |
| rb/spec/unit/selenium/webdriver/chrome/service_spec.rb | Updates expected error message when :url is provided for a local driver. |
| rb/spec/unit/selenium/webdriver/chrome/driver_spec.rb | Adds coverage for rejecting client_config.server_url on local drivers and conflict handling. |
| rb/sig/lib/selenium/webdriver/safari/driver.rbs | Updates Safari driver initializer signature with http_client/client_config. |
| rb/sig/lib/selenium/webdriver/remote/http/default.rbs | Updates Http::Default initializer and accessors signatures. |
| rb/sig/lib/selenium/webdriver/remote/http/common.rbs | Updates Http::Common to accept client_config: and return URI server_url. |
| rb/sig/lib/selenium/webdriver/remote/driver.rbs | Updates Remote driver initializer signature with http_client/client_config. |
| rb/sig/lib/selenium/webdriver/remote/bridge.rbs | Updates Bridge initializer signature (now only http_client:). |
| rb/sig/lib/selenium/webdriver/ie/driver.rbs | Updates IE driver initializer signature with http_client/client_config. |
| rb/sig/lib/selenium/webdriver/firefox/driver.rbs | Updates Firefox driver initializer signature with http_client/client_config. |
| rb/sig/lib/selenium/webdriver/edge/driver.rbs | Updates Edge driver initializer signature with http_client/client_config. |
| rb/sig/lib/selenium/webdriver/common/local_driver.rbs | Updates LocalDriver helper signatures for new parameters. |
| rb/sig/lib/selenium/webdriver/common/driver.rbs | Updates create_bridge signature (drops url:). |
| rb/sig/lib/selenium/webdriver/common/client_config.rbs | Adds RBS for the new ClientConfig class. |
| rb/sig/lib/selenium/webdriver/chrome/driver.rbs | Updates Chrome driver initializer signature with http_client/client_config. |
| rb/lib/selenium/webdriver/safari/driver.rb | Passes http_client through local driver initialization into base Driver. |
| rb/lib/selenium/webdriver/remote/http/default.rb | Refactors HTTP default client to be backed by ClientConfig and to use config for redirects/proxy/timeouts. |
| rb/lib/selenium/webdriver/remote/http/common.rb | Moves per-instance headers/user-agent and server URL storage into ClientConfig. |
| rb/lib/selenium/webdriver/remote/driver.rb | Allows client_config: to build/configure the HTTP client and enforces exclusivity with http_client:. |
| rb/lib/selenium/webdriver/remote/bridge.rb | Removes url: from Bridge initialization; uses provided configured HTTP client. |
| rb/lib/selenium/webdriver/ie/driver.rb | Passes http_client through local driver initialization into base Driver. |
| rb/lib/selenium/webdriver/firefox/driver.rb | Passes http_client through local driver initialization into base Driver. |
| rb/lib/selenium/webdriver/edge/driver.rb | Passes http_client through local driver initialization into base Driver. |
| rb/lib/selenium/webdriver/common/local_driver.rb | Builds/configures HTTP client for local services and validates url/config conflicts. |
| rb/lib/selenium/webdriver/common/driver.rb | Updates bridge creation to only pass http_client:. |
| rb/lib/selenium/webdriver/common/client_config.rb | Adds the new ClientConfig implementation and defaults. |
| rb/lib/selenium/webdriver/common.rb | Requires the new ClientConfig. |
| rb/lib/selenium/webdriver/chrome/driver.rb | Passes http_client through local driver initialization into base Driver. |
| rb/.rubocop.yml | Updates RuboCop parameter list metrics config. |
|
Code review by qodo was updated up to the latest commit 06b7fd2 |
06b7fd2 to
46f2685
Compare
|
Code review by qodo was updated up to the latest commit 46f2685 |
46f2685 to
4848282
Compare
|
Code review by qodo was updated up to the latest commit 4848282 |
4848282 to
56cbcc9
Compare
|
Code review by qodo was updated up to the latest commit 56cbcc9 |
🔗 Related Issues
Implements Ruby portion of #12368 and replaces previous implementation #16486
💥 What does this PR do?
ClientConfigfor Ruby that centralizes HTTP transport settings for: open/read timeouts, max redirects, proxy, extra headers, user-agent, and server URLClientConfigowns the defaults🔧 Implementation Notes
Dataclass, but that's the wrong approach; this should take the same approach as Options allowing setting values after creation before usage🤖 AI assistance
💡 Additional Considerations
Http::Commonclass methods in favor of ClientConfig class methodsClientConfigis positioned to carry the WebSocket settings for the BiDi follow-up.🔄 Types of changes