Skip to content

Commit 442cdcf

Browse files
sbabcocsbabcoc
committed
Ignore caps with special suffixes
1 parent 583d008 commit 442cdcf

File tree

3 files changed

+135
-51
lines changed

3 files changed

+135
-51
lines changed

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

+60-41
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,24 @@
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>"se:" - Selenium
51+
* </ul>
52+
* <li>Extension capabilities with these suffixes:
53+
* <ul>
54+
* <li>"Options"
55+
* <li>"options"
56+
* <li>"loggingPrefs"
57+
* <li>"debuggerAddress"
58+
* </ul>
59+
* </ul>
4460
*/
4561
public class DefaultSlotMatcher implements SlotMatcher, Serializable {
4662

@@ -51,6 +67,13 @@ public class DefaultSlotMatcher implements SlotMatcher, Serializable {
5167
private static final List<String> EXTENSION_CAPABILITIES_PREFIXES =
5268
Arrays.asList("goog:", "moz:", "ms:", "se:");
5369

70+
/*
71+
List of extension capability suffixes we never should try to match, they should be
72+
matched in the Node or in the browser driver.
73+
*/
74+
private static final List<String> EXTENSION_CAPABILITY_SUFFIXES =
75+
Arrays.asList("Options", "options", "loggingPrefs", "debuggerAddress");
76+
5477
@Override
5578
public boolean matches(Capabilities stereotype, Capabilities capabilities) {
5679

@@ -76,14 +99,14 @@ public boolean matches(Capabilities stereotype, Capabilities capabilities) {
7699

77100
// At the end, a simple browser, browserVersion and platformName match
78101
boolean browserNameMatch =
79-
(capabilities.getBrowserName() == null || capabilities.getBrowserName().isEmpty())
102+
capabilities.getBrowserName() == null
103+
|| capabilities.getBrowserName().isEmpty()
80104
|| Objects.equals(stereotype.getBrowserName(), capabilities.getBrowserName());
81105
boolean browserVersionMatch =
82-
(capabilities.getBrowserVersion() == null
83-
|| capabilities.getBrowserVersion().isEmpty()
84-
|| Objects.equals(capabilities.getBrowserVersion(), "stable"))
85-
|| browserVersionMatch(
86-
stereotype.getBrowserVersion(), capabilities.getBrowserVersion());
106+
capabilities.getBrowserVersion() == null
107+
|| capabilities.getBrowserVersion().isEmpty()
108+
|| Objects.equals(capabilities.getBrowserVersion(), "stable")
109+
|| browserVersionMatch(stereotype.getBrowserVersion(), capabilities.getBrowserVersion());
87110
boolean platformNameMatch =
88111
capabilities.getPlatformName() == null
89112
|| Objects.equals(stereotype.getPlatformName(), capabilities.getPlatformName())
@@ -102,21 +125,17 @@ private Boolean initialMatch(Capabilities stereotype, Capabilities capabilities)
102125
.filter(name -> !name.contains(":"))
103126
// Platform matching is special, we do it later
104127
.filter(name -> !"platformName".equalsIgnoreCase(name))
105-
.map(
128+
.filter(name -> capabilities.getCapability(name) != null)
129+
.allMatch(
106130
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));
116-
}
117-
})
118-
.reduce(Boolean::logicalAnd)
119-
.orElse(true);
131+
if (stereotype.getCapability(name) instanceof String
132+
&& capabilities.getCapability(name) instanceof String) {
133+
return ((String) stereotype.getCapability(name))
134+
.equalsIgnoreCase((String) capabilities.getCapability(name));
135+
}
136+
return Objects.equals(
137+
stereotype.getCapability(name), capabilities.getCapability(name));
138+
});
120139
}
121140

