diff --git a/java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java b/java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java index 3db617bbd5335..54cec38d8b315 100644 --- a/java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java +++ b/java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java @@ -39,8 +39,24 @@ * Then the {@code stereotype} must contain the same values. * * - *

One thing to note is that extension capabilities are not considered when matching slots, since - * the matching of these is implementation-specific to each driver. + *

Note that extension capabilities are considered for slot matching, with the following exceptions: + * + *

*/ public class DefaultSlotMatcher implements SlotMatcher, Serializable { @@ -51,6 +67,13 @@ public class DefaultSlotMatcher implements SlotMatcher, Serializable { private static final List EXTENSION_CAPABILITIES_PREFIXES = Arrays.asList("goog:", "moz:", "ms:", "se:"); + /* + List of extension capability suffixes we never should try to match, they should be + matched in the Node or in the browser driver. + */ + private static final List EXTENSION_CAPABILITY_SUFFIXES = + Arrays.asList("Options", "options", "loggingPrefs", "debuggerAddress"); + @Override public boolean matches(Capabilities stereotype, Capabilities capabilities) { @@ -76,14 +99,14 @@ public boolean matches(Capabilities stereotype, Capabilities capabilities) { // At the end, a simple browser, browserVersion and platformName match boolean browserNameMatch = - (capabilities.getBrowserName() == null || capabilities.getBrowserName().isEmpty()) + capabilities.getBrowserName() == null + || capabilities.getBrowserName().isEmpty() || Objects.equals(stereotype.getBrowserName(), capabilities.getBrowserName()); boolean browserVersionMatch = - (capabilities.getBrowserVersion() == null - || capabilities.getBrowserVersion().isEmpty() - || Objects.equals(capabilities.getBrowserVersion(), "stable")) - || browserVersionMatch( - stereotype.getBrowserVersion(), capabilities.getBrowserVersion()); + capabilities.getBrowserVersion() == null + || capabilities.getBrowserVersion().isEmpty() + || Objects.equals(capabilities.getBrowserVersion(), "stable") + || browserVersionMatch(stereotype.getBrowserVersion(), capabilities.getBrowserVersion()); boolean platformNameMatch = capabilities.getPlatformName() == null || Objects.equals(stereotype.getPlatformName(), capabilities.getPlatformName()) @@ -102,21 +125,17 @@ private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities) .filter(name -> !name.contains(":")) // Platform matching is special, we do it later .filter(name -> !"platformName".equalsIgnoreCase(name)) - .map( + .filter(name -> capabilities.getCapability(name) != null) + .allMatch( name -> { - if (capabilities.getCapability(name) instanceof String) { - return stereotype - .getCapability(name) - .toString() - .equalsIgnoreCase(capabilities.getCapability(name).toString()); - } else { - return capabilities.getCapability(name) == null - || Objects.equals( - stereotype.getCapability(name), capabilities.getCapability(name)); - } - }) - .reduce(Boolean::logicalAnd) - .orElse(true); + if (stereotype.getCapability(name) instanceof String + && capabilities.getCapability(name) instanceof String) { + return ((String) stereotype.getCapability(name)) + .equalsIgnoreCase((String) capabilities.getCapability(name)); + } + return Objects.equals( + stereotype.getCapability(name), capabilities.getCapability(name)); + }); } private Boolean managedDownloadsEnabled(Capabilities stereotype, Capabilities capabilities) { @@ -140,13 +159,11 @@ private Boolean platformVersionMatch(Capabilities stereotype, Capabilities capab */ return capabilities.getCapabilityNames().stream() .filter(name -> name.contains("platformVersion")) - .map( + .allMatch( platformVersionCapName -> Objects.equals( stereotype.getCapability(platformVersionCapName), - capabilities.getCapability(platformVersionCapName))) - .reduce(Boolean::logicalAnd) - .orElse(true); + capabilities.getCapability(platformVersionCapName))); } private Boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities capabilities) { @@ -156,23 +173,25 @@ private Boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities of the new session request contains that specific extension capability. */ return stereotype.getCapabilityNames().stream() + // examine only extension capabilities .filter(name -> name.contains(":")) - .filter(name -> capabilities.asMap().containsKey(name)) + // ignore special extension capability prefixes .filter(name -> EXTENSION_CAPABILITIES_PREFIXES.stream().noneMatch(name::contains)) - .map( + // ignore special extension capability suffixes + .filter(name -> EXTENSION_CAPABILITY_SUFFIXES.stream().noneMatch(name::endsWith)) + // ignore capabilities not specified in the request + .filter(name -> capabilities.getCapability(name) != null) + .allMatch( name -> { - if (capabilities.getCapability(name) instanceof String) { - return stereotype - .getCapability(name) - .toString() - .equalsIgnoreCase(capabilities.getCapability(name).toString()); - } else { - return capabilities.getCapability(name) == null - || Objects.equals( - stereotype.getCapability(name), capabilities.getCapability(name)); - } - }) - .reduce(Boolean::logicalAnd) - .orElse(true); + // evaluate capabilities with String values + if (stereotype.getCapability(name) instanceof String + && capabilities.getCapability(name) instanceof String) { + return ((String) stereotype.getCapability(name)) + .equalsIgnoreCase((String) capabilities.getCapability(name)); + } + // evaluate capabilities with non-String values + return Objects.equals( + stereotype.getCapability(name), capabilities.getCapability(name)); + }); } } diff --git a/java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java b/java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java index 2c531ef18f190..f4ad9114edcb6 100644 --- a/java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java +++ b/java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java @@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.util.Map; import org.junit.jupiter.api.Test; import org.openqa.selenium.Capabilities; import org.openqa.selenium.ImmutableCapabilities; @@ -540,7 +541,7 @@ void multipleExtensionPrefixedCapabilitiesDoNotMatchWhenOneIsDifferent() { } @Test - void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() { + void seleniumExtensionCapabilitiesAreIgnoredForMatching() { Capabilities stereotype = new ImmutableCapabilities( CapabilityType.BROWSER_NAME, @@ -549,12 +550,29 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() { "84", CapabilityType.PLATFORM_NAME, Platform.WINDOWS, - "goog:cheese", - "amsterdam", - "ms:fruit", - "mango"); + "se:cdpVersion", + 1, + "se:downloadsEnabled", + true); Capabilities capabilities = + new ImmutableCapabilities( + CapabilityType.BROWSER_NAME, + "chrome", + CapabilityType.BROWSER_VERSION, + "84", + CapabilityType.PLATFORM_NAME, + Platform.WINDOWS, + "se:cdpVersion", + 2, + "se:downloadsEnabled", + false); + assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue(); + } + + @Test + void vendorOptionsCapabilitiesAreIgnoredForMatching() { + Capabilities stereotype = new ImmutableCapabilities( CapabilityType.BROWSER_NAME, "chrome", @@ -562,10 +580,53 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() { "84", CapabilityType.PLATFORM_NAME, Platform.WINDOWS, - "goog:cheese", - "gouda", - "ms:fruit", - "orange"); + "food:fruitOptions", + "mango", + "dairy:options", + Map.of("cheese", "amsterdam")); + + Capabilities capabilities = + new ImmutableCapabilities( + CapabilityType.BROWSER_NAME, + "chrome", + CapabilityType.BROWSER_VERSION, + "84", + CapabilityType.PLATFORM_NAME, + Platform.WINDOWS, + "food:fruitOptions", + "orange", + "dairy:options", + Map.of("cheese", "gouda")); + assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue(); + } + + @Test + void specialExtensionCapabilitiesAreIgnoredForMatching() { + Capabilities stereotype = + new ImmutableCapabilities( + CapabilityType.BROWSER_NAME, + "chrome", + CapabilityType.BROWSER_VERSION, + "84", + CapabilityType.PLATFORM_NAME, + Platform.WINDOWS, + "food:loggingPrefs", + "mango", + "food:debuggerAddress", + Map.of("cheese", "amsterdam")); + + Capabilities capabilities = + new ImmutableCapabilities( + CapabilityType.BROWSER_NAME, + "chrome", + CapabilityType.BROWSER_VERSION, + "84", + CapabilityType.PLATFORM_NAME, + Platform.WINDOWS, + "food:loggingPrefs", + "orange", + "food:debuggerAddress", + Map.of("cheese", "gouda")); assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue(); } diff --git a/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java b/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java index 69f1ba30a9ee9..3def22ea8dde3 100644 --- a/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java +++ b/java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java @@ -63,6 +63,7 @@ import org.openqa.selenium.internal.Either; import org.openqa.selenium.json.Json; import org.openqa.selenium.net.NetworkUtils; +import org.openqa.selenium.remote.Browser; import org.openqa.selenium.safari.SafariDriverInfo; @SuppressWarnings("DuplicatedCode") @@ -148,7 +149,10 @@ boolean isDownloadEnabled(WebDriverInfo driver, String customMsg) { reported.add(caps); return Collections.singleton(HelperFactory.create(config, caps)); }); - String expected = driver.getDisplayName(); + String expected = + "Edge".equals(driver.getDisplayName()) + ? Browser.EDGE.browserName() + : driver.getDisplayName(); Capabilities found = reported.stream()