Avoid sleep in tests

[asolntsev]

User description

💥 What does this PR do?

This PR removes most of "SLEEP N" from Java tests.

🔧 Implementation Notes

Instead of just silly waiting for N seconds, it's better to wait for the needed event. E.g. to wait the needed button on the screen etc.

Thus we make our tests faster and more stable.

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix, Enhancement

---

Description

• Replace artificial sleeps with proper wait conditions in tests

• Improve test stability by waiting for elements to be visible/ready

• Reduce test execution time significantly (e.g., SessionHandlingTest from 28s to 7s)

• Add blank page navigation before each test to isolate test state

• Refactor test pages to show elements only after initialization

---

Diagram Walkthrough

flowchart LR
  A["Test Code with Sleep"] -->|Replace| B["Wait for Element Visibility"]
  C["Test Page Elements"] -->|Initialize| D["Show After Ready"]
  E["Test Setup"] -->|Add| F["Navigate to Blank Page"]
  B --> G["Faster & More Stable Tests"]
  D --> G
  F --> G

Loading

File Walkthrough

	Relevant files
Code cleanup	1 files ExpectedConditions.java Remove explicit type parameters using diamond operator      +48/-50
Bug fix	16 files CorrectEventFiringTest.java Replace sleep loop with wait condition for focus event      +3/-23    ElementAttributeTest.java Remove artificial sleep and unused logger                                +0/-9      ReferrerTest.java Add null checks in tearDown to prevent NPE                              +9/-3      SessionHandlingTest.java Replace sleeps with wait conditions for session closure    +20/-14  CombinedInputActionsTest.java Replace sleep with visibility wait in hover test                  +22/-21  DefaultKeyboardTest.java Remove artificial sleep and improve assertions                      +11/-17  DefaultMouseTest.java Replace sleeps and polling with wait conditions                    +31/-56  DragAndDropTest.java Replace sleep and polling with visibility waits                    +18/-42  CdpFacadeTest.java Add null check and reset server reference                                +1/-0      RemoteWebDriverBiDiTest.java Add null checks in cleanup methods                                              +4/-3      JdbcBackedSessionMapTest.java Add null checks and reset references in tearDown                  +8/-2      CombinedInputActionsTest.java Replace sleep with visibility wait in hover test                  +22/-20  DefaultMouseTest.java Replace sleeps and polling with wait conditions                    +30/-49  DragAndDropTest.java Replace sleep and polling with visibility waits                    +21/-44  PenPointerTest.java Replace sleeps with visibility waits in hover tests            +30/-39  CrossDomainTest.java Add null checks in server cleanup                                                +5/-1
Enhancement	8 files WaitingConditions.java Add new wait conditions and use diamond operator                  +39/-21  JupiterTestBase.java Add blank page navigation before each test                              +6/-0      SeleniumExtension.java Reduce polling interval for faster test execution                +2/-2      dragAndDropTest.html Hide elements initially and show after initialization        +18/-13  draggableLists.html Hide elements initially and show after jQuery setup            +22/-21  droppableItems.html Hide elements initially and show after jQuery setup            +26/-23  macbeth.html Add h1 header element for test visibility                                +2/-0      blank_page.html Clear storage on blank page load                                                  +4/-1
Tests	1 files WaitingConditionsTest.java Add new test class for waiting conditions                                +58/-0

---

[selenium-ci]

Thank you, @asolntsev for this code suggestion.

The support packages contain example code that many users find helpful, but they do not necessarily represent
the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

After reviewing the change, unless it is a critical fix or a feature that is needed for Selenium
to work, we will likely close the PR.

We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
If you have any questions, please contact us

