Skip to content

Commit 9c4cbbc

Browse files
sbabcocsbabcoc
committed
Only ignore "special" extension capabilities
1 parent f02c68a commit 9c4cbbc

File tree

4 files changed

+127
-67
lines changed

4 files changed

+127
-67
lines changed

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

Lines changed: 52 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,27 @@
3939
* Then the {@code stereotype} must contain the same values.
4040
* </ul>
4141
*
42-
* <p>One thing to note is that extension capabilities are not considered when matching slots, since
43-
* the matching of these is implementation-specific to each driver.
42+
* <p>Note that extension capabilities are considered for slot matching, with the following exceptions:
43+
*
44+
* <ul>
45+
* <li>Extension capabilities with prefix "se:"
46+
* <li>Extension capabilities with these suffixes:
47+
* <ul>
48+
* <li>"options"
49+
* <li>"Options"
50+
* <li>"loggingPrefs"
51+
* <li>"debuggerAddress"
52+
* </ul>
53+
* </ul>
4454
*/
4555
public class DefaultSlotMatcher implements SlotMatcher, Serializable {
4656

4757
/*
48-
List of prefixed extension capabilities we never should try to match, they should be
58+
List of extension capability suffixes we never should try to match, they should be
4959
matched in the Node or in the browser driver.
5060
*/
51-
private static final List<String> EXTENSION_CAPABILITIES_PREFIXES =
52-
Arrays.asList("goog:", "moz:", "ms:", "se:");
61+
private static final List<String> EXTENSION_CAPABILITY_SUFFIXES =
62+
Arrays.asList("Options", "options", "loggingPrefs", "debuggerAddress");
5363

5464
@Override
5565
public boolean matches(Capabilities stereotype, Capabilities capabilities) {
@@ -76,14 +86,14 @@ public boolean matches(Capabilities stereotype, Capabilities capabilities) {
7686

7787
// At the end, a simple browser, browserVersion and platformName match
7888
boolean browserNameMatch =
79-
(capabilities.getBrowserName() == null || capabilities.getBrowserName().isEmpty())
89+
capabilities.getBrowserName() == null
90+
|| capabilities.getBrowserName().isEmpty()
8091
|| Objects.equals(stereotype.getBrowserName(), capabilities.getBrowserName());
8192
boolean browserVersionMatch =
82-
(capabilities.getBrowserVersion() == null
83-
|| capabilities.getBrowserVersion().isEmpty()
84-
|| Objects.equals(capabilities.getBrowserVersion(), "stable"))
85-
|| browserVersionMatch(
86-
stereotype.getBrowserVersion(), capabilities.getBrowserVersion());
93+
capabilities.getBrowserVersion() == null
94+
|| capabilities.getBrowserVersion().isEmpty()
95+
|| Objects.equals(capabilities.getBrowserVersion(), "stable")
96+
|| browserVersionMatch(stereotype.getBrowserVersion(), capabilities.getBrowserVersion());
8797
boolean platformNameMatch =
8898
capabilities.getPlatformName() == null
8999
|| Objects.equals(stereotype.getPlatformName(), capabilities.getPlatformName())
@@ -102,21 +112,17 @@ private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities)
102112
.filter(name -> !name.contains(":"))
103113
// Platform matching is special, we do it later
104114
.filter(name -> !"platformName".equalsIgnoreCase(name))
105-
.map(
115+
.filter(name -> capabilities.getCapability(name) != null)
116+
.allMatch(
106117
name -> {
107-
if (capabilities.getCapability(name) instanceof String) {
108-
return stereotype
109-
.getCapability(name)
110-
.toString()
111-
.equalsIgnoreCase(capabilities.getCapability(name).toString());
112-
} else {
113-
return capabilities.getCapability(name) == null
114-
|| Objects.equals(
115-
stereotype.getCapability(name), capabilities.getCapability(name));
118+
if (stereotype.getCapability(name) instanceof String
119+
&& capabilities.getCapability(name) instanceof String) {
120+
return ((String) stereotype.getCapability(name))
121+
.equalsIgnoreCase((String) capabilities.getCapability(name));
116122
}
117-
})
118-
.reduce(Boolean::logicalAnd)
119-
.orElse(true);
123+
return Objects.equals(
124+
stereotype.getCapability(name), capabilities.getCapability(name));
125+
});
120126
}
121127

122128
private Boolean managedDownloadsEnabled(Capabilities stereotype, Capabilities capabilities) {
@@ -140,39 +146,39 @@ private Boolean platformVersionMatch(Capabilities stereotype, Capabilities capab
140146
*/
141147
return capabilities.getCapabilityNames().stream()
142148
.filter(name -> name.contains("platformVersion"))
143-
.map(
149+
.allMatch(
144150
platformVersionCapName ->
145151
Objects.equals(
146152
stereotype.getCapability(platformVersionCapName),
147-
capabilities.getCapability(platformVersionCapName)))
148-
.reduce(Boolean::logicalAnd)
149-
.orElse(true);
153+
capabilities.getCapability(platformVersionCapName)));
150154
}
151155

152156
private Boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities capabilities) {
153157
/*
154-
We match extension capabilities when they are not prefixed with any of the
155-
EXTENSION_CAPABILITIES_PREFIXES items. Also, we match them only when the capabilities
156-
of the new session request contains that specific extension capability.
158+
We match extension capabilities in new session requests whose names do not have the prefix "se:" or
159+
one of the reserved suffixes ("options", "Options", "loggingPrefs", or "debuggerAddress"). These are
160+
forwarded to the matched node for use in configuration, but are not considered for node matching.
157161
*/
158162
return stereotype.getCapabilityNames().stream()
163+
// examine only extension capabilities
159164
.filter(name -> name.contains(":"))
160-
.filter(name -> capabilities.asMap().containsKey(name))
161-
.filter(name -> EXTENSION_CAPABILITIES_PREFIXES.stream().noneMatch(name::contains))
162-
.map(
165+
// ignore Selenium extension capabilities
166+
.filter(name -> !name.startsWith("se:"))
167+
// ignore special extension capability suffixes
168+
.filter(name -> EXTENSION_CAPABILITY_SUFFIXES.stream().noneMatch(name::endsWith))
169+
// ignore capabilities not specified in the request
170+
.filter(name -> capabilities.getCapability(name) != null)
171+
.allMatch(
163172
name -> {
164-
if (capabilities.getCapability(name) instanceof String) {
165-
return stereotype
166-
.getCapability(name)
167-
.toString()
168-
.equalsIgnoreCase(capabilities.getCapability(name).toString());
169-
} else {
170-
return capabilities.getCapability(name) == null
171-
|| Objects.equals(
172-
stereotype.getCapability(name), capabilities.getCapability(name));
173+
// evaluate capabilities with String values
174+
if (stereotype.getCapability(name) instanceof String
175+
&& capabilities.getCapability(name) instanceof String) {
176+
return ((String) stereotype.getCapability(name))
177+
.equalsIgnoreCase((String) capabilities.getCapability(name));
173178
}
174-
})
175-
.reduce(Boolean::logicalAnd)
176-
.orElse(true);
179+
// evaluate capabilities with Number or Boolean values
180+
return Objects.equals(
181+
stereotype.getCapability(name), capabilities.getCapability(name));
182+
});
177183
}
178184
}

