Skip to content

Commit c10d00f

Browse files
sbabcocsbabcoc
committed
Ignore caps with special suffixes
1 parent 2b950b1 commit c10d00f

File tree

3 files changed

+135
-50
lines changed

3 files changed

+135
-50
lines changed

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

Lines changed: 60 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,25 @@
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 these prefixes:
46+
* <ul>
47+
* <li>"goog:" - Google
48+
* <li>"moz:" - Mozilla
49+
* <li>"ms:" - Microsoft
50+
* <li>"safari:" - Apple
51+
* <li>"se:" - Selenium
52+
* </ul>
53+
* <li>Extension capabilities with these suffixes:
54+
* <ul>
55+
* <li>"Options"
56+
* <li>"options"
57+
* <li>"loggingPrefs"
58+
* <li>"debuggerAddress"
59+
* </ul>
60+
* </ul>
4461
*/
4562
public class DefaultSlotMatcher implements SlotMatcher, Serializable {
4663

@@ -55,6 +72,13 @@ public class DefaultSlotMatcher implements SlotMatcher, Serializable {
5572
public static final List<String> MANDATORY_CAPABILITIES =
5673
Arrays.asList("platformName", "browserName", "browserVersion");
5774

75+
/*
76+
List of extension capability suffixes we never should try to match, they should be
77+
matched in the Node or in the browser driver.
78+
*/
79+
private static final List<String> EXTENSION_CAPABILITY_SUFFIXES =
80+
Arrays.asList("Options", "options", "loggingPrefs", "debuggerAddress");
81+
5882
@Override
5983
public boolean matches(Capabilities stereotype, Capabilities capabilities) {
6084

@@ -80,13 +104,14 @@ public boolean matches(Capabilities stereotype, Capabilities capabilities) {
80104

81105
// At the end, a simple browser, browserVersion and platformName match
82106
boolean browserNameMatch =
83-
(capabilities.getBrowserName() == null || capabilities.getBrowserName().isEmpty())
107+
capabilities.getBrowserName() == null
108+
|| capabilities.getBrowserName().isEmpty()
84109
|| Objects.equals(stereotype.getBrowserName(), capabilities.getBrowserName())
85110
|| matchConditionToRemoveCapability(capabilities);
86111
boolean browserVersionMatch =
87-
(capabilities.getBrowserVersion() == null
88-
|| capabilities.getBrowserVersion().isEmpty()
89-
|| Objects.equals(capabilities.getBrowserVersion(), "stable"))
112+
capabilities.getBrowserVersion() == null
113+
|| capabilities.getBrowserVersion().isEmpty()
114+
|| Objects.equals(capabilities.getBrowserVersion(), "stable")
90115
|| browserVersionMatch(stereotype.getBrowserVersion(), capabilities.getBrowserVersion())
91116
|| matchConditionToRemoveCapability(capabilities);
92117
boolean platformNameMatch =
@@ -110,21 +135,17 @@ private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities)
110135
name ->
111136
MANDATORY_CAPABILITIES.stream()
112137
.noneMatch(mandatory -> mandatory.equalsIgnoreCase(name)))
113-
.map(
138+
.filter(name -> capabilities.getCapability(name) != null)
139+
.allMatch(
114140
name -> {
115-
if (capabilities.getCapability(name) instanceof String) {
116-
return stereotype
117-
.getCapability(name)
118-
.toString()
119-
.equalsIgnoreCase(capabilities.getCapability(name).toString());
120-
} else {
121-
return capabilities.getCapability(name) == null
122-
|| Objects.equals(
123-
stereotype.getCapability(name), capabilities.getCapability(name));
124-
}
125-
})
126-
.reduce(Boolean::logicalAnd)
127-
.orElse(true);
141+
if (stereotype.getCapability(name) instanceof String
142+
&& capabilities.getCapability(name) instanceof String) {
143+
return ((String) stereotype.getCapability(name))
144+
.equalsIgnoreCase((String) capabilities.getCapability(name));
145+
}
146+
return Objects.equals(
147+
stereotype.getCapability(name), capabilities.getCapability(name));
148+
});
128149
}
129150

130151
private Boolean managedDownloadsEnabled(Capabilities stereotype, Capabilities capabilities) {
@@ -148,13 +169,11 @@ private Boolean platformVersionMatch(Capabilities stereotype, Capabilities capab
148169
*/
149170
return capabilities.getCapabilityNames().stream()
150171
.filter(name -> name.contains("platformVersion"))
151-
.map(
172+
.allMatch(
152173
platformVersionCapName ->
153174
Objects.equals(
154175
stereotype.getCapability(platformVersionCapName),
155-
capabilities.getCapability(platformVersionCapName)))
156-
.reduce(Boolean::logicalAnd)
157-
.orElse(true);
176+
capabilities.getCapability(platformVersionCapName)));
158177
}
159178

