[java] [refactor] replace most of Guava collections by Java built-in collections #16776

asolntsev · 2025-12-22T10:22:59Z

User description

List.of is preferred because it

Optimized, and
Returns a truly immutable object.

While Arrays.asList returns a list that allows modifications, e.g. using method *.set(index, value).

🔄 Types of changes

Cleanup (formatting, renaming)

PR Type

Enhancement, Tests

Description

This comprehensive refactoring replaces Guava collection utilities with Java built-in equivalents across the Selenium codebase:

Key Changes:

Replaced ImmutableMap.of() with Map.of() for immutable map creation
Replaced ImmutableList.of() and Arrays.asList() with List.of() for immutable lists
Replaced ImmutableSet.of() with Set.of() for immutable sets
Replaced ImmutableMap.copyOf() with Map.copyOf(), ImmutableList.copyOf() with List.copyOf(), and ImmutableSet.copyOf() with Set.copyOf()
Replaced Guava collectors (toImmutableList(), toImmutableSet(), toImmutableMap()) with Java stream collectors (toUnmodifiableList(), toUnmodifiableSet(), toUnmodifiableMap())
Refactored builder patterns to use Java's native collection factory methods
Removed all Guava collection imports (ImmutableList, ImmutableMap, ImmutableSet)
Added static imports for emptyList(), emptyMap(), emptySet() from java.util.Collections

Benefits:

Java built-in collections are optimized and return truly immutable objects
Reduces external dependency on Guava library
Improves code clarity and maintainability
Leverages modern Java features (Java 9+ factory methods)

Scope:

80+ files modified across test and source code
Includes grid, remote, devtools, docker, and configuration modules
Some files include additional refactoring (e.g., extracting helper methods, improving assertions, modernizing test parameterization)

Diagram Walkthrough

flowchart LR
  A["Guava Collections<br/>ImmutableMap/List/Set<br/>Arrays.asList"] -->|Replace| B["Java Built-ins<br/>Map.of/List.of/Set.of<br/>copyOf methods"]
  B -->|Result| C["Optimized<br/>Truly Immutable<br/>No External Dependency"]

File Walkthrough

Relevant files

Cleanup

41 files

