[java] Refactor tests

[asolntsev]

User description

💥 What does this PR do?

Improve tests:

1. use the right AssertJ method assertThatThrownBy
2. remove unneeded try/catch from tests

🔄 Types of changes

• Cleanup (formatting, renaming)

---

PR Type

Tests

---

Description

Comprehensive refactoring of Java test files to improve code quality and consistency:

• Replaced JUnit assertions with AssertJ methods: Standardized assertion patterns across 50+ test files by replacing assertEquals(), .size(), and .get() calls with idiomatic AssertJ methods like assertThat().isEqualTo(), hasSize(), and hasValue()

• Removed try-catch blocks: Eliminated exception handling boilerplate by using assertThatThrownBy() and assertThatCode() for exception testing, or propagating exceptions with throws declarations

• Consolidated AssertJ imports: Unified fragmented imports from AssertionsForClassTypes and AssertionsForInterfaceTypes to the single Assertions class for consistency

• Improved error messages: Enhanced debugging information in exception messages and fail() calls by including contextual details like session IDs and credential information

• Code cleanup: Removed unused imports, fixed variable naming typos, and simplified resource management with try-with-resources blocks

• Updated IDE configuration: Modified code style settings to increase import-on-demand thresholds and standardize package import preferences

---

Diagram Walkthrough

flowchart LR
  A["Test Files<br/>50+ Java test classes"] -->|"Replace JUnit<br/>with AssertJ"| B["Standardized<br/>Assertions"]
  A -->|"Remove try-catch<br/>blocks"| C["Cleaner Exception<br/>Handling"]
  A -->|"Consolidate<br/>imports"| D["Unified Import<br/>Strategy"]
  A -->|"Improve error<br/>messages"| E["Better Debugging<br/>Information"]
  B --> F["Improved Code<br/>Quality"]
  C --> F
  D --> F
  E --> F

Loading

File Walkthrough

	Relevant files
