fix flaky Ruby test `element_spec.rb` #16709

asolntsev · 2025-12-09T09:30:32Z

User description

Wait until the new URL gets loaded.

Sometimes after executing driver.navigate.to url_for('new_page.html') the browser is still showing the previous page.

Example of failure:

Selenium::WebDriver::Element raises if different element receives click
     Failure/Error: element = wait_for_element(id: 'contents', timeout: 10)

     Selenium::WebDriver::Error::TimeoutError:
       could not find element {:id=>"contents", :timeout=>10} in 25 seconds;
       page url: http://localhost:49887/resultPage.html?x=25&y=25;
       page source:
       <p id="greeting">Success!</p> ...

     # ./rb/lib/selenium/webdriver/common/wait.rb:76:in `until'
     # /Users/runner/work/selenium/selenium/rb/spec/integration/selenium/webdriver/spec_support/helpers.rb:93:in `wait_for_element'
     # ./rb/spec/integration/selenium/webdriver/element_spec.rb:35:in `block (2 levels) in <module:WebDriver>'

See https://github.com/SeleniumHQ/selenium/actions/runs/20055175531/job/57520193372#step:19:920 for example.

💥 What does this PR do?

This PR adds a waiting for the new URL.

💡 Additional Considerations

Currently I did it only for one test file (that often fails).
Maybe it should be applied to all ruby tests...

🔄 Types of changes

Bug fix (backwards compatible)

PR Type

Bug fix

Description

Replace direct navigation with open_file helper for test isolation
Add blank page navigation to clear storage and stop background activity
Improve wait_for_url with longer timeout and better error messages
Create open_file helper method to standardize page loading in tests

Diagram Walkthrough

flowchart LR
  A["Test Navigation"] -->|"Replace with"| B["open_file Helper"]
  B -->|"Navigate to"| C["about:blank"]
  C -->|"Then to"| D["blank.html"]
  D -->|"Clear Storage"| E["localStorage/sessionStorage"]
  E -->|"Finally to"| F["Target Page"]
  F -->|"Wait for"| G["URL Confirmation"]

File Walkthrough

Relevant files

Bug fix

element_spec.rb `Standardize page navigation using open_file helper` rb/spec/integration/selenium/webdriver/element_spec.rb Replace all `driver.navigate.to url_for()` calls with new `open_file` helper method Standardize page loading across 30+ test cases to ensure proper test isolation Maintain all existing test logic and assertions unchanged	+33/-33

Enhancement

helpers.rb `Add open_file helper and improve wait timeouts` rb/spec/integration/selenium/webdriver/spec_support/helpers.rb Add new `open_file` helper method that navigates through blank pages before target page Increase `wait_for_url` timeout from 5 to 15 seconds with improved error messaging Increase `wait_for_title` timeout from 5 to 15 seconds for consistency Add message provider to `wait_for_url` for better debugging information	+12/-3
blank.html `Add storage clearing to blank page` common/src/web/blank.html Add JavaScript to clear localStorage and sessionStorage on blank page load Ensure clean state between tests by removing persisted data	+6/-1

qodo-code-review · 2025-12-09T09:30:58Z

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: 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 new helpers add navigation and waiting logic without emitting any audit logs for critical actions, but as this is test code, logging may be intentionally omitted. Referred Code def wait_for_url(new_url, timeout = 5) wait = Wait.new(timeout: timeout, message_provider: lambda { "could not wait for URL #{new_url} in #{timeout} seconds;\nactual page url: #{driver.current_url};\n" }) wait.until do driver.current_url.include?(new_url) end end def wait_for_devtools_target(target_type:) wait = Wait.new(timeout: 3, ignore: Error::NoSuchTargetError) wait.until { driver.devtools(target_type: target_type).target } end def wait_for_title(title:) wait = Wait.new(timeout: 5) wait.until { driver.title == title } end def open_file(file_name) driver.navigate.to url_for(file_name) ... (clipped 2 lines) 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: Verbose error data: Error messages include full page source and current URL in test waits, which could expose internal details if surfaced beyond test logs. 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, timeout = 5) wait = Wait.new(timeout: timeout, message_provider: lambda { "could not wait for URL #{new_url} in #{timeout} seconds;\nactual page url: #{driver.current_url};\n" }) wait.until do driver.current_url.include?(new_url) 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: Potential sensitive logs: The wait message providers log current URL and full page body HTML, which might contain sensitive data depending on test pages, though likely acceptable for test fixtures. 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, timeout = 5) wait = Wait.new(timeout: timeout, message_provider: lambda { "could not wait for URL #{new_url} in #{timeout} seconds;\nactual page url: #{driver.current_url};\n" }) wait.until do driver.current_url.include?(new_url) 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 · 2025-12-09T09:32:16Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	The fix introduces an inconsistent pattern The PR introduces a safer `open_file` helper for navigation but only applies it to one file. To ensure consistency and prevent future test flakiness, this helper should be used to replace all `driver.navigate.to` calls across the entire test suite. Examples: rb/spec/integration/selenium/webdriver/element_spec.rb [26] open_file 'formPage.html' Solution Walkthrough: Before: # In element_spec.rb (fixed file) describe Element do it 'clicks' do open_file 'formPage.html' # ... end end # In other_spec.rb (other unfixed files) describe OtherFeature do it 'does something' do driver.navigate.to url_for('somePage.html') # Old, flaky pattern remains # ... end end After: # In element_spec.rb describe Element do it 'clicks' do open_file 'formPage.html' # ... end end # In other_spec.rb (and all other test files) describe OtherFeature do it 'does something' do open_file 'somePage.html' # New, robust pattern applied consistently # ... end end Suggestion importance[1-10]: 8 __ Why: This is a valid and high-impact suggestion that correctly identifies a strategic flaw in the PR's scope; while the fix is correct, its limited application introduces inconsistency and leaves other tests vulnerable to the same flakiness.	Medium
General	Make helper timeout configurable Add a configurable `timeout` parameter to the `open_file` helper method and pass it to `wait_for_url` to make tests more robust in different environments. rb/spec/integration/selenium/webdriver/spec_support/helpers.rb [115-118] -def open_file(file_name) +def open_file(file_name, timeout: 5) driver.navigate.to url_for(file_name) - wait_for_url(file_name) + wait_for_url(file_name, timeout) end Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that the new `open_file` helper uses a hardcoded timeout, and making it configurable improves the flexibility and robustness of the test helper.	Low
Update