RemoteWebDriverUnitTest.java `Replace Guava collections with Java built-in equivalents` java/test/org/openqa/selenium/remote/RemoteWebDriverUnitTest.java Replaced Guava `ImmutableMap.of()` with Java `Map.of()` throughout the test file Replaced Guava `Arrays.asList()` with Java `List.of()` for immutable list creation Added static import for `UUID.randomUUID()` and consolidated wildcard import for `java.util.*` Removed Guava `ImmutableMap` import and consolidated individual `java.util` imports	+51/-71
RemoteWebElementTest.java `Replace Guava ImmutableMap with Java Map.of()` java/test/org/openqa/selenium/remote/RemoteWebElementTest.java Replaced all `ImmutableMap.of()` calls with `Map.of()` throughout test assertions Removed Guava `ImmutableMap` import and added `java.util.Map` import Simplified test code by using Java's native immutable map factory method	+26/-38
GreedySlotSelectorTest.java `Replace Guava collections with Java equivalents in tests` java/test/org/openqa/selenium/grid/distributor/selector/GreedySlotSelectorTest.java Replaced `ImmutableList.of()` with `List.of()` for test data creation Replaced `ImmutableMap.of()` with `Map.of()` for capability maps Removed unused imports for Guava collections and test infrastructure classes Consolidated `java.util.*` imports into wildcard import Removed unused `createNodeWithStereotypes(List)` method overload	+22/-49
JsonOutputTest.java `Replace Guava collections with Java built-in equivalents` java/test/org/openqa/selenium/json/JsonOutputTest.java Replaced all `ImmutableMap.of()` calls with `Map.of()` throughout test file Replaced `Arrays.asList()` with `List.of()` for immutable list creation Removed Guava `ImmutableMap` import and `Arrays` import Removed static import of `asList` from `java.util.Arrays`	+18/-22
ExecutingJavascriptTest.java `Replace Guava collections with Java built-in equivalents` java/test/org/openqa/selenium/ExecutingJavascriptTest.java Replaced `ImmutableList.of()` with `List.of()` for immutable list creation Replaced `ImmutableMap.of()` with `Map.of()` for immutable map creation Replaced `Arrays.asList()` with `List.of()` Removed unused `compareLists()` helper method Removed `@SuppressWarnings("unchecked")` annotation where no longer needed Removed Guava imports for `ImmutableList` and `ImmutableMap`	+15/-38
LocalNodeTest.java `Replace Guava collections with Java equivalents` java/test/org/openqa/selenium/grid/node/local/LocalNodeTest.java Replaced `ImmutableSet.of()` with `Set.of()` for immutable set creation Replaced `ImmutableMap.of()` with `emptyMap()` for empty maps Consolidated `java.util.*` imports into wildcard import Removed Guava `ImmutableMap` and `ImmutableSet` imports Added static import for `emptyMap()` from `java.util.Collections`	+12/-26
DefaultSlotSelectorTest.java `Replace Guava collections with Java built-in equivalents` java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java Replaced `ImmutableList.of()` with `List.of()` for test data Replaced `ImmutableMap.of()` with `Map.of()` for capability maps Consolidated `java.util.*` imports into wildcard import Added `Stream` import for stream operations Changed parameter type from `List` to `List>`	+19/-28
NewSessionPayloadTest.java `Replace Guava collections with Java built-in equivalents` java/test/org/openqa/selenium/remote/NewSessionPayloadTest.java Replaced `ImmutableMap.of()` with `Map.of()` throughout test file Replaced `Arrays.asList()` with `List.of()` for immutable list creation Removed static import of `asList` from `java.util.Arrays` Removed Guava `ImmutableMap` import	+22/-24
NodeTest.java `Replace Guava collections with Java built-in equivalents` java/test/org/openqa/selenium/grid/node/NodeTest.java Replaced `ImmutableSet.of()` with `Set.of()` and `ImmutableSet.copyOf()` with `EnumSet.allOf()` Replaced `ImmutableMap.of()` with `emptyMap()` for empty maps Consolidated multiple static imports into wildcard imports Consolidated `java.util.*` imports into wildcard import Removed Guava `ImmutableMap` and `ImmutableSet` imports Added static imports for `emptyMap()` and `emptySet()` from `java.util.Collections`	+15/-38
LocalNode.java `Replace Guava collections with Java built-in equivalents` java/src/org/openqa/selenium/grid/node/local/LocalNode.java Replaced `ImmutableList.copyOf()` with `List.copyOf()` for immutable list creation Replaced `Arrays.asList()` with `List.of()` for immutable list creation Replaced `toImmutableSet()` collector with `toUnmodifiableSet()` from `java.util.stream.Collectors` Simplified `Ticker` anonymous class to lambda expression Changed `Builder.factories` from `ImmutableList.Builder` to `ArrayList` Removed Guava `ImmutableList` import and related static imports	+11/-25
StubNetworkInterfaceProvider.java `Replace Arrays.asList with List.of()` java/test/org/openqa/selenium/net/StubNetworkInterfaceProvider.java Replaced all `Arrays.asList()` calls with `List.of()` for immutable list creation Changed import from `java.util.Arrays` to `java.util.List`	+11/-11
ContainerConfig.java `Replace Guava collections with Java built-in equivalents` java/src/org/openqa/selenium/docker/ContainerConfig.java Replaced `ImmutableMap.of()` with `Map.of()` throughout the class Replaced `ImmutableList.of()` with `emptyList()` for empty list initialization Replaced `ImmutableMap.copyOf()` with `Map.copyOf()` for immutable map creation Added static import for `emptyList()` from `java.util.Collections` Removed Guava `ImmutableList` and `ImmutableMap` imports Made `toJson()` method return immutable map using `Map.copyOf()`	+13/-13
SessionCapabilitiesMutatorTest.java `Replace Arrays.asList with List.of()` java/test/org/openqa/selenium/grid/node/config/SessionCapabilitiesMutatorTest.java Replaced `Arrays.asList()` with `List.of()` for immutable list creation Replaced `Collections.singletonList()` with `List.of()` for single-element lists Consolidated `java.util.*` imports into wildcard import Removed unnecessary `ArrayList` wrapping of list literals	+12/-19
CustomLocatorHandler.java `Replace Guava collections with Java built-in equivalents` java/src/org/openqa/selenium/grid/node/CustomLocatorHandler.java Replaced `ImmutableSet.of()` with `Set.of()` for immutable set creation Replaced `ImmutableMap.of()` with `Map.of()` for immutable map creation Removed Guava `ImmutableMap` and `ImmutableSet` imports Removed unused `IOException` import	+7/-10
CompoundConfig.java `Replace Guava collections with Java built-in equivalents` java/src/org/openqa/selenium/grid/config/CompoundConfig.java Replaced `ImmutableList.copyOf()` with `List.of()` for immutable list creation Replaced `toImmutableList()` collector with `toUnmodifiableList()` from `java.util.stream.Collectors` Removed Guava `ImmutableList` import and related static imports	+3/-4
NodeOptionsTest.java `Replace Guava collections with Java built-in Map.of()` java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java Removed Guava `ImmutableMap` and `emptyMap` imports, replaced with Java `Map.of()` Replaced `singletonMap()` with `Map.of()` for immutable map creation Replaced `ImmutableMap.of()` with `Map.of()` throughout test cases Simplified `MapConfig(emptyMap())` to `MapConfig()` constructor call	+12/-19
JsonTest.java `Replace Guava collections with Java built-in Map and List` java/test/org/openqa/selenium/json/JsonTest.java Removed Guava `ImmutableMap` import, replaced with Java `Map.of()` Replaced `Arrays.asList()` with `List.of()` for immutable lists Replaced all `ImmutableMap.of()` calls with `Map.of()`	+9/-15
ConfigTest.java `Replace Guava immutable collections with Java built-ins` java/test/org/openqa/selenium/grid/config/ConfigTest.java Removed Guava `ImmutableList` and `ImmutableMap` imports Replaced `ImmutableMap.of()` with `Map.of()` throughout Replaced `ImmutableList.of()` with `List.of()` Simplified `MapConfig(ImmutableMap.of())` to `MapConfig()` constructor	+14/-17
NewSessionCreationTest.java `Replace Guava collections with Java built-in equivalents` java/test/org/openqa/selenium/grid/router/NewSessionCreationTest.java Removed Guava `ImmutableList`, `ImmutableMap`, `ImmutableSet` imports Added `emptyList()` and `emptySet()` static imports Replaced `ImmutableMap.of()` with `Map.of()` Replaced `ImmutableList.of()` with `emptyList()` Replaced `ImmutableSet.of()` with `emptySet()` Simplified `MapConfig(ImmutableMap.of())` to `MapConfig()`	+14/-18
ErrorHandlerTest.java `Replace Guava ImmutableMap with Java Map.of()` java/test/org/openqa/selenium/remote/ErrorHandlerTest.java Removed Guava `ImmutableMap` import Replaced all `ImmutableMap.of()` calls with `Map.of()`	+10/-14
WebElementToJsonConverterTest.java `Replace Guava collections with Java built-in Map and List` java/test/org/openqa/selenium/remote/WebElementToJsonConverterTest.java Removed Guava `ImmutableMap` import Replaced `ImmutableMap.of()` with `Map.of()` Replaced `asList()` calls with `List.of()`	+8/-22
LocalNodeRegistry.java `Replace Guava immutable collections with Java built-ins` java/src/org/openqa/selenium/grid/distributor/local/LocalNodeRegistry.java Removed Guava `ImmutableMap` and `ImmutableSet` imports Added stream collector import for `toUnmodifiableSet()` Replaced `ImmutableMap.copyOf()` with `Map.copyOf()` Replaced `ImmutableSet.toImmutableSet()` with `toUnmodifiableSet()` Replaced `ImmutableSet.of()` with `Set.of()`	+6/-9
SessionQueueGridTest.java `Replace Guava collections with Java built-in Map and List` java/test/org/openqa/selenium/grid/router/SessionQueueGridTest.java Removed Guava `ImmutableMap` import Removed `Arrays` import, replaced `Arrays.asList()` with `List.of()` Replaced `ImmutableMap.of()` with `Map.of()`	+8/-11
JsonConfigTest.java `Replace Arrays.asList with List.of() and simplify assertions` java/test/org/openqa/selenium/grid/config/JsonConfigTest.java Removed `Arrays` import, replaced `Arrays.asList()` with `List.of()` Added `emptyList()` static import Simplified test assertions using `containsExactly()` instead of comparing lists	+11/-17
DriverServiceSessionFactoryTest.java `Replace Guava collections with Java built-in equivalents` java/test/org/openqa/selenium/grid/node/config/DriverServiceSessionFactoryTest.java Removed Guava `ImmutableMap` and `ImmutableSet` imports Added `emptyMap()` and `emptySet()` static imports Replaced `ImmutableSet.of()` with `Set.of()` Replaced `ImmutableMap.of()` with `emptyMap()`	+8/-12
RemoteLogsTest.java `Replace Guava ImmutableMap with Java Map.of()` java/test/org/openqa/selenium/remote/RemoteLogsTest.java Removed Guava `ImmutableMap` import Replaced `ImmutableMap.of()` with `Map.of()`	+8/-16
CreateSessionTest.java `Replace Guava collections with Java built-in equivalents` java/test/org/openqa/selenium/grid/node/local/CreateSessionTest.java Removed Guava `ImmutableMap` and `ImmutableSet` imports Added `emptyMap()` static import Replaced `ImmutableMap.of()` with `Map.of()` Replaced `ImmutableSet.of()` with `Set.of()` Replaced `ImmutableMap.of()` with `emptyMap()`	+8/-12
NettyServerTest.java `Replace Guava ImmutableMap with Java Map.of()` java/test/org/openqa/selenium/netty/server/NettyServerTest.java Removed Guava `ImmutableMap` import Replaced `ImmutableMap.of()` with `Map.of()` Simplified `MapConfig(ImmutableMap.of())` to `MapConfig()`	+6/-14
DeploymentTypes.java `Replace Arrays with List.of() and simplify string arrays` java/test/org/openqa/selenium/grid/router/DeploymentTypes.java Removed `Arrays` import Replaced `new String[] {...}` array with varargs in `String.join()` call Replaced `Arrays.asList()` with `List.of()`	+14/-23
EnsureSpecCompliantHeadersTest.java `Replace Guava collections with Java built-in equivalents` java/test/org/openqa/selenium/grid/web/EnsureSpecCompliantHeadersTest.java Removed Guava `ImmutableList` and `ImmutableSet` imports Added `emptyList()` and `emptySet()` static imports Replaced `ImmutableList.of()` with `emptyList()` and `List.of()` Replaced `ImmutableSet.of()` with `emptySet()` and `Set.of()`	+9/-7
SessionCleanUpTest.java `Replace Guava collections with Java built-in equivalents` java/test/org/openqa/selenium/grid/router/SessionCleanUpTest.java Removed Guava `ImmutableMap` and `ImmutableSet` imports Replaced `ImmutableMap.of()` with `Map.of()` Replaced `ImmutableSet.of()` with `Set.of()` Simplified `MapConfig(ImmutableMap.of())` to `MapConfig()`	+11/-16
UrlTemplateTest.java `Replace Guava ImmutableMap with Java Map.of()` java/test/org/openqa/selenium/remote/http/UrlTemplateTest.java Removed Guava `ImmutableMap` import Replaced `ImmutableMap.of()` with `Map.of()`	+6/-7
LocalNewSessionQueueTest.java `Replace Guava ImmutableMap with Java Map.of()` java/test/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueueTest.java Removed Guava `ImmutableMap` import Replaced `ImmutableMap.of()` with `Map.of()`	+12/-13
ConcatenatingConfig.java `Replace Guava collections with Java stream collectors` java/src/org/openqa/selenium/grid/config/ConcatenatingConfig.java Removed Guava `ImmutableList` and `ImmutableMap` imports Added stream collector import for `toUnmodifiableMap()` Replaced `ImmutableMap.toImmutableMap()` with `toUnmodifiableMap()` Replaced `ImmutableList::of` with `List::of`	+4/-10
DockerOptionsTest.java `Replace Arrays with Stream.of() and List.of()` java/test/org/openqa/selenium/grid/node/docker/DockerOptionsTest.java Removed `Arrays` import and `asList` static import Refactored test data provider from `Arrays.stream()` with array to `Stream.of()` with varargs Replaced `Arrays.asList()` with `List.of()`	+16/-26
CdpVersionFinderTest.java `Replace Guava ImmutableList with Java List.of()` java/test/org/openqa/selenium/devtools/CdpVersionFinderTest.java Removed Guava `ImmutableList` import Replaced `ImmutableList.of()` with `List.of()`	+5/-5
ChromeDriverServiceTest.java `Replace Arrays.asList with List.of()` java/test/org/openqa/selenium/chrome/ChromeDriverServiceTest.java Removed `Arrays` import Replaced `Arrays.asList()` with `List.of()`	+5/-6
EdgeDriverServiceTest.java `Replace Arrays.asList with List.of()` java/test/org/openqa/selenium/edge/EdgeDriverServiceTest.java Removed `Arrays` import Replaced `Arrays.asList()` with `List.of()`	+5/-6
W3CHandshakeResponseTest.java `Replace Guava ImmutableMap with Java Map.of()` java/test/org/openqa/selenium/remote/W3CHandshakeResponseTest.java Removed Guava `ImmutableMap` import Replaced `ImmutableMap.of()` with `Map.of()`	+5/-9
EventBusCommand.java `Replace Guava collections with Java built-in equivalents` java/src/org/openqa/selenium/grid/commands/EventBusCommand.java Removed Guava `ImmutableMap` and `ImmutableSet` imports Replaced `ImmutableSet.of()` with `Set.of()` Replaced `ImmutableMap.of()` with `Map.of()`	+7/-12
Node.java `Replace Guava ImmutableMap with Java Map.of()` java/src/org/openqa/selenium/grid/node/Node.java Removed Guava `ImmutableMap` import Changed `OS_INFO` field type from `ImmutableMap` to `Map` Replaced `ImmutableMap.of()` with `Map.of()` in `loadOsInfo()` method Updated method return type from `ImmutableMap` to `Map`	+4/-5

