fix flaky Ruby test devtools_spec.rb

[asolntsev]

User description

Wait until service worker gets registered. Sometimes it's slow and can take more than 0.5 seconds.

Example of failure:

Failures:

  1) Selenium::WebDriver::DevTools#target target type is service_worker
     Failure/Error: target = driver.devtools(target_type: 'service_worker').target

     Selenium::WebDriver::Error::WebDriverError:
       Target type 'service_worker' not found
     # ./rb/lib/selenium/webdriver/devtools.rb:87:in `start_session'
     # ./rb/lib/selenium/webdriver/devtools.rb:34:in `initialize'
     # ./rb/lib/selenium/webdriver/common/driver_extensions/has_devtools.rb:36:in `new'
     # ./rb/lib/selenium/webdriver/common/driver_extensions/has_devtools.rb:36:in `devtools'
     # ./rb/spec/integration/selenium/webdriver/devtools_spec.rb:66:in `block (3 levels) in <module:WebDriver>'

@jpawlyn FYI

🔗 Related Issues

For example, this build failed:
https://github.com/SeleniumHQ/selenium/actions/runs/19984104789/job/57315854174

💥 What does this PR do?

1. Waits until the service worker gets registered
2. Postpones the service worker registration for 100ms - it will make any similar test fail if developer forgets to wait. So we avoid flaky tests in future.
3. Instead of generic class WebDriverError "Target type 'unknown' not found", now a more specific subclass NoSuchTargetError "Target type 'unknown' not found" is thrown.

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix, Tests

---

Description

• Add NoSuchTargetError exception for missing DevTools targets

• Replace hardcoded sleep with proper wait mechanism for service worker registration

• Introduce wait_for_devtools_target helper for reliable target detection

• Delay service worker registration by 100ms to catch timing issues early

---

Diagram Walkthrough

flowchart LR
  A["Service Worker Registration"] -->|100ms delay| B["Delayed Initialization"]
  C["DevTools Target Lookup"] -->|NoSuchTargetError| D["Specific Exception"]
  E["Test Execution"] -->|wait_for_devtools_target| F["Reliable Wait Logic"]
  B --> F
  D --> F

Loading

File Walkthrough

	Relevant files
Enhancement	error.rb Add NoSuchTargetError exception class                                        rb/lib/selenium/webdriver/common/error.rb Added new NoSuchTargetError exception class for DevTools target lookup failures Provides specific error type instead of generic WebDriverError +7/-0      helpers.rb Add wait_for_devtools_target helper method                              rb/spec/integration/selenium/webdriver/spec_support/helpers.rb Added new wait_for_devtools_target helper method with 3-second timeout Uses Wait with ignore parameter to handle NoSuchTargetError gracefully Provides reusable mechanism for waiting on DevTools target availability +5/-0
Bug fix	devtools.rb Use NoSuchTargetError for target lookup                                    rb/lib/selenium/webdriver/devtools.rb Changed exception type from WebDriverError to NoSuchTargetError in start_session method Improves error specificity for target not found scenarios +1/-1
Tests	devtools_spec.rb Replace sleep with proper wait helper                                        rb/spec/integration/selenium/webdriver/devtools_spec.rb Replaced hardcoded sleep 0.5 with wait_for_devtools_target helper call Updated error expectation to match new NoSuchTargetError exception type Improves test reliability and readability +3/-2      service_worker.html Add 100ms delay to service worker registration                      common/src/web/service_worker.html Wrapped service worker registration in setTimeout with 100ms delay Intentionally slows down registration to expose timing issues in tests Ensures tests properly wait for service worker availability +7/-5

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit logs: The new error path for missing DevTools target does not record any audit log of the failure, making it unclear when such critical actions occur. Referred Code def start_session(target_type:) targets = target.get_targets.dig('result', 'targetInfos') found_target = targets.find { |target| target['type'] == target_type } raise Error::NoSuchTargetError, "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 Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Improve wait logic for efficiency Refactor wait_for_devtools_target to improve efficiency by polling for the target's existence using the existing DevTools connection, rather than repeatedly creating new sessions. rb/spec/integration/selenium/webdriver/spec_support/helpers.rb [100-103] def wait_for_devtools_target(target_type:) - wait = Wait.new(timeout: 3, ignore: Error::NoSuchTargetError) - wait.until { driver.devtools(target_type: target_type).target } + wait = Wait.new(timeout: 3) + wait.until do + targets = driver.devtools.target.get_targets.dig('result', 'targetInfos') + targets&.any? { |target| target['type'] == target_type } + end end Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies an inefficiency in the new helper method and proposes a more performant implementation that avoids creating a new DevTools session on each retry.	Medium
Learned best practice	Avoid hardcoded timeouts Avoid hardcoded magic timeout values; extract to a named constant and allow an override to prevent brittle tests. rb/spec/integration/selenium/webdriver/spec_support/helpers.rb [100-103] -def wait_for_devtools_target(target_type:) - wait = Wait.new(timeout: 3, ignore: Error::NoSuchTargetError) +DEVTOOLS_TARGET_TIMEOUT_SECONDS = 3 + +def wait_for_devtools_target(target_type:, timeout: DEVTOOLS_TARGET_TIMEOUT_SECONDS) + wait = Wait.new(timeout: timeout, ignore: Error::NoSuchTargetError) wait.until { driver.devtools(target_type: target_type).target } end Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Guard numeric time calculations and timeouts by clamping to safe bounds and using clear, configurable units.	Low
More