122141
private Boolean managedDownloadsEnabled(Capabilities stereotype, Capabilities capabilities) {
@@ -140,13 +159,11 @@ private Boolean platformVersionMatch(Capabilities stereotype, Capabilities capab
140159
*/
141160
return capabilities.getCapabilityNames().stream()
142161
.filter(name -> name.contains("platformVersion"))
143-
.map(
162+
.allMatch(
144163
platformVersionCapName ->
145164
Objects.equals(
146165
stereotype.getCapability(platformVersionCapName),
147-
capabilities.getCapability(platformVersionCapName)))
148-
.reduce(Boolean::logicalAnd)
149-
.orElse(true);
166+
capabilities.getCapability(platformVersionCapName)));
150167
}
151168

152169
private Boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities capabilities) {
@@ -156,23 +173,25 @@ private Boolean extensionCapabilitiesMatch(Capabilities stereotype, Capabilities
156173
of the new session request contains that specific extension capability.
157174
*/
158175
return stereotype.getCapabilityNames().stream()
176+
// examine only extension capabilities
159177
.filter(name -> name.contains(":"))
160-
.filter(name -> capabilities.asMap().containsKey(name))
178+
// ignore special extension capability prefixes
161179
.filter(name -> EXTENSION_CAPABILITIES_PREFIXES.stream().noneMatch(name::contains))
162-
.map(
180+
// ignore special extension capability suffixes
181+
.filter(name -> EXTENSION_CAPABILITY_SUFFIXES.stream().noneMatch(name::endsWith))
182+
// ignore capabilities not specified in the request
183+
.filter(name -> capabilities.getCapability(name) != null)
184+
.allMatch(
163185
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-
}
174-
})
175-
.reduce(Boolean::logicalAnd)
176-
.orElse(true);
186+
// evaluate capabilities with String values
187+
if (stereotype.getCapability(name) instanceof String
188+
&& capabilities.getCapability(name) instanceof String) {
189+
return ((String) stereotype.getCapability(name))
190+
.equalsIgnoreCase((String) capabilities.getCapability(name));
191+
}
192+
// evaluate capabilities with non-String values
193+
return Objects.equals(
194+
stereotype.getCapability(name), capabilities.getCapability(name));
195+
});
177196
}
178197
}

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

+70-9
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,7 @@ void multipleExtensionPrefixedCapabilitiesDoNotMatchWhenOneIsDifferent() {
540541
}
541542

542543
@Test
543-
void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
544+
void seleniumExtensionCapabilitiesAreIgnoredForMatching() {
544545
Capabilities stereotype =
545546
new ImmutableCapabilities(
546547
CapabilityType.BROWSER_NAME,
@@ -549,23 +550,83 @@ void vendorExtensionPrefixedCapabilitiesAreIgnoredForMatching() {
549550
"84",
550551
CapabilityType.PLATFORM_NAME,
551552
Platform.WINDOWS,
552-
"goog:cheese",
553-
"amsterdam",
554-
"ms:fruit",
555-
"mango");
553+
"se:cdpVersion",
554+
1,
555+
"se:downloadsEnabled",
556+
true);
556557

557558
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() {
575+
Capabilities stereotype =
558576
new ImmutableCapabilities(
559577
CapabilityType.BROWSER_NAME,
560578
"chrome",
561579
CapabilityType.BROWSER_VERSION,
562580
"84",
563581
CapabilityType.PLATFORM_NAME,
564582
Platform.WINDOWS,
565-
"goog:cheese",
566-
"gouda",
567-
"ms:fruit",
568-
"orange");
583+
"food:fruitOptions",
584+
"mango",
585+
"dairy:options",
586+
Map.of("cheese", "amsterdam"));
587+
588+
Capabilities capabilities =
589+
new ImmutableCapabilities(
590+
CapabilityType.BROWSER_NAME,
591+
"chrome",
592+
CapabilityType.BROWSER_VERSION,
593+
"84",
594+
CapabilityType.PLATFORM_NAME,
595+
Platform.WINDOWS,
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

+5-1
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)