Enhancement

44 files

CdpClientGenerator.java `Refactor to use Java built-in collections and improve code clarity` java/src/org/openqa/selenium/devtools/CdpClientGenerator.java Replaced `ImmutableMap.Builder` patterns with `Map.of()` for simpler map creation Added new helper method `mergeMaps()` to combine maps and return unmodifiable result Replaced `Arrays.asList()` with `List.of()` and added `toUnmodifiableList()` collector import Improved error message formatting in exception handling with `String.format()` Refactored lambda expressions to use block statements for better readability	+119/-108
ReferenceTest.java `Modernize parameterized test data structure` java/test/org/openqa/selenium/docker/internal/ReferenceTest.java Converted test data from `Arrays.asList()` with nested array to `Stream.of()` with `Arguments.of()` Changed method visibility from public to private for `data()` method Removed `Arrays` import and added `Stream` import Improved test parameterization using JUnit 5 stream-based approach	+52/-74
CapabilitiesTest.java `Replace Guava maps and improve assertion methods` java/test/org/openqa/selenium/CapabilitiesTest.java Replaced `ImmutableMap.of()` with `Map.of()` for capability assertions Changed assertion methods from `isEqualTo()` to `containsExactlyInAnyOrderEntriesOf()` for map comparisons Updated hash code assertions to use `hasSameHashCodeAs()` matcher Removed Guava `ImmutableMap` import	+21/-20
SelectTest.java `Replace Arrays.asList and improve mock setup` java/test/org/openqa/selenium/support/ui/SelectTest.java Replaced `Arrays.asList()` with `List.of()` for immutable list creation Added mock setup for `getCssValue()` method in mock options Changed import from `ArgumentMatchers` class to static import of `any()` method Renamed poorly named mock variable from `optionBad` to `option`	+18/-16
Reference.java `Replace ImmutableMap with typed DockerDomain class` java/src/org/openqa/selenium/docker/internal/Reference.java Replaced `ImmutableMap.of()` with new `DockerDomain` record class for better type safety Created `DockerDomain` inner class to replace map-based return value Renamed method from `splitDockerDomain()` to `parseDockerDomain()` for clarity Changed `toLowerCase()` to `toLowerCase(ROOT)` for explicit locale specification Removed Guava `ImmutableMap` import	+29/-19
MapConfig.java `Refactor MapConfig to use Java built-in collections` java/src/org/openqa/selenium/grid/config/MapConfig.java Removed Guava `ImmutableList`, `ImmutableMap`, `ImmutableSet` imports Added `emptyMap()` static import and stream collectors for unmodifiable collections Added no-argument constructor that delegates to `emptyMap()` Refactored map building logic to use stream collectors instead of `ImmutableMap.builder()` Replaced `ImmutableSet.copyOf()` with `Set.copyOf()` and `ImmutableList.of()` with `List.of()`	+27/-31
Grid.java `Refactor Grid.getNodes() to use streams and Java collectors` java/src/org/openqa/selenium/grid/graphql/Grid.java Removed Guava `ImmutableList` import, added stream collector import Refactored `getNodes()` method from builder pattern to stream-based approach Replaced `ImmutableList.builder()` with `stream().map().collect(toUnmodifiableList())`	+39/-45
GridStatusHandler.java `Replace Guava ImmutableMap and extract node mapping logic` java/src/org/openqa/selenium/grid/router/GridStatusHandler.java Removed Guava `ImmutableMap` import Replaced `ImmutableMap.of()` with `Map.of()` Extracted node mapping logic into separate `nodeAsMap()` helper method Simplified status response building using `Map.of()` instead of builder pattern	+27/-28
InternetExplorerOptions.java `Replace Arrays with Set.of() and List.of() in IE options` java/src/org/openqa/selenium/ie/InternetExplorerOptions.java Removed `Arrays` import, added `emptyList()` static import Changed `CAPABILITY_NAMES` from `List` to `Set` using `Set.of()` Refactored `addCommandSwitches()` to use `List.of()` instead of `Arrays.asList()` Improved null handling and type safety in switch processing logic	+11/-10
DefaultSlotMatcher.java `Replace Arrays.asList with List.of for immutability` java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java Removed `java.util.Arrays` import Replaced three `Arrays.asList()` calls with `List.of()` for immutable list creation Applied to static final list constants for capabilities prefixes	+3/-4
ExpectedConditionsTest.java `Replace Arrays.asList with List.of in tests` java/test/org/openqa/selenium/support/ui/ExpectedConditionsTest.java Removed `java.util.Arrays` import Replaced three `Arrays.asList()` calls with `List.of()` in test mock setup	+3/-4
SafariTechPreviewDriverService.java `Simplify immutable collection creation with copyOf methods` java/src/org/openqa/selenium/safari/SafariTechPreviewDriverService.java Replaced wildcard import with `java.util.*` Removed `unmodifiableMap` import, replaced with `Map.copyOf()` Simplified constructor to use `List.copyOf()` and `Map.copyOf()` instead of wrapping with `ArrayList` and `HashMap` Refactored `createArgs()` to use `ArrayList` constructor with capacity and manual adds, returning `unmodifiableList()`	+6/-14
RetryRequestTest.java `Replace Guava ImmutableMap with Java Map.of` java/test/org/openqa/selenium/remote/http/RetryRequestTest.java Removed Guava `ImmutableMap` import Added `java.util.Map` import Replaced three `ImmutableMap.of()` calls with `Map.of()`	+4/-4
Config.java `Replace Guava ImmutableList with List.copyOf` java/src/org/openqa/selenium/grid/config/Config.java Replaced wildcard import for java.util classes Removed Guava `ImmutableList` import Replaced `ImmutableList.builder().addAll().build()` with `List.copyOf()`	+2/-7
CustomLocatorHandlerTest.java `Replace Guava ImmutableMap with Java Map.of` java/test/org/openqa/selenium/grid/node/CustomLocatorHandlerTest.java Removed Guava `ImmutableMap` import Replaced four `ImmutableMap.of()` calls with `Map.of()`	+4/-5
OneShotNode.java `Replace Guava immutable collections with Java built-ins` java/src/org/openqa/selenium/grid/node/k8s/OneShotNode.java Removed Guava `ImmutableMap` and `ImmutableSet` imports Replaced wildcard import for java.util classes Replaced six `ImmutableMap.of()` calls with `Map.of()` Replaced one `ImmutableSet.of()` call with `Set.of()`	+6/-12
RemoteWebDriverBuilderTest.java `Replace Guava ImmutableMap with Java Map.of` java/test/org/openqa/selenium/remote/RemoteWebDriverBuilderTest.java Removed Guava `ImmutableMap` import Replaced eight `ImmutableMap.of()` calls with `Map.of()`	+8/-9
Keys.java `Replace Arrays.asList with List.of` java/src/org/openqa/selenium/Keys.java Changed import from `java.util.Arrays` to `java.util.List` Replaced `Arrays.asList()` with `List.of()` in `chord()` method	+2/-2
ProxyWebsocketTest.java `Replace Guava immutable collections with Java built-ins` java/test/org/openqa/selenium/grid/router/ProxyWebsocketTest.java Removed Guava `ImmutableMap` and `ImmutableSet` imports Added `java.util.Map` import Replaced `ImmutableSet.of().stream()` with `Stream.of()` for test data Replaced two `ImmutableMap.of()` calls with `Map.of()`	+3/-5
DrainNode.java `Replace Guava ImmutableMap with Java Map.of` java/src/org/openqa/selenium/grid/distributor/DrainNode.java Removed Guava `ImmutableMap` import Added `java.util.Map` import Replaced two `ImmutableMap.of()` calls with `Map.of()`	+3/-4
ReverseProxyHandler.java `Replace Guava ImmutableSet with Java Set.of` java/src/org/openqa/selenium/grid/web/ReverseProxyHandler.java Removed Guava `ImmutableSet` import and unused Logger field Added `java.util.Set` import Replaced `ImmutableSet.builder()` pattern with `Set.of()` for header names constant	+13/-17
CheckContentTypeHeader.java `Replace Guava immutable collections with Java built-ins` java/src/org/openqa/selenium/grid/web/CheckContentTypeHeader.java Removed Guava `ImmutableMap` and `ImmutableSet` imports Added `java.util.Map` import Replaced four `ImmutableMap.of()` calls with `Map.of()` Replaced `ImmutableSet.copyOf()` with `Set.copyOf()`	+6/-8
NewNodeSession.java `Replace Guava ImmutableMap with Java Map.of` java/src/org/openqa/selenium/grid/node/NewNodeSession.java Removed Guava `ImmutableMap` import Added `java.util.Map` import Replaced `ImmutableMap.of()` with `Map.of()`	+2/-2
SafariDriverService.java `Simplify immutable collection creation with copyOf methods` java/src/org/openqa/selenium/safari/SafariDriverService.java Removed `unmodifiableMap` import and `Arrays` import Removed `HashMap` import Replaced constructor to use `List.copyOf()` and `Map.copyOf()` instead of wrapping Refactored `createArgs()` to use `ArrayList` with capacity and manual adds, returning `unmodifiableList()`	+5/-11
ChromiumOptions.java `Replace Arrays.asList with List.of` java/src/org/openqa/selenium/chromium/ChromiumOptions.java Replaced three `Arrays.asList()` calls with `List.of()` in method overloads	+3/-3
Proxy.java `Replace Arrays.asList with List.of` java/src/org/openqa/selenium/Proxy.java Removed `java.util.Arrays` import Replaced `Arrays.asList()` with `List.of()` in `toJson()` method	+1/-2
DescribedOption.java `Replace Guava ImmutableSet with Java Set.copyOf` java/src/org/openqa/selenium/grid/config/DescribedOption.java Removed Guava `ImmutableSet` import Replaced `ImmutableSet.copyOf()` with `Set.copyOf()`	+1/-2
StatusHandler.java `Replace Guava ImmutableMap with Java Map.of` java/src/org/openqa/selenium/grid/node/StatusHandler.java Removed Guava `ImmutableMap` import Added `java.util.Map` import Replaced three `ImmutableMap.of()` calls with `Map.of()`	+4/-4
FirefoxDriverTest.java `Replace Guava ImmutableMap and improve resource handling` java/test/org/openqa/selenium/firefox/FirefoxDriverTest.java Removed Guava `ImmutableMap` import Added `java.util.Map` import Replaced `ImmutableMap.of()` with `Map.of()` Refactored test to use try-with-resources for proper resource management	+5/-6
TemplateGridServerCommand.java `Replace Arrays.asList with List.of` java/src/org/openqa/selenium/grid/TemplateGridServerCommand.java Removed `java.util.Arrays` import Replaced `Arrays.asList()` with `List.of()` in route building logic	+1/-2
StatusHandler.java `Replace Guava ImmutableMap with Java Map.of` java/src/org/openqa/selenium/grid/distributor/StatusHandler.java Removed Guava `ImmutableMap` import Added `java.util.Map` import Replaced three `ImmutableMap.of()` calls with `Map.of()`	+4/-4
GetNodeSession.java `Replace Guava ImmutableMap with Java Map.of` java/src/org/openqa/selenium/grid/node/GetNodeSession.java Removed Guava `ImmutableMap` import Added `java.util.Map` import Replaced `ImmutableMap.of()` with `Map.of()`	+2/-2
IsSessionOwner.java `Replace Guava ImmutableMap with Java Map.of` java/src/org/openqa/selenium/grid/node/IsSessionOwner.java Removed Guava `ImmutableMap` import Added `java.util.Map` import Replaced `ImmutableMap.of()` with `Map.of()`	+2/-2
GetContainerLogs.java `Replace Arrays.asList with List.of` java/src/org/openqa/selenium/docker/client/GetContainerLogs.java Removed `java.util.Arrays` import Replaced `Arrays.asList()` with `List.of()` for log lines	+1/-2
DriverServiceSessionFactory.java `Replace Arrays.asList with List.of` java/src/org/openqa/selenium/grid/node/config/DriverServiceSessionFactory.java Removed `java.util.Arrays` import Replaced `Arrays.asList()` with `List.of()` for vendor options capabilities	+1/-2
EdgeOptionsTest.java `Replace Arrays.asList with List.of` java/test/org/openqa/selenium/edge/EdgeOptionsTest.java Replaced wildcard import for java.util classes Replaced two `Arrays.asList()` calls with `List.of()`	+4/-7
CheckOriginHeader.java `Replace Guava immutable collections with Java built-ins` java/src/org/openqa/selenium/grid/web/CheckOriginHeader.java Removed Guava `ImmutableMap` and `ImmutableSet` imports Added `java.util.Map` import Replaced two `ImmutableSet.copyOf()` calls with `Set.copyOf()` Replaced four `ImmutableMap.of()` calls with `Map.of()`	+5/-7
ImageSummary.java `Replace Guava ImmutableSet with Java Set.copyOf` java/src/org/openqa/selenium/docker/internal/ImageSummary.java Removed Guava `ImmutableSet` import Replaced `ImmutableSet.copyOf()` with `Set.copyOf()`	+1/-2
EndToEndTest.java `Replace Guava immutable collections with Java built-ins` java/test/org/openqa/selenium/grid/router/EndToEndTest.java Removed Guava `ImmutableMap` and `ImmutableSet` imports Replaced `ImmutableSet.of().stream()` with `Stream.of()` for test data Replaced three `ImmutableMap.of()` calls with `Map.of()`	+2/-7
AddingNodesTest.java `Replace Guava immutable collections with Java built-ins` java/test/org/openqa/selenium/grid/distributor/AddingNodesTest.java Removed Guava `ImmutableMap` and `ImmutableSet` imports Replaced two `ImmutableSet.of()` calls with `Set.of()` Replaced `ImmutableMap.copyOf()` with `Map.copyOf()`	+3/-6
DefaultHubConfig.java `Replace Guava ImmutableMap with Java Map.of` java/src/org/openqa/selenium/grid/commands/DefaultHubConfig.java Removed Guava `ImmutableMap` import Added `java.util.Map` import Replaced three `ImmutableMap.of()` calls with `Map.of()`	+5/-5
SelectElementTest.java `Replace Arrays.asList with List.of` java/test/org/openqa/selenium/support/ui/SelectElementTest.java Removed `java.util.Arrays` import Replaced two `Arrays.asList()` calls with `List.of()`	+2/-3
Hub.java `Replace Guava ImmutableSet with Java Set.of` java/src/org/openqa/selenium/grid/commands/Hub.java Removed Guava `ImmutableSet` import Replaced `ImmutableSet.of()` with `Set.of()` in `getConfigurableRoles()`	+1/-3
LocalDistributor.java `Replace Guava ImmutableSet with Java Set.of` java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java Removed Guava `ImmutableSet` import Replaced `ImmutableSet.of()` with `Set.of()` in `isReady()` method	+1/-2

