Skip to content

Commit

Permalink
Only ignore extension caps with object/array values
Browse files Browse the repository at this point in the history
  • Loading branch information
sbabcoc committed Sep 18, 2024
1 parent 6dd13d7 commit f0f42b7
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 47 deletions.
52 changes: 18 additions & 34 deletions java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
package org.openqa.selenium.grid.data;

import java.io.Serializable;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import org.openqa.selenium.Capabilities;

Expand All @@ -44,13 +42,6 @@
*/
public class DefaultSlotMatcher implements SlotMatcher, Serializable {

/*
List of prefixed extension capabilities we never should try to match, they should be
matched in the Node or in the browser driver.
*/
private static final List<String> EXTENSION_CAPABILITIES_PREFIXES =
Arrays.asList("goog:", "moz:", "ms:", "se:");

@Override
public boolean matches(Capabilities stereotype, Capabilities capabilities) {

Expand Down Expand Up @@ -97,18 +88,16 @@ private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities)
.filter(name -> !name.contains(":"))
// Platform matching is special, we do it later
.filter(name -> !"platformName".equalsIgnoreCase(name))
.filter(name -> capabilities.getCapability(name) != null)
.map(
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));
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));
})
.reduce(Boolean::logicalAnd)
.orElse(true);
Expand Down Expand Up @@ -145,27 +134,22 @@ private Boolean platformVersionMatch(Capabilities stereotype, Capabilities capab
}

private Boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities capabilities) {
/*
We match extension capabilities when they are not prefixed with any of the
EXTENSION_CAPABILITIES_PREFIXES items. Also, we match them only when the capabilities
of the new session request contains that specific extension capability.
*/
return stereotype.getCapabilityNames().stream()
.filter(name -> name.contains(":"))
.filter(name -> capabilities.asMap().containsKey(name))
.filter(name -> EXTENSION_CAPABILITIES_PREFIXES.stream().noneMatch(name::contains))
.filter(name -> capabilities.getCapability(name) != null)
.map(
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));
if (stereotype.getCapability(name) instanceof String
&& capabilities.getCapability(name) instanceof String) {
return ((String) stereotype.getCapability(name))
.equalsIgnoreCase((String) capabilities.getCapability(name));
}
if (capabilities.getCapability(name) instanceof Number
|| capabilities.getCapability(name) instanceof Boolean) {
return Objects.equals(
stereotype.getCapability(name), capabilities.getCapability(name));
}
return true;
})
.reduce(Boolean::logicalAnd)
.orElse(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import java.util.logging.Logger;
import org.openqa.selenium.Capabilities;
import org.openqa.selenium.ImmutableCapabilities;
import org.openqa.selenium.MutableCapabilities;
import org.openqa.selenium.SessionNotCreatedException;
import org.openqa.selenium.WebDriverException;
import org.openqa.selenium.grid.data.CreateSessionRequest;
Expand All @@ -50,7 +49,6 @@
import org.openqa.selenium.internal.Debug;
import org.openqa.selenium.internal.Either;
import org.openqa.selenium.internal.Require;
import org.openqa.selenium.remote.CapabilityType;
import org.openqa.selenium.remote.Command;
import org.openqa.selenium.remote.Dialect;
import org.openqa.selenium.remote.DriverCommand;
Expand Down Expand Up @@ -149,15 +147,6 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
"New session request capabilities do not " + "match the stereotype."));
}

// remove browserName capability if 'appium:app' is present as it breaks appium tests when app
// is provided
// they are mutually exclusive
MutableCapabilities filteredStereotype = new MutableCapabilities(stereotype);
if (capabilities.getCapability("appium:app") != null) {
filteredStereotype.setCapability(CapabilityType.BROWSER_NAME, (String) null);
}

capabilities = capabilities.merge(filteredStereotype);
LOG.info("Starting session for " + capabilities);

try (Span span = tracer.getCurrentContext().createSpan("relay_session_factory.apply")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

import static org.assertj.core.api.Assertions.assertThat;

import java.util.List;
import java.util.Map;
import org.junit.jupiter.api.Test;
import org.openqa.selenium.Capabilities;
import org.openqa.selenium.ImmutableCapabilities;
Expand Down Expand Up @@ -424,7 +426,7 @@ void multipleExtensionPrefixedCapabilitiesDoNotMatchWhenOneIsDifferent() {
}

@Test
void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
void vendorExtensionPrefixedCapabilitiesWithSimpleValuesAreConsideredForMatching() {
Capabilities stereotype =
new ImmutableCapabilities(
CapabilityType.BROWSER_NAME,
Expand All @@ -450,6 +452,36 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
"gouda",
"ms:fruit",
"orange");
assertThat(slotMatcher.matches(stereotype, capabilities)).isFalse();
}

@Test
void vendorExtensionPrefixedCapabilitiesWithComplexValuesAreIgnoredForMatching() {
Capabilities stereotype =
new ImmutableCapabilities(
CapabilityType.BROWSER_NAME,
"chrome",
CapabilityType.BROWSER_VERSION,
"84",
CapabilityType.PLATFORM_NAME,
Platform.WINDOWS,
"food:dairy",
Map.of("cheese", "amsterdam"),
"food:fruit",
List.of("mango"));

Capabilities capabilities =
new ImmutableCapabilities(
CapabilityType.BROWSER_NAME,
"chrome",
CapabilityType.BROWSER_VERSION,
"84",
CapabilityType.PLATFORM_NAME,
Platform.WINDOWS,
"food:dairy",
Map.of("cheese", "gouda"),
"food:fruit",
List.of("orange"));
assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit f0f42b7

Please sign in to comment.