Wait until the new URL gets loaded. Sometimes after executing `driver.navigate.to url_for('new_page.html')` the browser is still showing the previous page. Example of failure: ```ruby Selenium::WebDriver::Element raises if different element receives click Failure/Error: element = wait_for_element(id: 'contents', timeout: 10) Selenium::WebDriver::Error::TimeoutError: could not find element {:id=>"contents", :timeout=>10} in 25 seconds; page url: http://localhost:49887/resultPage.html?x=25&y=25; page source: <p id="greeting">Success!</p> ... # ./rb/lib/selenium/webdriver/common/wait.rb:76:in `until' # /Users/runner/work/selenium/selenium/rb/spec/integration/selenium/webdriver/spec_support/helpers.rb:93:in `wait_for_element' # ./rb/spec/integration/selenium/webdriver/element_spec.rb:35:in `block (2 levels) in <module:WebDriver>' ``` See https://github.com/SeleniumHQ/selenium/actions/runs/20055175531/job/57520193372#step:19:920 for example.

It's a good practice to avoid tests affecting each other: 1. Open "about:blank" page - this stops any current activity / background requests / animations on the previous page 2. Open an empty page which clears sessionStorage, localStorage and cookies. This technique allows reusing the browser between tests, while keeping the tests independent.

qodo-code-review · 2025-12-10T09:50:04Z

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: 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 helper and wait methods perform navigation and state changes without emitting any audit logs, but this is a test suite where audit trails may be intentionally omitted. Referred Code def open_file(file_name) driver.navigate.to 'about:blank' driver.navigate.to url_for('blank.html') driver.navigate.to url_for(file_name) wait_for_url(file_name) end 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: Verbose errors: The wait_for_element message includes current URL and full page body innerHTML which could expose internal details, though this is test code not user-facing. 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 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: Potential PII logs: Error message providers capture the entire page body innerHTML and URLs which could include sensitive data depending on test pages, although likely safe in controlled test fixtures. 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 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 · 2025-12-10T09:51:22Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	The solution introduces significant performance overhead The `open_file` helper's three navigations per page load create a performance bottleneck. Instead, clear browser storage using `driver.execute_script` in a setup hook and simplify the helper to a single navigation and wait. Examples: rb/spec/integration/selenium/webdriver/spec_support/helpers.rb [115-120] def open_file(file_name) driver.navigate.to 'about:blank' driver.navigate.to url_for('blank.html') driver.navigate.to url_for(file_name) wait_for_url(file_name) end common/src/web/blank.html [1-6] <html><head><title>blank</title></head><body> <script> localStorage.clear(); sessionStorage.clear(); </script> </body></html> Solution Walkthrough: Before: # in helpers.rb def open_file(file_name) driver.navigate.to 'about:blank' driver.navigate.to url_for('blank.html') # This page clears storage driver.navigate.to url_for(file_name) wait_for_url(file_name) end # in element_spec.rb it 'does something' do open_file 'formPage.html' # ... test logic end After: # in a spec_helper or setup file before(:each) do driver.execute_script("localStorage.clear(); sessionStorage.clear();") end # in helpers.rb def open_file(file_name) driver.navigate.to url_for(file_name) wait_for_url(file_name) # The wait is still needed for flakiness end # in element_spec.rb it 'does something' do open_file 'formPage.html' # ... test logic end Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a significant performance bottleneck in the new `open_file` helper due to its three-step navigation, and proposes a much more efficient and standard approach using `execute_script` to achieve the same goal.	High
Possible issue	Use WebDriver API for storage clearing Revert the changes to `blank.html` and instead use WebDriver's built-in APIs to clear storage directly within the `open_file` helper method to prevent potential race conditions. common/src/web/blank.html [1-6] -<html><head><title>blank</title></head><body> -<script> - localStorage.clear(); - sessionStorage.clear(); -</script> -</body></html> +<html><head><title>blank</title></head><body></body></html> Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential race condition with the PR's approach and proposes a more robust and idiomatic solution using WebDriver's native APIs, which improves test reliability.	Medium
Possible issue	Improve test isolation using WebDriver API Refactor the `open_file` method to use `driver.local_storage.clear` and `driver.session_storage.clear` and remove the navigation to `blank.html` for a more reliable and direct way to clear storage. rb/spec/integration/selenium/webdriver/spec_support/helpers.rb [115-120] def open_file(file_name) driver.navigate.to 'about:blank' - driver.navigate.to url_for('blank.html') + driver.local_storage.clear + driver.session_storage.clear driver.navigate.to url_for(file_name) wait_for_url(file_name) end Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion provides a more robust and direct implementation for clearing storage by using WebDriver's native APIs, which is a significant improvement for test reliability over navigating to a page with a script.	Medium
More