160179
private Boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities capabilities) {
@@ -166,25 +185,26 @@ private Boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities
166185
by the remote endpoint.
167186
*/
168187
return stereotype.getCapabilityNames().stream()
188+
// examine only extension capabilities
169189
.filter(name -> name.contains(":"))
170-
.filter(name -> !name.toLowerCase().contains("options"))
171-
.filter(name -> capabilities.asMap().containsKey(name))
190+
// ignore special extension capability prefixes
172191
.filter(name -> EXTENSION_CAPABILITIES_PREFIXES.stream().noneMatch(name::contains))
173-
.map(
192+
// ignore special extension capability suffixes
193+
.filter(name -> EXTENSION_CAPABILITY_SUFFIXES.stream().noneMatch(name::endsWith))
194+
// ignore capabilities not specified in the request
195+
.filter(name -> capabilities.getCapability(name) != null)
196+
.allMatch(
174197
name -> {
175-
if (capabilities.getCapability(name) instanceof String) {
176-
return stereotype
177-
.getCapability(name)
178-
.toString()
179-
.equalsIgnoreCase(capabilities.getCapability(name).toString());
180-
} else {
181-
return capabilities.getCapability(name) == null
182-
|| Objects.equals(
183-
stereotype.getCapability(name), capabilities.getCapability(name));
184-
}
185-
})
186-
.reduce(Boolean::logicalAnd)
187-
.orElse(true);
198+
// evaluate capabilities with String values
199+
if (stereotype.getCapability(name) instanceof String
200+
&& capabilities.getCapability(name) instanceof String) {
201+
return ((String) stereotype.getCapability(name))
202+
.equalsIgnoreCase((String) capabilities.getCapability(name));
203+
}
204+
// evaluate capabilities with non-String values
205+
return Objects.equals(
206+
stereotype.getCapability(name), capabilities.getCapability(name));
207+
});
188208
}
189209

190210
public static Boolean matchConditionToRemoveCapability(Capabilities capabilities) {

‎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;
@@ -659,7 +660,7 @@ void multipleExtensionPrefixedCapabilitiesDoNotMatchWhenOneIsDifferent() {
659660
}
660661

661662
@Test
662-
void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
663+
void seleniumExtensionCapabilitiesAreIgnoredForMatching() {
663664
Capabilities stereotype =
664665
new ImmutableCapabilities(
665666
CapabilityType.BROWSER_NAME,
@@ -668,23 +669,83 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
668669
"84",
669670
CapabilityType.PLATFORM_NAME,
670671
Platform.WINDOWS,
671-
"goog:cheese",
672-
"amsterdam",
673-
"ms:fruit",
674-
"mango");
672+
"se:cdpVersion",
673+
1,
674+
"se:downloadsEnabled",
675+
true);
675676

676677
Capabilities capabilities =
678+
new ImmutableCapabilities(
679+
CapabilityType.BROWSER_NAME,
680+
"chrome",
681+
CapabilityType.BROWSER_VERSION,
682+
"84",
683+
CapabilityType.PLATFORM_NAME,
684+
Platform.WINDOWS,
685+
"se:cdpVersion",
686+
2,
687+
"se:downloadsEnabled",
688+
false);
689+
assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue();
690+
}
691+
692+
@Test
693+
void vendorOptionsCapabilitiesAreIgnoredForMatching() {
694+
Capabilities stereotype =
677695
new ImmutableCapabilities(
678696
CapabilityType.BROWSER_NAME,
679697
"chrome",
680698
CapabilityType.BROWSER_VERSION,
681699
"84",
682700
CapabilityType.PLATFORM_NAME,
683701
Platform.WINDOWS,
684-
"goog:cheese",
685-
"gouda",
686-
"ms:fruit",
687-
"orange");
702+
"food:fruitOptions",
703+
"mango",
704+
"dairy:options",
705+
Map.of("cheese", "amsterdam"));
706+
707+
Capabilities capabilities =
708+
new ImmutableCapabilities(
709+
CapabilityType.BROWSER_NAME,
710+
"chrome",
711+
CapabilityType.BROWSER_VERSION,
712+
"84",
713+
CapabilityType.PLATFORM_NAME,
714+
Platform.WINDOWS,
715+
"food:fruitOptions",
716+
"orange",
717+
"dairy:options",
718+
Map.of("cheese", "gouda"));
719+
assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue();
720+
}
721+
722+
@Test
723+
void specialExtensionCapabilitiesAreIgnoredForMatching() {
724+
Capabilities stereotype =
725+
new ImmutableCapabilities(
726+
CapabilityType.BROWSER_NAME,
727+
"chrome",
728+
CapabilityType.BROWSER_VERSION,
729+
"84",
730+
CapabilityType.PLATFORM_NAME,
731+
Platform.WINDOWS,
732+
"food:loggingPrefs",
733+
"mango",
734+
"food:debuggerAddress",
735+
Map.of("cheese", "amsterdam"));
736+
737+
Capabilities capabilities =
738+
new ImmutableCapabilities(
739+
CapabilityType.BROWSER_NAME,
740+
"chrome",
741+
CapabilityType.BROWSER_VERSION,
742+
"84",
743+
CapabilityType.PLATFORM_NAME,
744+
Platform.WINDOWS,
745+
"food:loggingPrefs",
746+
"orange",
747+
"food:debuggerAddress",
748+
Map.of("cheese", "gouda"));
688749
assertThat(slotMatcher.matches(stereotype, capabilities)).isTrue();
689750
}
690751

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