[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: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Not Applicable: The PR modifies test code and waiting conditions without introducing or altering any critical action flows that would require audit logging; applicability of audit trails cannot be determined from the provided diff. Referred Code class DragAndDropTest extends JupiterTestBase { @Test void testDragAndDropRelative() { driver.get(pages.dragAndDropPage); WebElement img = wait.until(visibilityOfElementLocated(By.id("test1"))); Point expectedLocation = img.getLocation(); expectedLocation = drag(img, expectedLocation, 150, 200); wait.until(elementLocationToBe(img, expectedLocation)); expectedLocation = drag(img, expectedLocation, -50, -25); wait.until(elementLocationToBe(img, expectedLocation)); expectedLocation = drag(img, expectedLocation, 0, 0); wait.until(elementLocationToBe(img, expectedLocation)); expectedLocation = drag(img, expectedLocation, 1, -1); wait.until(elementLocationToBe(img, expectedLocation)); } @Test void testDragAndDropToElement() { driver.get(pages.dragAndDropPage); ... (clipped 116 lines) 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: Null Returns: Several new ExpectedConditions return null on mismatch without additional context or logging, which is typical for waits but limits actionable error context if used beyond testing. Referred Code }; } public static ExpectedCondition<String> elementTextToContain( final By locator, final String expected) { return new ExpectedCondition<>() { @Override public String apply(WebDriver driver) { String text = driver.findElement(locator).getText(); return text.contains(expected) ? text : null; } @Override public String toString() { return String.format("element text did not contain \"%s\"", expected); } }; } public static ExpectedCondition<String> elementTextToMatch(final By locator, final String regex) { return new ExpectedCondition<>() { ... (clipped 56 lines) 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: Delayed Init Risks: The added delayed initialization and dynamic showing of elements could create brief states where event handlers are not attached; while acceptable for tests, validation of external input is not in scope here and overall security impact cannot be assessed from the diff. Referred Code #droppable { width: 150px; height: 150px; padding: 0.5em; float: left; margin: 10px; } </style> <script type="text/javascript"> $(function () { setTimeout(function () { $("#draggable").draggable(); $("#droppable").droppable({ drop: function () { $(this).addClass('ui-state-highlight').find('p').html('Dropped!'); } }); const report_event = function (report_text) { const reportElement = $("#drop_reports"); const origText = reportElement.text(); reportElement.text(origText + " " + report_text); } $('body').mousedown(function () { report_event('down'); }); ... (clipped 25 lines) 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	Avoid hardcoded values in assertions In testDragTooFar, replace the assertion hasMessageContaining with a hardcoded coordinate with hasMessageMatching and a regular expression to make the test more robust against environmental variations. java/test/org/openqa/selenium/bidi/input/DragAndDropTest.java [131-148] @Test void testDragTooFar() { driver.get(pages.dragAndDropPage); WebElement img = wait.until(visibilityOfElementLocated(By.id("test1"))); assertThatThrownBy( () -> inputModule.perform( windowHandle, new Actions(driver) .dragAndDropBy(img, Integer.MAX_VALUE, Integer.MAX_VALUE) .getSequences())) .as("These coordinates are outside the page - expected to fail.") .isInstanceOf(BiDiException.class) - .hasMessageContaining("Move target (2147483664, 2147483664) is out of bounds"); + .hasMessageMatching("Move target \\(.*\\) is out of bounds"); // Release mouse button - move was interrupted in the middle. inputModule.perform(windowHandle, new Actions(driver).release().getSequences()); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that asserting a hardcoded coordinate value makes the test brittle. Using a regex to match the error message structure is a significant improvement for test robustness and maintainability.	Medium
	Use a more precise mouse action In the unfocusMenu method, replace the click() action with a moveToElement action to avoid unintended side effects and more accurately reflect the intent of moving the mouse. java/test/org/openqa/selenium/interactions/CombinedInputActionsTest.java [404-410] /** * Move to a different element to make sure the mouse is not over the menu items (from a previous * test). */ private void unfocusMenu() { - driver.findElement(By.id("dynamo")).click(); + new Actions(driver).moveToElement(driver.findElement(By.id("dynamo"))).build().perform(); } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that using click() to move the mouse can have unintended side effects and proposes using a moveToElement action instead, which is a more precise and safer approach for the intended purpose of unfocusing a menu.	Medium
Learned best practice	Centralize safe poll interval Avoid hard-coding very tight poll intervals; define a constant with safe bounds and reuse it to prevent overly aggressive polling and flaky behavior. java/test/org/openqa/selenium/testing/SeleniumExtension.java [310-311] -this.regularWait = new WebDriverWait(driver, regularWait, Duration.ofMillis(20)); -this.shortWait = new WebDriverWait(driver, shortWait, Duration.ofMillis(20)); +private static final Duration DEFAULT_POLL_INTERVAL = Duration.ofMillis(100); +... +this.regularWait = new WebDriverWait(driver, regularWait, DEFAULT_POLL_INTERVAL); +this.shortWait = new WebDriverWait(driver, shortWait, DEFAULT_POLL_INTERVAL); [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Pattern 5: Guard time calculations and use clear, centralized timeout configuration to avoid flaky tests.	Low
More

[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: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	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: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Unsafe driver cast: The new helper getSessionId() unconditionally casts driver to RemoteWebDriver, which may throw at runtime in environments where driver is not a RemoteWebDriver and is not guarded/handled. Referred Code private void waitUntilBrowserFullyClosed() { wait.until($ -> getSessionId() == null); } private SessionId getSessionId() { return ((RemoteWebDriver) driver).getSessionId(); } 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
High-level	Refactor test setup for isolation The PR adds test isolation logic to JupiterTestBase, but this is not applied to all tests. This logic should be refactored into a reusable utility and used consistently across all relevant test classes. Examples: java/test/org/openqa/selenium/testing/JupiterTestBase.java [88-92] if (driver != null) { driver.get("about:blank"); driver.get(pages.blankPage); driver.manage().deleteAllCookies(); } java/test/org/openqa/selenium/grid/router/RemoteWebDriverBiDiTest.java [67-82] void setup() { Browser browser = Objects.requireNonNull(Browser.detect()); Deployment deployment = DeploymentTypes.STANDALONE.start( browser.getCapabilities(), new TomlConfig( new StringReader( "[node]\n" + "selenium-manager = false\n" ... (clipped 6 lines) Solution Walkthrough: Before: // In JupiterTestBase.java class JupiterTestBase { @BeforeEach public void prepareEnvironment() { // ... driver = seleniumExtension.getDriver(); // ... if (driver != null) { driver.get("about:blank"); driver.get(pages.blankPage); driver.manage().deleteAllCookies(); } } } // In RemoteWebDriverBiDiTest.java (does not extend JupiterTestBase) class RemoteWebDriverBiDiTest { @BeforeEach void setup() { // ... creates driver, but no isolation logic } } After: // In a new TestIsolationUtil.java class TestIsolationUtil { public static void resetState(WebDriver driver, Pages pages) { if (driver != null) { driver.get("about:blank"); driver.get(pages.blankPage); driver.manage().deleteAllCookies(); } } } // In JupiterTestBase.java class JupiterTestBase { @BeforeEach public void prepareEnvironment() { // ... TestIsolationUtil.resetState(driver, pages); } } // In RemoteWebDriverBiDiTest.java class RemoteWebDriverBiDiTest { @BeforeEach void setup() { // ... creates driver TestIsolationUtil.resetState(driver, new Pages(server)); } } Suggestion importance[1-10]: 7 __ Why: This is a valid architectural suggestion that correctly identifies an inconsistency in the application of the new test isolation logic, which is a key goal of the PR.	Medium
Possible issue	Ensure mouse release in finally Wrap the assertion in testDragTooFar with a try...finally block to ensure the mouse release() action is always executed, preventing state leakage between tests. java/test/org/openqa/selenium/bidi/input/DragAndDropTest.java [131-148] @Test void testDragTooFar() { driver.get(pages.dragAndDropPage); WebElement img = wait.until(visibilityOfElementLocated(By.id("test1"))); - assertThatThrownBy( - () -> - inputModule.perform( - windowHandle, - new Actions(driver) - .dragAndDropBy(img, Integer.MAX_VALUE, Integer.MAX_VALUE) - .getSequences())) - .as("These coordinates are outside the page - expected to fail.") - .isInstanceOf(BiDiException.class) - .hasMessageContaining("Move target (2147483664, 2147483664) is out of bounds"); - - // Release mouse button - move was interrupted in the middle. - inputModule.perform(windowHandle, new Actions(driver).release().getSequences()); + try { + assertThatThrownBy( + () -> + inputModule.perform( + windowHandle, + new Actions(driver) + .dragAndDropBy(img, Integer.MAX_VALUE, Integer.MAX_VALUE) + .getSequences())) + .as("These coordinates are outside the page - expected to fail.") + .isInstanceOf(BiDiException.class) + .hasMessageContaining("Move target (2147483664, 2147483664) is out of bounds"); + } finally { + // Release mouse button - move was interrupted in the middle. + inputModule.perform(windowHandle, new Actions(driver).release().getSequences()); + } } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out a potential test state leakage issue and proposes using a finally block to ensure the mouse is always released, improving test suite robustness.	Medium
General	Use mouse-move instead of click Replace the click() call in unfocusMenu with a moveToElement action to avoid unintended click events when moving the mouse. java/test/org/openqa/selenium/interactions/CombinedInputActionsTest.java [407-410] /** * Move to a different element to make sure the mouse is not over the menu items (from a previous * test). */ private void unfocusMenu() { - driver.findElement(By.id("dynamo")).click(); + new Actions(driver).moveToElement(driver.findElement(By.id("dynamo"))).perform(); } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that using click() to move the mouse has side effects and proposes using moveToElement for a more precise and robust implementation.	Low
Learned best practice	Use safe, bounded timeouts A 1ms timeout is effectively guaranteed to be flaky; use a reasonable, bounded duration (and ideally a named constant) so timing is stable across environments. java/test/org/openqa/selenium/WaitingConditionsTest.java [34-39] @BeforeEach void setUp() { - wait = new WebDriverWait(driver, Duration.ofMillis(1)); + wait = new WebDriverWait(driver, Duration.ofSeconds(1)); driver.get(pages.macbethPage); shortWait.until(visibilityOfElementLocated(header)); } Apply / Chat Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Guard time calculations by using clear units and safe bounds; avoid unrealistically small timeouts that cause flakiness.	Low
More