rb/spec/integration/selenium/webdriver/spec_support/helpers.rb

selenium-ci added the C-rb Ruby Bindings label Dec 9, 2025

qodo-code-review bot added the Review effort 2/5 label Dec 9, 2025

asolntsev force-pushed the fix/flaky-ruby-test-v3 branch from 6cc333a to 122a21a Compare December 9, 2025 22:44

asolntsev marked this pull request as draft December 10, 2025 06:32

asolntsev force-pushed the fix/flaky-ruby-test-v3 branch from 122a21a to 718eb0d Compare December 10, 2025 06:33

asolntsev requested a review from p0deje December 10, 2025 09:48

asolntsev self-assigned this Dec 10, 2025

asolntsev added the ruby Pull requests that update ruby code label Dec 10, 2025

asolntsev marked this pull request as ready for review December 10, 2025 09:49

p0deje reviewed Dec 10, 2025

View reviewed changes

rb/spec/integration/selenium/webdriver/spec_support/helpers.rb Show resolved Hide resolved

p0deje approved these changes Dec 10, 2025

View reviewed changes

Merge branch 'trunk' into fix/flaky-ruby-test-v3

42080fe

asolntsev enabled auto-merge (squash) December 10, 2025 16:51

asolntsev merged commit 88a8296 into SeleniumHQ:trunk Dec 10, 2025
38 checks passed

asolntsev deleted the fix/flaky-ruby-test-v3 branch December 10, 2025 18:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix flaky Ruby test `element_spec.rb` #16709

fix flaky Ruby test `element_spec.rb` #16709

asolntsev commented Dec 9, 2025 •

edited by qodo-code-review bot

Loading

Uh oh!

qodo-code-review bot commented Dec 9, 2025 •

edited

Loading

Uh oh!

qodo-code-review bot commented Dec 9, 2025 •

edited

Loading

Examples:

Solution Walkthrough:

Before:

After:

Uh oh!

qodo-code-review bot commented Dec 10, 2025

Uh oh!

qodo-code-review bot commented Dec 10, 2025

Examples:

Solution Walkthrough:

Before:

After:

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

fix flaky Ruby test element_spec.rb #16709

fix flaky Ruby test element_spec.rb #16709

Conversation

asolntsev commented Dec 9, 2025 • edited by qodo-code-review bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

💥 What does this PR do?

💡 Additional Considerations

🔄 Types of changes

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

qodo-code-review bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Compliance Guide 🔍

Uh oh!

qodo-code-review bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Examples:

Solution Walkthrough:

Before:

After:

Uh oh!

qodo-code-review bot commented Dec 10, 2025

PR Compliance Guide 🔍

Uh oh!

qodo-code-review bot commented Dec 10, 2025

PR Code Suggestions ✨

Examples:

Solution Walkthrough:

Before:

After:

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix flaky Ruby test `element_spec.rb` #16709

fix flaky Ruby test `element_spec.rb` #16709

asolntsev commented Dec 9, 2025 •

edited by qodo-code-review bot

Loading

qodo-code-review bot commented Dec 9, 2025 •

edited

Loading

qodo-code-review bot commented Dec 9, 2025 •

edited

Loading