‎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: 70 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

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

22+
import java.util.Map;
2223
import org.junit.jupiter.api.Test;
2324
import org.openqa.selenium.Capabilities;
2425
import org.openqa.selenium.ImmutableCapabilities;
@@ -540,7 +541,37 @@ void multipleExtensionPrefixedCapabilitiesDoNotMatchWhenOneIsDifferent() {
540541
}
541542

542543
@Test
543-
void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
544+
void seleniumExtensionCapabilitiesAreIgnoredForMatching() {
545+
Capabilities stereotype =
546+
new ImmutableCapabilities(
547+
CapabilityType.BROWSER_NAME,
548+
"chrome",
549+
CapabilityType.BROWSER_VERSION,
550+
"84",
551+
CapabilityType.PLATFORM_NAME,
552+
Platform.WINDOWS,
553+
"se:cdpVersion",
554+
1,
555+
"se:downloadsEnabled",
556+
true);
557+
558+
Capabilities capabilities =
559+
new ImmutableCapabilities(
560+
CapabilityType.BROWSER_NAME,
561+
"chrome",
562+
CapabilityType.BROWSER_VERSION,
563+
"84",
564+
CapabilityType.PLATFORM_NAME,
565+
Platform.WINDOWS,
566+
"se:cdpVersion",
567+
2,
568+
"se:downloadsEnabled",
569+
false);
570+
assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue();
571+
}
572+
573+
@Test
574+
void vendorOptionsCapabilitiesAreIgnoredForMatching() {
544575
Capabilities stereotype =
545576
new ImmutableCapabilities(
546577
CapabilityType.BROWSER_NAME,
@@ -549,10 +580,10 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
549580
"84",
550581
CapabilityType.PLATFORM_NAME,
551582
Platform.WINDOWS,
552-
"goog:cheese",
553-
"amsterdam",
554-
"ms:fruit",
555-
"mango");
583+
"food:fruitOptions",
584+
"mango",
585+
"dairy:options",
586+
Map.of("cheese", "amsterdam"));
556587

557588
Capabilities capabilities =
558589
new ImmutableCapabilities(
@@ -562,10 +593,40 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
562593
"84",
563594
CapabilityType.PLATFORM_NAME,
564595
Platform.WINDOWS,
565-
"goog:cheese",
566-
"gouda",
567-
"ms:fruit",
568-
"orange");
596+
"food:fruitOptions",
597+
"orange",
598+
"dairy:options",
599+
Map.of("cheese", "gouda"));
600+
assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue();
601+
}
602+
603+
@Test
604+
void specialExtensionCapabilitiesAreIgnoredForMatching() {
605+
Capabilities stereotype =
606+
new ImmutableCapabilities(
607+
CapabilityType.BROWSER_NAME,
608+
"chrome",
609+
CapabilityType.BROWSER_VERSION,
610+
"84",
611+
CapabilityType.PLATFORM_NAME,
612+
Platform.WINDOWS,
613+
"food:loggingPrefs",
614+
"mango",
615+
"food:debuggerAddress",
616+
Map.of("cheese", "amsterdam"));
617+
618+
Capabilities capabilities =
619+
new ImmutableCapabilities(
620+
CapabilityType.BROWSER_NAME,
621+
"chrome",
622+
CapabilityType.BROWSER_VERSION,
623+
"84",
624+
CapabilityType.PLATFORM_NAME,
625+
Platform.WINDOWS,
626+
"food:loggingPrefs",
627+
"orange",
628+
"food:debuggerAddress",
629+
Map.of("cheese", "gouda"));
569630
assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue();
570631
}
571632

‎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)