Fix few more flaky ruby tests

[asolntsev]

User description

🔗 Related Issues

e.g. failed build: https://github.com/SeleniumHQ/selenium/actions/runs/19987626355/job/57324241360?pr=13739

💥 What does this PR do?

Fixes few flaky Ruby tests

🔧 Implementation Notes

This PR adds few "waits" to Ruby tests.

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix, Tests

---

Description

• Adds message_provider option to Wait class for better error messages

• Replaces hardcoded sleeps with proper wait conditions in Ruby tests

• Improves wait_for_element helper with timeout parameter and detailed error output

• Refactors devtools and element specs to use wait helpers instead of sleep

---

Diagram Walkthrough

flowchart LR
  A["Wait class"] -->|adds message_provider| B["Better error messages"]
  C["Test helpers"] -->|enhance wait_for_element| D["Timeout + detailed output"]
  E["Ruby tests"] -->|replace sleep with wait| F["Reliable test execution"]
  B --> G["Reduced flakiness"]
  D --> G
  F --> G

Loading

File Walkthrough

	Relevant files
Enhancement	wait.rb Add message_provider option to Wait class                                rb/lib/selenium/webdriver/common/wait.rb Adds message_provider option to constructor for custom error message generation Updates until method to call message_provider.call when provided Allows dynamic error messages based on application state at timeout +4/-1      helpers.rb Improve wait helpers with better error messages and flexibility rb/spec/integration/selenium/webdriver/spec_support/helpers.rb Enhances wait_for_element with optional timeout parameter and detailed error messages Adds message_provider lambda that includes page URL and HTML source on timeout Renames wait_for_new_url to wait_for_url with simplified logic Adds new wait_for_title helper method for title-based waits +13/-5
Bug fix	element_spec.rb Replace direct element finding with wait helpers                  rb/spec/integration/selenium/webdriver/element_spec.rb Replaces direct find_element calls with wait_for_element helper throughout Removes hardcoded sleep calls and replaces with wait_for_url helper Adds timeout parameter to wait_for_element for element click intercepted test Improves reliability of element interaction tests by ensuring elements are rendered +55/-47  devtools_spec.rb Remove sleeps and improve log event test reliability          rb/spec/integration/selenium/webdriver/devtools_spec.rb Removes hardcoded sleep calls between console.log executions Changes log collection to store only log.args[0] instead of full log object Replaces object attribute matching with direct value inclusion checks Adds wait.until condition to ensure all logs are collected before assertions +7/-13

---