Additional files

101 files

javascriptPage.html	+6/-1
ListImages.java	+3/-4
FirefoxOptions.java	+2/-3
GeckoDriverService.java	+1/-2
DefaultStandaloneConfig.java	+4/-5
Standalone.java	+1/-3
AnnotatedConfig.java	+4/-4
ConfigFlags.java	+3/-10
EnvConfig.java	+1/-2
StandardGridRoles.java	+8/-13
TomlConfig.java	+5/-8
GetDistributorStatus.java	+2/-2
DefaultDistributorConfig.java	+4/-4
DistributorServer.java	+4/-5
LocalGridModel.java	+1/-2
Node.java	+4/-3
LoggingOptions.java	+1/-1
CapabilityResponseEncoder.java	+11/-10
ProxyNodeWebsockets.java	+3/-3
SessionCapabilitiesMutator.java	+2/-3
DockerSessionFactory.java	+2/-3
DefaultNodeConfig.java	+4/-4
NodeServer.java	+1/-2
RemoteNode.java	+2/-4
Router.java	+2/-2
RouterServer.java	+3/-5
RequiresSecretFilter.java	+3/-4
EventBusFlags.java	+1/-2
NetworkOptions.java	+4/-4
AddToSessionMap.java	+2/-2
GetFromSessionMap.java	+2/-2
DefaultSessionMapConfig.java	+5/-5
SessionMapServer.java	+4/-5
RedisBackedSessionMap.java	+2/-4
ClearSessionQueue.java	+4/-4
DefaultNewSessionQueueConfig.java	+2/-2
NewSessionQueueServer.java	+4/-5
LocalNewSessionQueue.java	+4/-5
GridUiRoute.java	+1/-2
JarFileResource.java	+5/-3
MergedResource.java	+3/-4
NoHandler.java	+1/-2
PathResource.java	+2/-2
RoutableHttpClientFactory.java	+3/-4
Actions.java	+1/-1
InstanceCoercer.java	+1/-2
JsonInput.java	+1/-2
NetworkInterface.java	+2/-5
RequestConverter.java	+3/-4
ExecutableFinder.java	+2/-3
DriverFinder.java	+1/-2
ThreadGuard.java	+2/-2
EventFiringDecorator.java	+2/-3
ExpectedConditions.java	+2/-3
Select.java	+2/-2
ElementAttributeTest.java	+1/-2
ElementDomAttributeTest.java	+1/-2
ProxyTest.java	+2/-2
ScriptPinningTest.java	+1/-2
UploadTest.java	+1/-2
WindowSwitchingTest.java	+2/-1
Build.java	+1/-2
DevMode.java	+1/-2
ChromeOptionsFunctionalTest.java	+2/-3
ChromeOptionsTest.java	+2/-3
NettyAppServer.java	+2/-2
FirefoxOptionsTest.java	+3/-6
AnnotatedConfigTest.java	+4/-6
ConcatenatingConfigTest.java	+6/-6
MapConfigTest.java	+6/-7
TomlConfigTest.java	+6/-7
NodeStatusTest.java	+6/-6
DistributorTest.java	+2/-3
DistributorTestBase.java	+3/-4
GraphqlHandlerTest.java	+3/-4
OverallGridTest.java	+2/-3
DistributedCdpTest.java	+1/-2
DistributedTest.java	+2/-3
JmxTest.java	+3/-5
ReverseProxyEndToEndTest.java	+1/-2
RouterTest.java	+5/-6
SessionQueueGridWithTimeoutTest.java	+5/-7
StressTest.java	+1/-1
NetworkOptionsTest.java	+1/-2
ClassPathResourceTest.java	+1/-2
PerformanceLoggingMockTest.java	+3/-5
WebSocketServingTest.java	+2/-3
AugmenterTest.java	+1/-2
DesiredCapabilitiesTest.java	+3/-5
ProtocolHandshakeTest.java	+1/-6
RemotableByTest.java	+5/-6
RemoteWebDriverInitializationTest.java	+3/-3
W3CHttpResponseCodecTest.java	+4/-6
PrefixedRouteTest.java	+1/-2
HttpClientTestBase.java	+4/-6
WebSocketTestBase.java	+2/-3
TracerTest.java	+1/-5
ByChainedTest.java	+1/-2
DefaultElementLocatorTest.java	+4/-5
LocatingElementListHandlerTest.java	+3/-4
Additional files not shown

