Skip to content

Commit 5e7e1ab

Browse files
sbabcocsbabcoc
committed
Only ignore extension caps with object/array values
1 parent b01041f commit 5e7e1ab

File tree

4 files changed

+61
-51
lines changed

4 files changed

+61
-51
lines changed

‎java/src/org/openqa/selenium/grid/data/DefaultSlotMatcher.java‎

Lines changed: 23 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
package org.openqa.selenium.grid.data;
1919

2020
import java.io.Serializable;
21-
import java.util.Arrays;
22-
import java.util.List;
2321
import java.util.Objects;
2422
import org.openqa.selenium.Capabilities;
2523

@@ -44,13 +42,6 @@
4442
*/
4543
public class DefaultSlotMatcher implements SlotMatcher, Serializable {
4644

47-
/*
48-
List of prefixed extension capabilities we never should try to match, they should be
49-
matched in the Node or in the browser driver.
50-
*/
51-
private static final List<String> EXTENSION_CAPABILITIES_PREFIXES =
52-
Arrays.asList("goog:", "moz:", "ms:", "se:");
53-
5445
@Override
5546
public boolean matches(Capabilities stereotype, Capabilities capabilities) {
5647

@@ -76,12 +67,13 @@ public boolean matches(Capabilities stereotype, Capabilities capabilities) {
7667

7768
// At the end, a simple browser, browserVersion and platformName match
7869
boolean browserNameMatch =
79-
(capabilities.getBrowserName() == null || capabilities.getBrowserName().isEmpty())
70+
capabilities.getBrowserName() == null
71+
|| capabilities.getBrowserName().isEmpty()
8072
|| Objects.equals(stereotype.getBrowserName(), capabilities.getBrowserName());
8173
boolean browserVersionMatch =
82-
(capabilities.getBrowserVersion() == null
83-
|| capabilities.getBrowserVersion().isEmpty()
84-
|| Objects.equals(capabilities.getBrowserVersion(), "stable"))
74+
capabilities.getBrowserVersion() == null
75+
|| capabilities.getBrowserVersion().isEmpty()
76+
|| Objects.equals(capabilities.getBrowserVersion(), "stable")
8577
|| Objects.equals(stereotype.getBrowserVersion(), capabilities.getBrowserVersion());
8678
boolean platformNameMatch =
8779
capabilities.getPlatformName() == null
@@ -97,18 +89,16 @@ private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities)
9789
.filter(name -> !name.contains(":"))
9890
// Platform matching is special, we do it later
9991
.filter(name -> !"platformName".equalsIgnoreCase(name))
92+
.filter(name -> capabilities.getCapability(name) != null)
10093
.map(
10194
name -> {
102-
if (capabilities.getCapability(name) instanceof String) {
103-
return stereotype
104-
.getCapability(name)
105-
.toString()
106-
.equalsIgnoreCase(capabilities.getCapability(name).toString());
107-
} else {
108-
return capabilities.getCapability(name) == null
109-
|| Objects.equals(
110-
stereotype.getCapability(name), capabilities.getCapability(name));
95+
if (stereotype.getCapability(name) instanceof String
96+
&& capabilities.getCapability(name) instanceof String) {
97+
return ((String) stereotype.getCapability(name))
98+
.equalsIgnoreCase((String) capabilities.getCapability(name));
11199
}
100+
return Objects.equals(
101+
stereotype.getCapability(name), capabilities.getCapability(name));
112102
})
113103
.reduce(Boolean::logicalAnd)
114104
.orElse(true);
@@ -145,27 +135,22 @@ private Boolean platformVersionMatch(Capabilities stereotype, Capabilities capab
145135
}
146136

147137
private Boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities capabilities) {
148-
/*
149-
We match extension capabilities when they are not prefixed with any of the
150-
EXTENSION_CAPABILITIES_PREFIXES items. Also, we match them only when the capabilities
151-
of the new session request contains that specific extension capability.
152-
*/
153138
return stereotype.getCapabilityNames().stream()
154139
.filter(name -> name.contains(":"))
155-
.filter(name -> capabilities.asMap().containsKey(name))
156-
.filter(name -> EXTENSION_CAPABILITIES_PREFIXES.stream().noneMatch(name::contains))
140+
.filter(name -> capabilities.getCapability(name) != null)
157141
.map(
158142
name -> {
159-
if (capabilities.getCapability(name) instanceof String) {
160-
return stereotype
161-
.getCapability(name)
162-
.toString()
163-
.equalsIgnoreCase(capabilities.getCapability(name).toString());
164-
} else {
165-
return capabilities.getCapability(name) == null
166-
|| Objects.equals(
167-
stereotype.getCapability(name), capabilities.getCapability(name));
143+
if (stereotype.getCapability(name) instanceof String
144+
&& capabilities.getCapability(name) instanceof String) {
145+
return ((String) stereotype.getCapability(name))
146+
.equalsIgnoreCase((String) capabilities.getCapability(name));
147+
}
148+
if (capabilities.getCapability(name) instanceof Number
149+
|| capabilities.getCapability(name) instanceof Boolean) {
150+
return Objects.equals(
151+
stereotype.getCapability(name), capabilities.getCapability(name));
168152
}
153+
return true;
169154
})
170155
.reduce(Boolean::logicalAnd)
171156
.orElse(true);

‎java/src/org/openqa/selenium/grid/node/relay/RelaySessionFactory.java‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import java.util.logging.Logger;
4141
import org.openqa.selenium.Capabilities;
4242
import org.openqa.selenium.ImmutableCapabilities;
43-
import org.openqa.selenium.MutableCapabilities;
4443
import org.openqa.selenium.SessionNotCreatedException;
4544
import org.openqa.selenium.WebDriverException;
4645
import org.openqa.selenium.grid.data.CreateSessionRequest;
@@ -50,7 +49,6 @@
5049
import org.openqa.selenium.internal.Debug;
5150
import org.openqa.selenium.internal.Either;
5251
import org.openqa.selenium.internal.Require;
53-
import org.openqa.selenium.remote.CapabilityType;
5452
import org.openqa.selenium.remote.Command;
5553
import org.openqa.selenium.remote.Dialect;
5654
import org.openqa.selenium.remote.DriverCommand;
@@ -149,15 +147,6 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
149147
"New session request capabilities do not " + "match the stereotype."));
150148
}
151149

152-
// remove browserName capability if 'appium:app' is present as it breaks appium tests when app
153-
// is provided
154-
// they are mutually exclusive
155-
MutableCapabilities filteredStereotype = new MutableCapabilities(stereotype);
156-
if (capabilities.getCapability("appium:app") != null) {
157-
filteredStereotype.setCapability(CapabilityType.BROWSER_NAME, (String) null);
158-
}
159-
160-
capabilities = capabilities.merge(filteredStereotype);
161150
LOG.info("Starting session for " + capabilities);
162151

163152
try (Span span = tracer.getCurrentContext().createSpan("relay_session_factory.apply")) {

‎java/test/org/openqa/selenium/grid/data/DefaultSlotMatcherTest.java‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

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

22+
import java.util.List;
23+
import java.util.Map;
2224
import org.junit.jupiter.api.Test;
2325
import org.openqa.selenium.Capabilities;
2426
import org.openqa.selenium.ImmutableCapabilities;
@@ -424,7 +426,7 @@ void multipleExtensionPrefixedCapabilitiesDoNotMatchWhenOneIsDifferent() {
424426
}
425427

426428
@Test
427-
void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
429+
void vendorExtensionPrefixedCapabilitiesWithSimpleValuesAreConsideredForMatching() {
428430
Capabilities stereotype =
429431
new ImmutableCapabilities(
430432
CapabilityType.BROWSER_NAME,
@@ -450,6 +452,36 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
450452
"gouda",
451453
"ms:fruit",
452454
"orange");
455+
assertThat(slotMatcher.matches(stereotype, capabilities)).isFalse();
456+
}
457+
458+
@Test
459+
void vendorExtensionPrefixedCapabilitiesWithComplexValuesAreIgnoredForMatching() {
460+
Capabilities stereotype =
461+
new ImmutableCapabilities(
462+
CapabilityType.BROWSER_NAME,
463+
"chrome",
464+
CapabilityType.BROWSER_VERSION,
465+
"84",
466+
CapabilityType.PLATFORM_NAME,
467+
Platform.WINDOWS,
468+
"food:dairy",
469+
Map.of("cheese", "amsterdam"),
470+
"food:fruit",
471+
List.of("mango"));
472+
473+
Capabilities capabilities =
474+
new ImmutableCapabilities(
475+
CapabilityType.BROWSER_NAME,
476+
"chrome",
477+
CapabilityType.BROWSER_VERSION,
478+
"84",
479+
CapabilityType.PLATFORM_NAME,
480+
Platform.WINDOWS,
481+
"food:dairy",
482+
Map.of("cheese", "gouda"),
483+
"food:fruit",
484+
List.of("orange"));
453485
assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue();
454486
}
455487

‎java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import org.openqa.selenium.internal.Either;
6464
import org.openqa.selenium.json.Json;
6565
import org.openqa.selenium.net.NetworkUtils;
66+
import org.openqa.selenium.remote.Browser;
6667
import org.openqa.selenium.safari.SafariDriverInfo;
6768

6869
@SuppressWarnings("DuplicatedCode")
@@ -148,7 +149,10 @@ boolean isDownloadEnabled(WebDriverInfo driver, String customMsg) {
148149
reported.add(caps);
149150
return Collections.singleton(HelperFactory.create(config, caps));
150151
});
151-
String expected = driver.getDisplayName();
152+
String expected =
153+
"Edge".equals(driver.getDisplayName())
154+
? Browser.EDGE.browserName()
155+
: driver.getDisplayName();
152156

153157
Capabilities found =
154158
reported.stream()

0 commit comments

Comments
 (0)