[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: 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: No audit logs: The added test code performs actions and assertions without introducing any audit logging for critical actions, though as test code this may be acceptable and outside the scope of production audit trails. Referred Code it 'notifies about log messages' do logs = [] driver.on_log_event(:console) { |log| logs.push(log.args[0]) } driver.navigate.to url_for('javascriptPage.html') driver.execute_script("console.log('I like cheese');") driver.execute_script('console.log(true);') driver.execute_script('console.log(null);') driver.execute_script('console.log(undefined);') driver.execute_script('console.log(document);') wait.until { logs.include?(true) } wait.until { logs.include?(nil) } wait.until { logs.include?({ 'type' => 'undefined' }) } wait.until { logs.include?('I like cheese')} end 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: Minimal wait handling: The new wait_for_title helper lacks explicit error context on timeout, relying on generic wait exceptions which may hinder debugging though this may be standard in test utilities. Referred Code def wait_for_title(title:) wait = Wait.new(timeout: 5) wait.until { driver.title == title } end Learn more about managing compliance generic rules or creating your own custom rules
Update

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	Consolidate multiple waits into one Consolidate multiple wait.until calls into a single block. This can be achieved by defining an array of expected logs and waiting until all of them are present in the logs array. rb/spec/integration/selenium/webdriver/devtools_spec.rb [112-115] -wait.until { logs.include?(true) } -wait.until { logs.include?(nil) } -wait.until { logs.include?({ 'type' => 'undefined' }) } -wait.until { logs.include?('I like cheese')} +expected_logs = [true, nil, { 'type' => 'undefined' }, 'I like cheese'] +wait.until { (expected_logs - logs).empty? } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly proposes consolidating multiple wait.until calls into a single, more efficient and readable check, which is a good practice for improving test code quality.	Low
Learned best practice	Replace hardcoded wait timeout Use a shared, configurable timeout (and a shorter poll interval) instead of a hardcoded 5 seconds to keep waits consistent and adjustable across tests. rb/spec/integration/selenium/webdriver/spec_support/helpers.rb [105-108] -def wait_for_title(title:) - wait = Wait.new(timeout: 5) - wait.until { driver.title == title } +DEFAULT_WAIT_TIMEOUT = 10 # seconds + +def wait_for_title(title:, timeout: DEFAULT_WAIT_TIMEOUT, interval: 0.1) + Wait.new(timeout: timeout, interval: interval).until { driver.title == title } end Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Guard time-based waits by computing and clamping timeouts and centralizing timeout values to avoid brittle tests.	Low
Update

[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: 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: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Sensitive Exposure: The new wait_for_element message_provider includes full page HTML source and current URL in error messages, potentially exposing sensitive internal details to users of test output. Referred Code def wait_for_element(locator, timeout = 25) wait = Wait.new(timeout: timeout, ignore: Error::NoSuchElementError, message_provider: lambda { url = "page url: #{driver.current_url};\n" source = "page source: #{driver.find_element(css: 'body').attribute('innerHTML')}\n" "could not find element #{locator} in #{timeout} seconds;\n#{url}#{source}" }) wait.until { driver.find_element(locator) } end def wait_for_url(new_url) wait = Wait.new(timeout: 5) wait.until do driver.current_url.include?(new_url) end end 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: No Audit Logs: The added waiting logic and test helpers introduce no logging of critical actions, but since these are test utilities and not production-critical actions, applicability of audit trail requirements is unclear. Referred Code def initialize(opts = {}) @timeout = opts.fetch(:timeout, DEFAULT_TIMEOUT) @interval = opts.fetch(:interval, DEFAULT_INTERVAL) @message = opts[:message] @message_provider = opts[:message_provider] @ignored = Array(opts[:ignore] || Error::NoSuchElementError) end # # Wait until the given block returns a true value. # # @raise [Error::TimeoutError] # @return [Object] the result of the block # def until end_time = current_time + @timeout last_error = nil until current_time > end_time begin ... (clipped 17 lines) 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: Verbose Logging: Error messages constructed in message_provider capture and emit the entire page source and URL, which could be written to logs by the test framework and may include sensitive data depending on pages under test. Referred Code def wait_for_element(locator, timeout = 25) wait = Wait.new(timeout: timeout, ignore: Error::NoSuchElementError, message_provider: lambda { url = "page url: #{driver.current_url};\n" source = "page source: #{driver.find_element(css: 'body').attribute('innerHTML')}\n" "could not find element #{locator} in #{timeout} seconds;\n#{url}#{source}" }) wait.until { driver.find_element(locator) } end def wait_for_url(new_url) wait = Wait.new(timeout: 5) wait.until do driver.current_url.include?(new_url) end 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
Possible issue	Improve error message generation resilience Add begin/rescue blocks within the message_provider lambda to prevent crashes when fetching the current URL or page source for the error message. rb/spec/integration/selenium/webdriver/spec_support/helpers.rb [87-94] def wait_for_element(locator, timeout = 25) wait = Wait.new(timeout: timeout, ignore: Error::NoSuchElementError, message_provider: lambda { - url = "page url: #{driver.current_url};\n" - source = "page source: #{driver.find_element(css: 'body').attribute('innerHTML')}\n" + url = begin + "page url: #{driver.current_url};\n" + rescue StandardError + "page url: <not available>;\n" + end + source = begin + "page source: #{driver.page_source}\n" + rescue StandardError + "page source: <not available>\n" + end "could not find element #{locator} in #{timeout} seconds;\n#{url}#{source}" }) wait.until { driver.find_element(locator) } end Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the message_provider lambda can fail, which would mask the original timeout error, and proposes a robust solution using begin/rescue to make the error reporting more resilient.	Medium
Learned best practice	Safeguard message provider exceptions Protect calls to a custom message_provider so exceptions there don't hide the original timeout; rescue and fall back to a default message with a hint. rb/lib/selenium/webdriver/common/wait.rb [66-72] msg = if @message @message.dup elsif @message_provider - @message_provider.call + begin + @message_provider.call + rescue StandardError => e + "timed out after #{@timeout} seconds (message_provider failed: #{e.class}: #{e.message})" + end else "timed out after #{@timeout} seconds" end Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Pattern 1: Ensure resource acquisition in error-message hooks can't raise and mask the original timeout error.	Low
More