[rb] Add target type param to devtools

[jpawlyn]

User description

Motivation and Context

Testing offline functionality of pages cached by a service worker in a Rails app entailed using a monkey patch. Ideally the code from the monkey patch could be added to the ruby gem which is what this PR does.

To test offline functionality for a service worker we want to be able to call the following:

page.driver.browser.devtools(target_type: 'service_worker').
  network.emulate_network_conditions(offline: true, latency: 0, download_throughput: 0, upload_throughput: 0)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement, Tests

---

Description

• Added target_type parameter to DevTools for flexibility.

• Enhanced DevTools initialization to support multiple target types.

• Improved error handling for unsupported target types.

• Added integration tests for new target_type functionality.

---

Changes walkthrough 📝

	Relevant files
Enhancement	driver.rb Update `quit` to handle multiple DevTools connections        rb/lib/selenium/webdriver/common/driver.rb Modified quit method to close all DevTools connections. +1/-1      has_devtools.rb Add `target_type` parameter to `devtools` method                  rb/lib/selenium/webdriver/common/driver_extensions/has_devtools.rb Added target_type parameter to devtools method. Updated DevTools initialization to support multiple target types. +4/-3      devtools.rb Enhance DevTools to support multiple target types                rb/lib/selenium/webdriver/devtools.rb Updated initialize to accept target_type. Modified start_session to handle different target types. Added error handling for unsupported target types. +7/-5
Tests	devtools_spec.rb Add integration tests for `target_type` in DevTools            rb/spec/integration/selenium/webdriver/devtools_spec.rb Added tests for default and custom target_type. Verified error handling for unsupported target_type. +20/-0    service_worker.html Add test page for service worker functionality                      common/src/web/service_worker.html Added a test page for service worker registration. Included JavaScript to register a service worker. +16/-0
Additional files	service-worker.js [link]

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[CLAassistant]

All committers have signed the CLA.

[jpawlyn]

@p0deje I've sketched out your suggestion and happy to adapt as necessary. Not sure about documentation as I couldn't find a place where it would go but I might have missed it.

[p0deje]

@jpawlyn That's pretty cool and exactly what I had in mind! I'll be happy to merge as long as CI passes.

[jpawlyn]

@jpawlyn That's pretty cool and exactly what I had in mind! I'll be happy to merge as long as CI passes.

Great. One minor improvement is possibly testing for an unknown target_type that is not service_worker just so we can remove the sleep. Will update that. And hopefully the Python tests will pass on a second run.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential Memory Leak The change to use @devtools&.values&.map(&:close) assumes all DevTools connections should be closed. Verify this is the intended behavior and won't cause issues if some connections need to remain open. @devtools&.values&.map(&:close) Error Handling The error message when a target type is not found could be more descriptive, perhaps including available target types to help users troubleshoot. raise Error::WebDriverError, "Target type '#{target_type}' not found" unless found_target

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Improve error handling The error handling in start_session will raise an exception if no target of the specified type is found, but it doesn't validate if targets is nil or empty, which could lead to a different error. Add a check to ensure targets exists before searching for the target type. rb/lib/selenium/webdriver/devtools.rb [84-91] def start_session(target_type:) targets = target.get_targets.dig('result', 'targetInfos') + raise Error::WebDriverError, "No targets available" if targets.nil? || targets.empty? + found_target = targets.find { |target| target['type'] == target_type } raise Error::WebDriverError, "Target type '#{target_type}' not found" unless found_target session = target.attach_to_target(target_id: found_target['targetId'], flatten: true) @session_id = session.dig('result', 'sessionId') end Apply this suggestion Suggestion importance[1-10]: 8 __ Why: The suggestion adds important validation to check if targets is nil or empty before attempting to find a specific target type. This prevents potential nil reference errors and provides a more descriptive error message, making debugging easier when no targets are available.	Medium
	✅ Fix DevTools connections closure Suggestion Impact: The commit implemented part of the suggestion by changing map to each for DevTools connections closure, but did not include the additional 'if @devtools' check that was suggested. code diff: - @devtools&.values&.map(&:close) + @devtools&.values&.each(&:close) The current implementation of the quit method might not properly close all DevTools connections if @devtools is initialized but empty. Use each instead of map to ensure proper closure of all connections. rb/lib/selenium/webdriver/common/driver.rb [187-192] def quit bridge.quit ensure @service_manager&.stop - @devtools&.values&.map(&:close) + @devtools&.values&.each(&:close) if @devtools end [Suggestion has been applied] Suggestion importance[1-10]: 7 __ Why: The suggestion improves error handling by ensuring DevTools connections are properly closed even if @devtools is initialized but empty. Using each instead of map is more appropriate since we're not collecting return values, and adding the if @devtools check provides additional safety.	Medium
Learned best practice	Add parameter validation to ensure required parameters are not nil before using them Add parameter validation for url and target_type to ensure they are not nil before using them. In Ruby, you should raise an ArgumentError with a descriptive message when required parameters are nil. rb/lib/selenium/webdriver/devtools.rb' [31-35] def initialize(url:, target_type:) + raise ArgumentError, "url cannot be nil" if url.nil? + raise ArgumentError, "target_type cannot be nil" if target_type.nil? + @ws = WebSocketConnection.new(url: url) @session_id = nil start_session(target_type: target_type) end [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6	Low
Update

[p0deje]

Can you please fix?

rb/lib/selenium/webdriver/common/driver.rb:191:20: C: [Correctable] Style/HashEachMethods: Use each_value instead of values&.each.
        @devtools&.values&.each(&:close)