selenium-ci · 2025-12-22T10:23:21Z

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 · 2025-12-22T10:24:13Z

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: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

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

qodo-code-review · 2025-12-22T10:26:05Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Use DOMContentLoaded listener Replace the `setTimeout` with a `DOMContentLoaded` event listener to show the 'close' button, avoiding a potential race condition. common/src/web/javascriptPage.html [286-290] <script type="application/javascript"> - setTimeout(() => { - document.getElementById('close').style.display = 'block'; - }, 100); + document.addEventListener('DOMContentLoaded', () => { + const btn = document.getElementById('close'); + if (btn) { + btn.style.display = 'block'; + } + }); </script> Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential race condition and replaces a fixed timeout with a more robust `DOMContentLoaded` event listener, improving test reliability.	Medium
General	Handle CRLF when splitting logs Update the log splitting logic to handle both Windows (`\r\n`) and Unix (`\n`) line endings by using the regex `\r?\n`. java/src/org/openqa/selenium/docker/client/GetContainerLogs.java [55] -List<String> logLines = List.of(Contents.string(res).split("\n")); +List<String> logLines = List.of(Contents.string(res).split("\\r?\\n")); Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion improves cross-platform compatibility by handling both Windows (CRLF) and Unix (LF) line endings when parsing container logs.	Medium
Possible issue	Restore sorted ALL_ROLES set Revert from `Set.of()` to a `TreeSet` wrapped in an unmodifiable set for `ALL_ROLES` to preserve the sorted iteration order. java/src/org/openqa/selenium/grid/config/StandardGridRoles.java [35-42] public static final Set<Role> ALL_ROLES = - Set.of( - DISTRIBUTOR_ROLE, - EVENT_BUS_ROLE, - NODE_ROLE, - ROUTER_ROLE, - SESSION_MAP_ROLE, - SESSION_QUEUE_ROLE); + Collections.unmodifiableSet( + new TreeSet<>( + List.of( + DISTRIBUTOR_ROLE, + EVENT_BUS_ROLE, + NODE_ROLE, + ROUTER_ROLE, + SESSION_MAP_ROLE, + SESSION_QUEUE_ROLE))); Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that changing from `TreeSet` to `Set.of()` loses the guaranteed sorted order, which could be an unintended side effect.	Low
Learned best practice	Prevent null copy NPEs `List.copyOf`/`Map.copyOf` will throw `NullPointerException` when `args` or `environment` is null; defensively default to empty collections before copying to preserve immutability without breaking nullable inputs. java/src/org/openqa/selenium/safari/SafariDriverService.java [71] -super(executable, port, timeout, List.copyOf(args), Map.copyOf(environment)); +super( + executable, + port, + timeout, + args == null ? List.of() : List.copyOf(args), + environment == null ? Map.of() : Map.copyOf(environment)); Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Use defensive copying safely (avoid null-sensitive immutable copies) when storing constructor inputs to prevent runtime failures and unintended mutation.	Low
Learned best practice	Reuse precomputed timezone IDs Avoid rebuilding a list of all time zone IDs on each call; precompute a static `Set` (or helper) once and reuse it for validation. java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java [459-466] -if (List.of(TimeZone.getAvailableIDs()).contains(tz)) { +private static final Set<String> AVAILABLE_TIMEZONES = Set.of(TimeZone.getAvailableIDs()); +... +if (AVAILABLE_TIMEZONES.contains(tz)) { return TimeZone.getTimeZone(tz); } ... -if (List.of(TimeZone.getAvailableIDs()).contains(envTz)) { +if (AVAILABLE_TIMEZONES.contains(envTz)) { return TimeZone.getTimeZone(envTz); } `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Replace repeated ad-hoc logic with a shared helper/constant to centralize behavior and avoid repeated work.	Low
Update

qodo-code-review · 2025-12-22T17:50:20Z

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: 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: Secrets in parameters: The new test parameters include embedded credentials and a secret token that could be exposed if propagated into user-facing exception messages without redaction. Referred Code final Map<String, String> parameters = Map.of("url", "https://user:[email protected]", "token", "12345Secret"); final CommandPayload commandPayload = new CommandPayload(DriverCommand.GET, parameters); 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: Sensitive data in logs: The added test data contains a `user:password@` URL and a `token` value, which could leak into logs depending on how command parameters are logged in failure/debug paths. Referred Code final Map<String, String> parameters = Map.of("url", "https://user:[email protected]", "token", "12345Secret"); final CommandPayload commandPayload = new CommandPayload(DriverCommand.GET, parameters); 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-22T17:52:21Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Correctly convert file array to list Replace `List.of(files)` with `Arrays.asList(files)` to correctly convert the `File[]` array into a `List`, preventing a bug where the list would contain the array as a single element. java/src/org/openqa/selenium/grid/node/local/LocalNode.java [884-887] private static List<File> downloadedFiles(File downloadsDirectory) { File[] files = requireNonNullElseGet(downloadsDirectory.listFiles(), () -> new File[0]); - return List.of(files); + return Arrays.asList(files); } `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a critical bug introduced in the PR where `List.of(files)` would create a `List<File[]>` instead of the expected `List<File>`, which would break functionality that relies on the list of files.	High
	Expand split hosts correctly Fix a bug in argument parsing by using `Arrays.asList()` instead of `List.of()` to correctly expand the `allowHosts` array into a list of strings. java/src/org/openqa/selenium/firefox/GeckoDriverService.java [292-295] if (allowHosts != null) { args.add("--allow-hosts"); - args.addAll(List.of(allowHosts.split(" "))); + args.addAll(java.util.Arrays.asList(allowHosts.split(" "))); } Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a bug where `List.of(String[])` creates a `List<String[]>` instead of a `List<String>`, which would break the logic for adding host arguments.	High
	Expand no-proxy entries properly Fix a bug in proxy configuration by using `Arrays.asList()` instead of `List.of()` to correctly expand the `noProxy` array into a list of strings. java/src/org/openqa/selenium/Proxy.java [143-145] if (noProxy != null) { - m.put(NO_PROXY, List.of(noProxy.split(",\\s"))); + m.put(NO_PROXY, java.util.Arrays.asList(noProxy.split(",\\s"))); } Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a bug where `List.of(String[])` creates a `List<String[]>` instead of a `List<String>`, which would break the `noProxy` configuration.	High
	Split PATH into segments correctly Fix a bug in path parsing by using `Arrays.asList()` instead of `List.of()` to correctly split the PATH environment variable into a list of individual paths. java/src/org/openqa/selenium/os/ExecutableFinder.java [86] return path != null - ? List.of(path.split(File.pathSeparator)) + ? java.util.Arrays.asList(path.split(File.pathSeparator)) : emptyList(); `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a bug where `List.of(String[])` creates a `List<String[]>` instead of a `List<String>`, which would break the logic for finding executables in the system's PATH.	High
	Prevent NullPointerException from nullable parameters Prevent a `NullPointerException` by checking for `null` on the `@Nullable` `args` and `environment` parameters before calling `List.copyOf()` and `Map.copyOf()`. java/src/org/openqa/selenium/safari/SafariDriverService.java [71] -super(executable, port, timeout, List.copyOf(args), Map.copyOf(environment)); +super( + executable, + port, + timeout, + List.copyOf(args == null ? List.of() : args), + Map.copyOf(environment == null ? Map.of() : environment)); `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that `List.copyOf` and `Map.copyOf` will throw a `NullPointerException` if passed `null`, which is possible given the `@Nullable` annotations on the `args` and `environment` parameters, thus fixing a potential crash.	Medium
Learned best practice	Fix incorrect map merging Ensure the returned maps include both `always` and each `first` entry by merging into a new map before wrapping with `Map.copyOf()`. java/test/org/openqa/selenium/remote/ProtocolHandshakeTest.java [157-158] List<Map<String, Object>> allCaps = - firsts.stream().map(first -> Map.copyOf(always)).collect(toList()); + firsts.stream() + .map( + first -> { + Map<String, Object> merged = new HashMap<>(always); + merged.putAll(first); + return Map.copyOf(merged); + }) + .collect(toList()); `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Use defensive copying and immutability for maps returned or stored to prevent unintended side effects and preserve intended merged content.	Low
Learned best practice	Add defensive type checks Guard the non-`String` branch with an `instanceof List` check (or handle unexpected types by treating them as empty) to avoid `ClassCastException` when `IE_SWITCHES` is set to a non-list value. java/src/org/openqa/selenium/ie/InternetExplorerOptions.java [150-167] public InternetExplorerOptions addCommandSwitches(String... switches) { final Object raw = getCapability(IE_SWITCHES); final List<?> rawSwitches; if (raw == null) { rawSwitches = emptyList(); } else if (raw instanceof String) { rawSwitches = List.of(((String) raw).split(" ")); + } else if (raw instanceof List) { + rawSwitches = (List<?>) raw; } else { - rawSwitches = (List<?>) raw; + rawSwitches = emptyList(); } return amend( IE_SWITCHES, Stream.concat(rawSwitches.stream(), Stream.of(switches)) .filter(i -> i instanceof String) .map(String.class::cast) .collect(toList())); } `[To ensure code accuracy, apply this suggestion manually]` Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Validate and sanitize external inputs (including capability values) before casting/processing to avoid runtime failures and invalid requests.	Low
More

VietND96 · 2025-12-22T18:18:03Z

Also, I saw Puja has made a few PRs for Grid to do the same. Should we resume her PR work?

iampopovich · 2025-12-22T19:38:32Z

@VietND96
#16226 - draft
#16225 - draft + conflict
#16224 - draft
#16223 - draft + conflict
#16206 - the most ready to merge PR .

I think it will take more time to revive these requests than to review this one

java/src/org/openqa/selenium/devtools/CdpClientGenerator.java

java/test/org/openqa/selenium/remote/RemoteLogsTest.java

java/src/org/openqa/selenium/grid/commands/EventBusCommand.java

java/src/org/openqa/selenium/grid/config/ConcatenatingConfig.java

java/src/org/openqa/selenium/grid/config/Config.java

java/test/org/openqa/selenium/grid/distributor/selector/DefaultSlotSelectorTest.java

java/test/org/openqa/selenium/grid/distributor/selector/GreedySlotSelectorTest.java

java/test/org/openqa/selenium/grid/node/config/SessionCapabilitiesMutatorTest.java

java/test/org/openqa/selenium/grid/node/local/LocalNodeTest.java

java/test/org/openqa/selenium/grid/node/NodeTest.java

`List.of` is preferred because it 1. Optimized, and 2. Returns a truly immutable object. While `Arrays.asList` returns a list that allows modifications, e.g. using method `*.set(index, value)`.

Guava's method `ImmutableMap.of` was needed until in Java 8 edge, when Java didn't have its own constructor for Maps. Nowadays `Map.of` is the proper way to create maps.

Guava's method `ImmutableList.of` was needed until in Java 8 edge, when Java didn't have its own constructor for Lists. Nowadays `List.of` is the proper way to create immutable lists.

Guava's method `ImmutableSet.of` was needed until in Java 8 edge, when Java didn't have its own constructor for Sets. Nowadays `Set.of` is the proper way to create immutable Sets. Sometimes we still need to use `ImmutableSet` - where we need to keep the order of elements.

All the requested changes have been implemented.

asolntsev self-assigned this Dec 22, 2025

asolntsev marked this pull request as draft December 22, 2025 10:23

selenium-ci added B-grid Everything grid and server related C-java Java Bindings B-support Issue or PR related to support classes labels Dec 22, 2025

asolntsev added this to the 4.40.0 milestone Dec 22, 2025

asolntsev added the I-enhancement Something could be better label Dec 22, 2025

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

asolntsev changed the title ~~[java] [refactor] replace Arrays.asList by List.of~~ [java] [refactor] replace most of Guava collections by Java built-in collections Dec 22, 2025

asolntsev marked this pull request as ready for review December 22, 2025 17:49

asolntsev requested review from VietND96, iampopovich, joerg1985 and navin772 December 22, 2025 17:49

qodo-code-review bot added the Review effort 4/5 label Dec 22, 2025

asolntsev force-pushed the refactor/old-java-collections branch from 35c922d to 4b625a3 Compare December 23, 2025 19:43

diemol previously requested changes Dec 24, 2025

View reviewed changes

asolntsev added 4 commits December 24, 2025 12:41

[refactor] replace Arrays.asList by List.of

77265e4

`List.of` is preferred because it 1. Optimized, and 2. Returns a truly immutable object. While `Arrays.asList` returns a list that allows modifications, e.g. using method `*.set(index, value)`.

[refactor] replace ImmutableMap.of by Map.of

3881023

Guava's method `ImmutableMap.of` was needed until in Java 8 edge, when Java didn't have its own constructor for Maps. Nowadays `Map.of` is the proper way to create maps.

[refactor] replace ImmutableList.of by List.of

209972c

Guava's method `ImmutableList.of` was needed until in Java 8 edge, when Java didn't have its own constructor for Lists. Nowadays `List.of` is the proper way to create immutable lists.

asolntsev force-pushed the refactor/old-java-collections branch from 4b625a3 to b32ed2d Compare December 24, 2025 10:41

use single imports instead of import x.y.*

496a2a6

asolntsev force-pushed the refactor/old-java-collections branch from b32ed2d to 496a2a6 Compare December 24, 2025 10:45

asolntsev requested a review from diemol December 24, 2025 10:45

asolntsev enabled auto-merge (squash) December 24, 2025 20:24

Merge branch 'trunk' into refactor/old-java-collections

2cb00f7

asolntsev merged commit e3d673a into SeleniumHQ:trunk Dec 25, 2025
40 checks passed

asolntsev deleted the refactor/old-java-collections branch December 25, 2025 13:40

Uh oh!

[java] [refactor] replace most of Guava collections by Java built-in collections #16776

[java] [refactor] replace most of Guava collections by Java built-in collections #16776

Uh oh!

Conversation

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

Uh oh!

User description

🔄 Types of changes

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

selenium-ci commented Dec 22, 2025

Uh oh!

qodo-code-review bot commented Dec 22, 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 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Uh oh!

qodo-code-review bot commented Dec 22, 2025

PR Compliance Guide 🔍

Uh oh!

qodo-code-review bot commented Dec 22, 2025

PR Code Suggestions ✨

Uh oh!

VietND96 commented Dec 22, 2025

Uh oh!

iampopovich commented Dec 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

asolntsev commented Dec 22, 2025 •

edited by qodo-code-review bot

Loading

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

edited

Loading

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

edited

Loading