Cleanup	44 files LocalNewSessionQueueTest.java Refactor tests to use AssertJ assertions and remove try-catch java/test/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueueTest.java Replaced JUnit assertions with AssertJ methods (e.g., assertEquals → assertThat().isEqualTo()) Removed try-catch blocks by using assertThatThrownBy for exception testing Consolidated wildcard imports for java.util.* and java.util.concurrent.* Added static import for randomUUID() and replaced UUID.randomUUID() calls Improved assertions using AssertJ methods like hasSize(), isEmpty(), containsExactly() +96/-126 JmxTest.java Remove try-catch blocks and simplify exception handling    java/test/org/openqa/selenium/grid/router/JmxTest.java Removed try-catch blocks and added throws Exception to test methods Consolidated wildcard imports for javax.management.* Removed unused fail() import from JUnit Simplified test code by removing exception handling boilerplate +100/-160 TakesScreenshotTest.java Remove try-catch blocks and use AssertJ assertions              java/test/org/openqa/selenium/TakesScreenshotTest.java Removed try-catch blocks and added throws IOException to test methods Replaced fail() calls with AssertJ assertions Simplified helper methods by propagating exceptions instead of catching them Used assertThat().containsExactlyInAnyOrderElementsOf() for color comparison +30/-56  LogInspectorTest.java Replace size assertions and remove exception handling boilerplate java/test/org/openqa/selenium/bidi/log/LogInspectorTest.java Replaced .size() with AssertJ hasSize() method Removed try-catch blocks and added throws TimeoutException to methods Fixed variable naming (errorLogfuture → errorLogFuture) Simplified exception handling by removing unnecessary try-catch +14/-26  WebScriptExecuteTest.java Standardize AssertJ imports and use hasValue/hasSize methods java/test/org/openqa/selenium/WebScriptExecuteTest.java Changed import from AssertionsForClassTypes to Assertions Replaced .get() calls with AssertJ hasValue() method Replaced .size() with AssertJ hasSize() method +12/-12  NetworkCommandsTest.java Refactor to use AssertJ and remove exception handling boilerplate java/test/org/openqa/selenium/bidi/network/NetworkCommandsTest.java Changed imports from AssertionsForClassTypes to Assertions Added static import for COMPLETE from ReadinessState Removed try-catch blocks and used assertThatThrownBy for exception testing Added logging for expected exceptions instead of silently ignoring them Replaced .get() with hasValue() assertion +23/-27  TakesFullPageScreenshotTest.java Remove try-catch blocks and use AssertJ assertions              java/test/org/openqa/selenium/firefox/TakesFullPageScreenshotTest.java Removed try-catch blocks and added throws IOException to test methods Replaced fail() calls with AssertJ assertions Simplified helper methods by propagating exceptions Used assertThat().containsExactlyInAnyOrderElementsOf() for color comparison +25/-49  ScriptCommandsTest.java Standardize AssertJ imports and use hasSize/hasValue methods java/test/org/openqa/selenium/bidi/script/ScriptCommandsTest.java Reordered imports to put Assertions before AssertionsForClassTypes Replaced .size() with AssertJ hasSize() method Replaced .get() with hasValue() assertion +8/-8      PageLoadTimeOutTest.java Replace try-catch with assertThatThrownBy and improve assertions java/test/org/openqa/selenium/PageLoadTimeOutTest.java Added static import for currentTimeMillis() and assertThatThrownBy Replaced try-catch blocks with assertThatThrownBy for exception testing Simplified duration calculations using isBetween() assertion Consolidated wildcard imports for Browser.* +27/-40  LocateNodesTest.java Standardize AssertJ imports and use hasSize/hasValue methods java/test/org/openqa/selenium/bidi/browsingcontext/LocateNodesTest.java Changed import from AssertionsForClassTypes to Assertions Replaced .size() with AssertJ hasSize() method Replaced .get() with hasValue() assertion Removed wildcard import and specified individual Browser imports +11/-11  LocalValueTest.java Standardize AssertJ imports and use hasSize/hasValue methods java/test/org/openqa/selenium/bidi/script/LocalValueTest.java Changed import from AssertionsForClassTypes to Assertions Replaced .size() with AssertJ hasSize() method Replaced .get() with hasValue() assertion +6/-6      EndToEndTest.java Replace try-catch with assertThatThrownBy and improve assertions java/test/org/openqa/selenium/grid/router/EndToEndTest.java Replaced try-catch blocks with assertThatThrownBy for exception testing Added assertThatThrownBy import Improved test structure with try-finally blocks for resource cleanup Enhanced assertion messages with .as() method +32/-25  WebScriptTest.java Replace try-catch with assertThatThrownBy and use hasSize method java/test/org/openqa/selenium/WebScriptTest.java Changed imports to use Assertions.* wildcard Replaced try-catch blocks with assertThatThrownBy for exception testing Replaced .size() with AssertJ hasSize() method +10/-16  NodeOptionsTest.java Replace try-catch with assertThatThrownBy and improve assertions java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java Replaced try-catch blocks with assertThatThrownBy for exception testing Added assertThatThrownBy import Enhanced assertions with .as() and .hasMessage() methods Removed unused fail() import +24/-25  HeartBeatTest.java Remove try-catch and use try-with-resources for resource management java/test/org/openqa/selenium/grid/distributor/HeartBeatTest.java Removed try-catch block and added throws InterruptedException to test method Added try-with-resources for LocalNode resource management Simplified exception handling by propagating exceptions +17/-22  RetryRequestTest.java Replace atomic variable assertions with hasValue method    java/test/org/openqa/selenium/remote/http/RetryRequestTest.java Replaced .get() with AssertJ hasValue() method on atomic variables +8/-8      SessionQueueGridTest.java Remove try-catch blocks and simplify exception handling    java/test/org/openqa/selenium/grid/router/SessionQueueGridTest.java Removed try-catch blocks and added throws Exception to test methods Consolidated wildcard imports for java.util.concurrent.* Simplified exception handling by propagating exceptions +3/-22    LocalDistributorTest.java Replace size assertions with hasSize method                            java/test/org/openqa/selenium/grid/distributor/local/LocalDistributorTest.java Replaced .size() with AssertJ hasSize() method Improved fail() call with exception message and cause +5/-5      ChromeDriverFunctionalTest.java Replace try-catch with assertThatThrownBy and use try-with-resources java/test/org/openqa/selenium/chrome/ChromeDriverFunctionalTest.java Replaced try-catch blocks with assertThatThrownBy for exception testing Consolidated imports to use single Assertions class Added try-with-resources for ChromeDriverService resource management +9/-16    CorrectEventFiringTest.java Replace try-catch with assertThatCode and improve assertions java/test/org/openqa/selenium/CorrectEventFiringTest.java Changed imports to use Assertions.* wildcard Replaced try-catch blocks with assertThatCode and assertThat assertions Improved assertion messages with .as() method +7/-11    EdgeDriverFunctionalTest.java Replace try-catch with assertThatThrownBy and use try-with-resources java/test/org/openqa/selenium/edge/EdgeDriverFunctionalTest.java Replaced try-catch blocks with assertThatThrownBy for exception testing Consolidated imports to use single Assertions class Added try-with-resources for EdgeDriverService resource management +8/-16    RemoteFirefoxDriverTest.java Replace try-catch with assertThatCode and remove unused imports java/test/org/openqa/selenium/firefox/RemoteFirefoxDriverTest.java Replaced try-catch blocks with assertThatCode for exception testing Removed unused imports (MalformedURLException, WebDriverException) Added assertThatCode import +4/-9      RemotableByTest.java Use static import and replace get assertions with hasValue java/test/org/openqa/selenium/remote/RemotableByTest.java Added static import for ImmutableMap.of() Replaced .get() with AssertJ hasValue() method Removed ImmutableMap import in favor of static method import +6/-11    DragAndDropTest.java Replace try-catch with assertThatThrownBy and improve assertions java/test/org/openqa/selenium/bidi/input/DragAndDropTest.java Replaced try-catch blocks with assertThatThrownBy for exception testing Added assertThatThrownBy import Improved assertion messages with .as() and .hasMessageContaining() methods +15/-13  CallFunctionParameterTest.java Standardize AssertJ imports and use hasSize method              java/test/org/openqa/selenium/bidi/script/CallFunctionParameterTest.java Changed import from AssertionsForClassTypes to Assertions Replaced .size() with AssertJ hasSize() method +3/-4      StorageCommandsTest.java Replace size and get assertions with hasSize and hasValue java/test/org/openqa/selenium/bidi/storage/StorageCommandsTest.java Replaced .size() with AssertJ hasSize() method Replaced .get() with hasValue() assertion +3/-3      ScriptEventsTest.java Standardize AssertJ imports and use hasValue method            java/test/org/openqa/selenium/bidi/script/ScriptEventsTest.java Changed import from AssertionsForClassTypes to Assertions Replaced .get() with hasValue() assertion Removed wildcard import and specified individual Browser imports +6/-5      NetworkEventsTest.java Standardize AssertJ imports and use hasSize method              java/test/org/openqa/selenium/bidi/network/NetworkEventsTest.java Changed import from AssertionsForClassTypes to Assertions Replaced .size() with AssertJ hasSize() method Removed wildcard import and specified individual Browser imports +4/-3      EvaluateParametersTest.java Standardize AssertJ imports and use hasSize method              java/test/org/openqa/selenium/bidi/script/EvaluateParametersTest.java Changed import from AssertionsForClassTypes to Assertions Replaced .size() with AssertJ hasSize() method +2/-3      ExpectedConditionsTest.java Add static import and replace size assertions with hasSize java/test/org/openqa/selenium/support/ui/ExpectedConditionsTest.java Added static import for cssSelector Replaced .size() with AssertJ hasSize() method +4/-6      SessionQueueGridWithTimeoutTest.java Remove try-catch blocks and fix method naming                        java/test/org/openqa/selenium/grid/router/SessionQueueGridWithTimeoutTest.java Removed try-catch blocks and added throws Exception to test method Removed unused imports (ExecutionException, TimeoutException) Fixed method name typo (shouldBeAbleToDeleteTimedoutSessions → shouldBeAbleToDeleteTimedOutSessions) +1/-11    FirefoxDriverTest.java Replace try-catch with assertThatThrownBy                                java/test/org/openqa/selenium/firefox/FirefoxDriverTest.java Replaced try-catch blocks with assertThatThrownBy for exception testing Consolidated imports to use single Assertions class +4/-6      NetworkInterceptorTest.java Consolidate AssertJ imports to unified Assertions class    java/test/org/openqa/selenium/devtools/NetworkInterceptorTest.java Consolidated AssertJ imports from separate AssertionsForClassTypes and AssertionsForInterfaceTypes to unified Assertions class Simplified import statements for better consistency and maintainability +2/-2      RemoteWebDriverBiDiTest.java Consolidate imports and use AssertJ hasSize() method          java/test/org/openqa/selenium/grid/router/RemoteWebDriverBiDiTest.java Updated import from AssertionsForClassTypes to unified Assertions class Replaced wildcard import of Browser with explicit imports for IE and SAFARI Replaced assertThat(logEntry.getArgs().size()).isEqualTo(1) with assertThat(logEntry.getArgs()).hasSize(1) +4/-3      ClosureTestStatement.java Remove unnecessary try-catch and unused imports                    java/test/org/openqa/selenium/javascript/ClosureTestStatement.java Removed unused imports for fail() and WebDriverException Removed try-catch block around driver.get() call, allowing exceptions to propagate naturally Simplified error handling by eliminating unnecessary exception catching +1/-8      WebNetworkTest.java Consolidate AssertJ imports to unified Assertions class    java/test/org/openqa/selenium/WebNetworkTest.java Consolidated AssertJ imports from AssertionsForClassTypes to unified Assertions class Reordered imports for consistency +1/-1      SetTimezoneOverrideTest.java Consolidate AssertJ imports to unified Assertions class    java/test/org/openqa/selenium/bidi/emulation/SetTimezoneOverrideTest.java Updated import from AssertionsForClassTypes to unified Assertions class +1/-1      SetScriptingEnabledTest.java Consolidate AssertJ imports to unified Assertions class    java/test/org/openqa/selenium/bidi/emulation/SetScriptingEnabledTest.java Updated import from AssertionsForClassTypes to unified Assertions class +1/-1      BiDiTest.java Consolidate AssertJ imports to unified Assertions class    java/test/org/openqa/selenium/bidi/BiDiTest.java Updated import from AssertionsForClassTypes to unified Assertions class +1/-1      AddInterceptParametersTest.java Consolidate AssertJ imports to unified Assertions class    java/test/org/openqa/selenium/bidi/network/AddInterceptParametersTest.java Updated import from AssertionsForClassTypes to unified Assertions class +1/-1      SetGeolocationOverrideTest.java Consolidate AssertJ imports to unified Assertions class    java/test/org/openqa/selenium/bidi/emulation/SetGeolocationOverrideTest.java Updated import from AssertionsForClassTypes to unified Assertions class +1/-1      BiDiSessionTest.java Consolidate AssertJ imports to unified Assertions class    java/test/org/openqa/selenium/bidi/BiDiSessionTest.java Updated import from AssertionsForClassTypes to unified Assertions class +1/-1      RemoteWebDriverDownloadTest.java Consolidate imports using wildcard notation                            java/test/org/openqa/selenium/grid/router/RemoteWebDriverDownloadTest.java Consolidated multiple individual imports into a single wildcard import for org.openqa.selenium package +1/-6      SafariDriverTest.java Consolidate AssertJ imports using wildcard notation            java/test/org/openqa/selenium/safari/SafariDriverTest.java Consolidated multiple AssertJ imports into a single wildcard import from Assertions class +1/-3
Enhancement	20 files LazyTest.java Add generic type parameters to Lazy declarations                  java/test/org/openqa/selenium/concurrent/LazyTest.java Added generic type parameters to Lazy variable declarations +5/-5      VirtualAuthenticatorTest.java Use AssertJ hasSize() method and improve error messages    java/test/org/openqa/selenium/virtualauthenticator/VirtualAuthenticatorTest.java Replaced assertThat(credentialList.size()).isEqualTo(1) with assertThat(credentialList).hasSize(1) for more idiomatic AssertJ usage Replaced assertThat(credentials.size()).isEqualTo(2) with assertThat(credentials).hasSize(2) Enhanced error message in fail() call to include the unrecognized credential ID for better debugging +3/-3      RemoteWebDriverBuilderTest.java Use AssertJ hasValue() for Optional assertions                      java/test/org/openqa/selenium/remote/RemoteWebDriverBuilderTest.java Replaced assertThat(seen.get()).isEqualTo(uri) with assertThat(seen).hasValue(uri) in four test methods Uses more appropriate AssertJ method for Optional assertions +4/-4      ProxyWebsocketTest.java Use AssertJ hasValue() for Optional assertions                      java/test/org/openqa/selenium/grid/router/ProxyWebsocketTest.java Replaced assertThat(text.get()).isEqualTo("Cheese!") with assertThat(text).hasValue("Cheese!") in three test methods Uses more appropriate AssertJ method for Optional assertions +3/-3      PrintOptionsTest.java Use AssertJ hasSize() method for collection assertions      java/test/org/openqa/selenium/print/PrintOptionsTest.java Replaced assertThat(map.size()).isEqualTo(7) with assertThat(map).hasSize(7) Replaced assertThat(margin.size()).isEqualTo(4) with assertThat(margin).hasSize(4) Replaced assertThat(page.size()).isEqualTo(2) with assertThat(page).hasSize(2) +3/-3      RedisBackedSessionMapTest.java Use assertThatThrownBy() and consolidate imports                  java/test/org/openqa/selenium/grid/sessionmap/redis/RedisBackedSessionMapTest.java Consolidated AssertJ imports to use wildcard import from Assertions Replaced try-catch block with assertThatThrownBy() for cleaner exception assertion Removed explicit fail() call in favor of AssertJ's exception handling +3/-8      ElementFindingTest.java Use AssertJ hasSize() method for list assertions                  java/test/org/openqa/selenium/ElementFindingTest.java Replaced assertThat(elements.size()).isEqualTo(2) with assertThat(elements).hasSize(2) +1/-1      WebElementToJsonConverterTest.java Use AssertJ hasSize() method for map assertions                    java/test/org/openqa/selenium/remote/WebElementToJsonConverterTest.java Replaced assertThat(map.size()).isEqualTo(1) with assertThat(map).hasSize(1) in two test methods +2/-2      RedisBackedSessionMap.java Improve error message with session ID information                java/src/org/openqa/selenium/grid/sessionmap/redis/RedisBackedSessionMap.java Enhanced error message in NoSuchSessionException to include the session ID for better debugging +2/-1      JdbcBackedSessionMap.java Improve error message with session ID information                java/src/org/openqa/selenium/grid/sessionmap/jdbc/JdbcBackedSessionMap.java Enhanced error message in NoSuchSessionException to include the session ID for better debugging +1/-1      WebSocketTestBase.java Use AssertJ hasValue() for Optional assertions                      java/test/org/openqa/selenium/remote/internal/WebSocketTestBase.java Replaced assertThat(message.get()).isEqualTo("Hello, World!") with assertThat(message).hasValue("Hello, World!") Replaced assertThat(message.get()).isEqualTo("brie".getBytes(UTF_8)) with assertThat(message).hasValue("brie".getBytes(UTF_8)) +2/-2      RelayOptionsTest.java Use AssertJ hasSize() method for collection assertions      java/test/org/openqa/selenium/grid/node/relay/RelayOptionsTest.java Replaced assertThat(sessionFactories.get(chrome).size()).isEqualTo(2) with assertThat(sessionFactories.get(chrome)).hasSize(2) +1/-1      ChildrenFindingTest.java Use AssertJ hasSize() method for list assertions                  java/test/org/openqa/selenium/ChildrenFindingTest.java Replaced assertThat(allPs.size()).isEqualTo(children.size()) with assertThat(allPs).hasSize(children.size()) +1/-1      CookieImplementationTest.java Use AssertJ hasSize() method for collection assertions      java/test/org/openqa/selenium/CookieImplementationTest.java Replaced assertThat(cookies.size()).isEqualTo(countBefore + 2) with assertThat(cookies).hasSize(countBefore + 2) +1/-1      DistributorNodeAvailabilityTest.java Use AssertJ hasSize() method for collection assertions      java/test/org/openqa/selenium/grid/distributor/DistributorNodeAvailabilityTest.java Replaced assertThat(status.getNodes().size()).isEqualTo(1) with assertThat(status.getNodes()).hasSize(1) +1/-1      FilterTest.java Use AssertJ hasValue() for Optional assertions                      java/test/org/openqa/selenium/remote/http/FilterTest.java Replaced assertThat(rootCalls.get()).isEqualTo(1) with assertThat(rootCalls).hasValue(1) Replaced assertThat(filterOneCount.get()).isEqualTo(1) with assertThat(filterOneCount).hasValue(1) Replaced assertThat(filterTwoCount.get()).isEqualTo(1) with assertThat(filterTwoCount).hasValue(1) +3/-3      NettyServerTest.java Use AssertJ hasValue() for Optional assertions                      java/test/org/openqa/selenium/netty/server/NettyServerTest.java Replaced assertThat(count.get()).isEqualTo(1) with assertThat(count).hasValue(1) in two test methods +2/-2      JsonTest.java Use AssertJ hasSize() method for map assertions                    java/test/org/openqa/selenium/json/JsonTest.java Replaced assertThat(converted.size()).isEqualTo(1) with assertThat(converted).hasSize(1) +1/-1      EventFiringDecoratorTest.java Use AssertJ hasValue() for Optional assertions                      java/test/org/openqa/selenium/support/events/EventFiringDecoratorTest.java Replaced assertThat(invocationCount.get()).isEqualTo(1) with assertThat(invocationCount).hasValue(1) +1/-1      JsonOutputTest.java Use AssertJ hasSize() method for collection assertions      java/test/org/openqa/selenium/json/JsonOutputTest.java Replaced assertThat(converted.size()).isEqualTo(1) with assertThat(converted).hasSize(1) +1/-1
Configuration changes	1 files Project.xml Update IDE code style configuration settings                          .idea/codeStyles/Project.xml Reordered code style options for consistency Updated CLASS_COUNT_TO_USE_IMPORT_ON_DEMAND and NAMES_COUNT_TO_USE_IMPORT_ON_DEMAND from 99 to 999 Added PACKAGES_TO_USE_IMPORT_ON_DEMAND configuration for java.awt and javax.swing Removed deprecated XML_LEGACY_SETTINGS_IMPORTED option Fixed file ending with newline +10/-7
Formatting	1 files modules.xml Fix file ending with newline                                                          .idea/modules.xml Added newline at end of file for consistency +1/-1
Additional files	9 files BiDiSessionCleanUpTest.java +2/-2      ListImagesTest.java +2/-2      FederatedCredentialManagementTest.java +2/-2      DistributorDrainingTest.java +2/-2      SessionCleanUpTest.java +2/-11    JdbcBackedSessionMapTest.java +6/-9      InternetExplorerDriverTest.java +6/-7      DragAndDropTest.java +11/-10  PrefixedRouteTest.java +5/-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: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No auditing: This PR primarily refactors tests and project configs and does not introduce or modify any audit logging for critical actions, which may be acceptable for test-only changes but cannot be confirmed from the diff alone. Referred Code ./go java:install toolchainType=TODO ./scripts/format.sh bazel test //java/... --test_size_filters=small bazel test //java/test/org/openqa/selenium/bidi/browsingcontext:BrowsingContextTest bazel test //java/test/org/openqa/selenium/grid/router:RemoteWebDriverDownloadTest 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: Config changes: Project configuration changes (import-on-demand thresholds and packages) were added which do not directly impact input validation but warrant human review for unintended side effects; no new input surfaces were introduced in code. Referred Code <option name="SPACES_WITHIN_IMPORTS" value="true" /> </JSCodeStyleSettings> <JavaCodeStyleSettings> <option name="CLASS_COUNT_TO_USE_IMPORT_ON_DEMAND" value="999" /> <option name="NAMES_COUNT_TO_USE_IMPORT_ON_DEMAND" value="999" /> <option name="PACKAGES_TO_USE_IMPORT_ON_DEMAND"> <value> <package name="java.awtNOPE" withSubpackages="false" static="false" /> <package name="javax.swing" withSubpackages="false" static="false" /> </value> </option> <option name="IMPORT_LAYOUT_TABLE"> 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	Make exception message assertion more robust Improve the robustness of an exception message assertion by checking for the substring "out of bounds" instead of a hardcoded, specific message with coordinates. java/test/org/openqa/selenium/bidi/input/DragAndDropTest.java [145-154] assertThatThrownBy( () -> inputModule.perform( windowHandle, actions .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"); + .hasMessageContaining("out of bounds"); Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that asserting on a very specific error message makes the test brittle. Relaxing the assertion to check for a key substring (out of bounds) improves test robustness and maintainability.	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: 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: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No audit impact: The PR refactors test assertions and exception handling without adding or modifying runtime audit logging; no critical action logging changes are present in the added lines. Referred Code // The node added only has a single node. Make sure we can start and stop sessions. Capabilities caps = new ImmutableCapabilities("browserName", "cheese", "se:type", "cheddar"); WebDriver driver = new RemoteWebDriver(server.getUrl(), caps); try { driver.get("https://www.google.com"); // The node is still open. Now try to create a second session. It will be added to the queue. // A retry will be attempted and once request times out, it should fail. assertThatThrownBy( () -> { WebDriver disposable = new RemoteWebDriver(server.getUrl(), caps); disposable.quit(); }) .as("Should not have been able to create driver") .isInstanceOf(SessionNotCreatedException.class) .hasMessageContaining("browserName: cheese, se:type: cheddar"); } finally { driver.quit(); } // and wait until the grid says it's ready ... (clipped 10 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: Test-only changes: The changes are confined to tests (assertions, exception expectations, and minor logging) and do not modify production input validation or data handling paths visible in the diff. Referred Code BrowsingContext browsingContext = new BrowsingContext(driver, driver.getWindowHandle()); try { browsingContext.navigate(page, COMPLETE); } catch (BiDiException expectedForChrome) { LOG.log( Level.FINE, "Expected exception for chrome because because the navigation " + "did not complete as expected: {0}", expectedForChrome.toString()); } latch.await(10, TimeUnit.SECONDS); assertThat(status).hasValue(401); } 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	Fix race condition in test Fix a race condition by registering the NodeHeartBeatEvent listener on the EventBus before the LocalNode is created and started. java/test/org/openqa/selenium/grid/distributor/HeartBeatTest.java [37-67] @Test void shouldStartHeartBeatOnNodeStart() throws InterruptedException { EventBus bus = new GuavaEventBus(); + AtomicBoolean heartbeatStarted = new AtomicBoolean(); + CountDownLatch latch = new CountDownLatch(1); + + bus.addListener( + NodeHeartBeatEvent.listener( + nodeStatus -> { + latch.countDown(); + heartbeatStarted.set(true); + })); try (LocalNode node = LocalNode.builder(tracer, bus, routableUri, routableUri, registrationSecret) .add( caps, new TestSessionFactory( (id, c) -> new Session(id, nodeUri, stereotype, c, Instant.now()))) .heartbeatPeriod(Duration.ofSeconds(1)) .build()) { - AtomicBoolean heartbeatStarted = new AtomicBoolean(); - CountDownLatch latch = new CountDownLatch(1); - - bus.addListener( - NodeHeartBeatEvent.listener( - nodeStatus -> { - if (node.getId().equals(nodeStatus.getNodeId())) { - latch.countDown(); - heartbeatStarted.set(true); - } - })); - boolean eventFiredAndListenedTo = latch.await(30, TimeUnit.SECONDS); assertThat(eventFiredAndListenedTo).isTrue(); assertThat(heartbeatStarted.get()).isTrue(); + assertThat(node.getId()).isEqualTo(node.getStatus().getNodeId()); } } Apply / Chat Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a critical race condition in the test logic that could lead to flaky test failures. Fixing this is important for test stability.	High
	Handle browser-specific exception logic correctly Implement browser-specific exception handling to ensure test robustness. Use assertThatThrownBy for Chrome to verify a BiDiException is thrown, and assertThatCode for other browsers to ensure no exception is thrown. java/test/org/openqa/selenium/bidi/network/NetworkCommandsTest.java [266-274] -try { - browsingContext.navigate(page, COMPLETE); -} catch (BiDiException expectedForChrome) { - LOG.log( - Level.FINE, - "Expected exception for chrome because because the navigation " - + "did not complete as expected: {0}", - expectedForChrome.toString()); +if (getBrowser() == CHROME) { + assertThatThrownBy(() -> browsingContext.navigate(page, COMPLETE)) + .isInstanceOf(BiDiException.class); +} else { + assertThatCode(() -> browsingContext.navigate(page, COMPLETE)) + .doesNotThrowAnyException(); } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a flaw where the test would not fail under certain conditions, and proposes a more robust, browser-specific assertion strategy that makes the test's intent clearer and its behavior more reliable.	Medium
	Start the driver service before asserting Start the InternetExplorerDriverService by calling service.start() to ensure the test actually verifies the service can launch, which aligns with the test's purpose. java/test/org/openqa/selenium/ie/InternetExplorerDriverTest.java [156-160] int port = PortProber.findFreePort(); try (InternetExplorerDriverService service = new InternetExplorerDriverService.Builder().usingPort(port).build()) { + service.start(); assertThat(service.getDriverName()).isEqualTo(IE_DRIVER_NAME); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that the InternetExplorerDriverService is never started, meaning the test shouldLaunchSuccessfullyWithArabicDate does not actually test the launch. Adding service.start() fixes this logical flaw in the test.	Medium
High-level	Resolve contradictory import style changes The PR contains conflicting changes regarding import styles. It updates IDE settings to discourage wildcard imports but simultaneously modifies numerous files to use them, creating an inconsistency that should be resolved. Examples: .idea/codeStyles/Project.xml [52-59] <option name="CLASS_COUNT_TO_USE_IMPORT_ON_DEMAND" value="999" /> <option name="NAMES_COUNT_TO_USE_IMPORT_ON_DEMAND" value="999" /> <option name="PACKAGES_TO_USE_IMPORT_ON_DEMAND"> <value> <package name="java.awt" withSubpackages="false" static="false" /> <package name="javax.swing" withSubpackages="false" static="false" /> </value> </option> java/test/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueueTest.java [36-37] Solution Walkthrough: Before: // .idea/codeStyles/Project.xml <JavaCodeStyleSettings> <option name="CLASS_COUNT_TO_USE_IMPORT_ON_DEMAND" value="99" /> <option name="NAMES_COUNT_TO_USE_IMPORT_ON_DEMAND" value="99" /> ... </JavaCodeStyleSettings> // SomeTest.java import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; ... After: // .idea/codeStyles/Project.xml <JavaCodeStyleSettings> <option name="CLASS_COUNT_TO_USE_IMPORT_ON_DEMAND" value="999" /> <option name="NAMES_COUNT_TO_USE_IMPORT_ON_DEMAND" value="999" /> ... </JavaCodeStyleSettings> // SomeTest.java import java.util.*; ... Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a significant contradiction between the IDE configuration change in .idea/codeStyles/Project.xml and the code changes in multiple Java files, which undermines the PR's goal of creating a unified import strategy.	Medium
Learned best practice	Ensure executors are fully terminated Use shutdownNow() and awaitTermination in finally to ensure executor threads are fully terminated and avoid hanging threads in tests. java/test/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueueTest.java [612-643] ExecutorService executor = Executors.newFixedThreadPool(2); try { Callable<HttpResponse> callable = () -> { SessionRequest sessionRequest = new SessionRequest( new RequestId(randomUUID()), Instant.now(), Set.of(W3C), Set.of(CAPS), Map.of(), Map.of()); - return queue.addToQueue(sessionRequest); }; Future<HttpResponse> firstRequest = executor.submit(callable); Future<HttpResponse> secondRequest = executor.submit(callable); HttpResponse firstResponse = firstRequest.get(30, SECONDS); HttpResponse secondResponse = secondRequest.get(30, SECONDS); String firstResponseContents = firstResponse.contentAsString(); String secondResponseContents = secondResponse.contentAsString(); assertThat(firstResponse.getStatus()).isEqualTo(HTTP_OK); assertThat(secondResponse.getStatus()).isEqualTo(HTTP_OK); assertThat(secondResponseContents).isNotEqualTo(firstResponseContents); } finally { - executor.shutdown(); processQueue.set(false); + executor.shutdownNow(); + try { + if (!executor.awaitTermination(5, SECONDS)) { + // fallback to force termination + executor.shutdownNow(); + } + } catch (InterruptedException ie) { + executor.shutdownNow(); + Thread.currentThread().interrupt(); + } } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Always wrap creation of external contexts or resources (executors, threads) in try/finally and perform explicit cleanup in finally to prevent leaks if assertions or exceptions occur.	Low
	Cleanup JMX registrations reliably Ensure the MBean is unregistered in a finally block (or use try-with-resources if available) to prevent leaked registrations affecting other tests. java/test/org/openqa/selenium/grid/router/JmxTest.java [163-194] @Test void shouldBeAbleToRegisterSessionQueue() throws Exception { ObjectName name = new ObjectName("org.seleniumhq.grid:type=SessionQueue,name=LocalSessionQueue"); - new JMXHelper().unregister(name); + JMXHelper jmx = new JMXHelper(); + jmx.unregister(name); + try { + Tracer tracer = DefaultTestTracer.createTracer(); - Tracer tracer = DefaultTestTracer.createTracer(); + NewSessionQueue sessionQueue = + new LocalNewSessionQueue( + tracer, + new DefaultSlotMatcher(), + Duration.ofSeconds(2), + Duration.ofSeconds(2), + Duration.ofSeconds(1), + new Secret(""), + 5); - NewSessionQueue sessionQueue = - new LocalNewSessionQueue( - tracer, - new DefaultSlotMatcher(), - Duration.ofSeconds(2), - Duration.ofSeconds(2), - Duration.ofSeconds(1), - new Secret(""), - 5); + assertThat(sessionQueue).isNotNull(); + MBeanInfo info = beanServer.getMBeanInfo(name); + assertThat(info).isNotNull(); - assertThat(sessionQueue).isNotNull(); - MBeanInfo info = beanServer.getMBeanInfo(name); - assertThat(info).isNotNull(); + MBeanAttributeInfo[] attributeInfoArray = info.getAttributes(); + assertThat(attributeInfoArray).hasSize(1); - MBeanAttributeInfo[] attributeInfoArray = info.getAttributes(); - assertThat(attributeInfoArray).hasSize(1); + AttributeList attributeList = getAttributeList(name, attributeInfoArray); + assertThat(attributeList).isNotNull().hasSize(1); - AttributeList attributeList = getAttributeList(name, attributeInfoArray); - assertThat(attributeList).isNotNull().hasSize(1); - - Object size = beanServer.getAttribute(name, "NewSessionQueueSize"); - assertNumberAttribute(size, 0); + Object size = beanServer.getAttribute(name, "NewSessionQueueSize"); + assertNumberAttribute(size, 0); + } finally { + jmx.unregister(name); + } } Apply / Chat Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Always wrap creation of external contexts or resources in try/finally and perform explicit cleanup; ensure JMX registrations are unregistered to avoid cross-test interference.	Low
More