Skip to content

Commit e43cce3

Browse files
barancevbarancev
committed
[java] Fixing FirefoxOptions.merge to consume data from both instances to be merged
1 parent 7c8e47a commit e43cce3

2 files changed

Lines changed: 66 additions & 32 deletions

File tree

‎java/client/src/org/openqa/selenium/firefox/FirefoxOptions.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,7 @@ public FirefoxOptions merge(Capabilities capabilities) {
328328
FirefoxOptions newInstance = new FirefoxOptions();
329329
this.asMap().forEach(newInstance::setCapability);
330330
capabilities.asMap().forEach(newInstance::setCapability);
331+
newInstance.mirror(this);
331332
if (capabilities instanceof FirefoxOptions) {
332333
newInstance.mirror((FirefoxOptions) capabilities);
333334
}

‎java/client/test/org/openqa/selenium/firefox/FirefoxOptionsTest.java‎

Lines changed: 65 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import static org.assertj.core.api.Assertions.assertThat;
2424
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
2525
import static org.assertj.core.api.Assumptions.assumeThat;
26+
import static org.assertj.core.api.InstanceOfAssertFactories.LIST;
27+
import static org.assertj.core.api.InstanceOfAssertFactories.MAP;
2628
import static org.openqa.selenium.PageLoadStrategy.EAGER;
2729
import static org.openqa.selenium.firefox.FirefoxDriver.Capability.BINARY;
2830
import static org.openqa.selenium.firefox.FirefoxDriver.Capability.MARIONETTE;
@@ -68,9 +70,9 @@ public class FirefoxOptionsTest {
6870
@Test
6971
public void canInitFirefoxOptionsWithCapabilities() {
7072
FirefoxOptions options = new FirefoxOptions(new ImmutableCapabilities(
71-
MARIONETTE, false,
72-
PAGE_LOAD_STRATEGY, PageLoadStrategy.EAGER,
73-
ACCEPT_INSECURE_CERTS, true));
73+
MARIONETTE, false,
74+
PAGE_LOAD_STRATEGY, PageLoadStrategy.EAGER,
75+
ACCEPT_INSECURE_CERTS, true));
7476

7577
assertThat(options.isLegacy()).isTrue();
7678
assertThat(options.getCapability(PAGE_LOAD_STRATEGY)).isEqualTo(EAGER);
@@ -80,7 +82,7 @@ public void canInitFirefoxOptionsWithCapabilities() {
8082
@Test
8183
public void canInitFirefoxOptionsWithCapabilitiesThatContainFirefoxOptions() {
8284
FirefoxOptions options = new FirefoxOptions().setLegacy(true).merge(
83-
new ImmutableCapabilities(PAGE_LOAD_STRATEGY, PageLoadStrategy.EAGER));
85+
new ImmutableCapabilities(PAGE_LOAD_STRATEGY, PageLoadStrategy.EAGER));
8486
Capabilities caps = new ImmutableCapabilities(FIREFOX_OPTIONS, options);
8587

8688
FirefoxOptions options2 = new FirefoxOptions(caps);
@@ -93,7 +95,7 @@ public void canInitFirefoxOptionsWithCapabilitiesThatContainFirefoxOptions() {
9395
public void canInitFirefoxOptionsWithCapabilitiesThatContainFirefoxOptionsAsMap() {
9496
FirefoxProfile profile = new FirefoxProfile();
9597
Capabilities caps = new ImmutableCapabilities(
96-
FIREFOX_OPTIONS, ImmutableMap.of("profile", profile));
98+
FIREFOX_OPTIONS, ImmutableMap.of("profile", profile));
9799

98100
FirefoxOptions options = new FirefoxOptions(caps);
99101

@@ -150,8 +152,8 @@ public void stringBasedBinaryRemainsAbsoluteIfSetAsAbsolute() {
150152
Map<String, Object> json = new FirefoxOptions().setBinary("/i/like/cheese").asMap();
151153

152154
assertThat(json.get(FIREFOX_OPTIONS))
153-
.asInstanceOf(InstanceOfAssertFactories.MAP)
154-
.containsEntry("binary", "/i/like/cheese");
155+
.asInstanceOf(InstanceOfAssertFactories.MAP)
156+
.containsEntry("binary", "/i/like/cheese");
155157
}
156158

157159
@Test
@@ -177,7 +179,7 @@ public void shouldPickUpBinaryFromSystemPropertyIfSet() throws IOException {
177179
FirefoxOptions options = new FirefoxOptions();
178180

179181
FirefoxBinary firefoxBinary =
180-
options.getBinaryOrNull().orElseThrow(() -> new AssertionError("No binary"));
182+
options.getBinaryOrNull().orElseThrow(() -> new AssertionError("No binary"));
181183

182184
assertThat(firefoxBinary.getPath()).isEqualTo(binary.toString());
183185
} finally {
@@ -249,7 +251,7 @@ public void shouldThrowAnExceptionIfSystemPropertyProfileDoesNotExist() {
249251
try {
250252
property.set(unlikelyProfileName);
251253
assertThatExceptionOfType(WebDriverException.class)
252-
.isThrownBy(FirefoxOptions::new);
254+
.isThrownBy(FirefoxOptions::new);
253255
} finally {
254256
property.reset();
255257
}
@@ -258,7 +260,7 @@ public void shouldThrowAnExceptionIfSystemPropertyProfileDoesNotExist() {
258260
@Test
259261
public void callingToStringWhenTheBinaryDoesNotExistShouldNotCauseAnException() {
260262
FirefoxOptions options =
261-
new FirefoxOptions().setBinary("there's nothing better in life than cake or peas.");
263+
new FirefoxOptions().setBinary("there's nothing better in life than cake or peas.");
262264
options.toString();
263265
// The binary does not exist on this machine, but could do elsewhere. Be chill.
264266
}
@@ -277,45 +279,46 @@ public void canBuildLogLevelFromStringRepresentation() {
277279
@Test
278280
public void canConvertOptionsWithArgsToCapabilitiesAndRestoreBack() {
279281
FirefoxOptions options = new FirefoxOptions(
280-
new MutableCapabilities(new FirefoxOptions().addArguments("-a", "-b")));
282+
new MutableCapabilities(new FirefoxOptions().addArguments("-a", "-b")));
281283
Object options2 = options.asMap().get(FirefoxOptions.FIREFOX_OPTIONS);
282284
assertThat(options2)
283-
.asInstanceOf(InstanceOfAssertFactories.MAP)
284-
.containsEntry("args", Arrays.asList("-a", "-b"));
285+
.asInstanceOf(InstanceOfAssertFactories.MAP)
286+
.containsEntry("args", Arrays.asList("-a", "-b"));
285287
}
286288

287289
@Test
288290
public void canConvertOptionsWithPrefsToCapabilitiesAndRestoreBack() {
289291
FirefoxOptions options = new FirefoxOptions(
290-
new MutableCapabilities(new FirefoxOptions()
291-
.addPreference("string.pref", "some value")
292-
.addPreference("int.pref", 42)
293-
.addPreference("boolean.pref", true)));
292+
new MutableCapabilities(
293+
new FirefoxOptions()
294+
.addPreference("string.pref", "some value")
295+
.addPreference("int.pref", 42)
296+
.addPreference("boolean.pref", true)));
294297
Object options2 = options.asMap().get(FirefoxOptions.FIREFOX_OPTIONS);
295298
assertThat(options2)
296-
.asInstanceOf(InstanceOfAssertFactories.MAP)
297-
.extractingByKey("prefs")
298-
.asInstanceOf(InstanceOfAssertFactories.MAP)
299-
.containsEntry("string.pref", "some value")
300-
.containsEntry("int.pref", 42)
301-
.containsEntry("boolean.pref", true);
299+
.asInstanceOf(InstanceOfAssertFactories.MAP)
300+
.extractingByKey("prefs")
301+
.asInstanceOf(InstanceOfAssertFactories.MAP)
302+
.containsEntry("string.pref", "some value")
303+
.containsEntry("int.pref", 42)
304+
.containsEntry("boolean.pref", true);
302305
}
303306

304307
@Test
305308
public void canConvertOptionsWithBinaryToCapabilitiesAndRestoreBack() {
306309
FirefoxOptions options = new FirefoxOptions(
307-
new MutableCapabilities(new FirefoxOptions().setBinary(new FirefoxBinary())));
310+
new MutableCapabilities(new FirefoxOptions().setBinary(new FirefoxBinary())));
308311
Object options2 = options.asMap().get(FirefoxOptions.FIREFOX_OPTIONS);
309312
assertThat(options2)
310-
.asInstanceOf(InstanceOfAssertFactories.MAP)
311-
.containsEntry("binary", new FirefoxBinary().getPath().replaceAll("\\\\", "/"));
313+
.asInstanceOf(InstanceOfAssertFactories.MAP)
314+
.containsEntry("binary", new FirefoxBinary().getPath().replaceAll("\\\\", "/"));
312315
}
313316

314317
@Test
315318
public void roundTrippingToCapabilitiesAndBackWorks() {
316319
FirefoxOptions expected = new FirefoxOptions()
317-
.setLegacy(true)
318-
.addPreference("cake", "walk");
320+
.setLegacy(true)
321+
.addPreference("cake", "walk");
319322

320323
// Convert to a Map so we can create a standalone capabilities instance, which we then use to
321324
// create a new set of options. This is the round trip, ladies and gentlemen.
@@ -328,19 +331,49 @@ public void roundTrippingToCapabilitiesAndBackWorks() {
328331
public void optionsAsMapShouldBeImmutable() {
329332
Map<String, Object> options = new FirefoxOptions().asMap();
330333
assertThatExceptionOfType(UnsupportedOperationException.class)
331-
.isThrownBy(() -> options.put("browserName", "chrome"));
334+
.isThrownBy(() -> options.put("browserName", "chrome"));
332335

333336
Map<String, Object> mozOptions = (Map<String, Object>) options.get(FIREFOX_OPTIONS);
334337
assertThatExceptionOfType(UnsupportedOperationException.class)
335-
.isThrownBy(() -> mozOptions.put("prefs", emptyMap()));
338+
.isThrownBy(() -> mozOptions.put("prefs", emptyMap()));
336339

337340
Map<String, Object> prefs = (Map<String, Object>) mozOptions.get("prefs");
338341
assertThatExceptionOfType(UnsupportedOperationException.class)
339-
.isThrownBy(() -> prefs.put("x", true));
342+
.isThrownBy(() -> prefs.put("x", true));
340343

341344
List<String> args = (List<String>) mozOptions.get("args");
342345
assertThatExceptionOfType(UnsupportedOperationException.class)
343-
.isThrownBy(() -> args.add("-help"));
346+
.isThrownBy(() -> args.add("-help"));
347+
}
348+
349+
@Test
350+
public void mergingOptionsMergesArguments() {
351+
FirefoxOptions one = new FirefoxOptions().addArguments("verbose");
352+
FirefoxOptions two = new FirefoxOptions().addArguments("silent");
353+
FirefoxOptions merged = one.merge(two);
354+
355+
assertThat(merged.asMap()).asInstanceOf(MAP)
356+
.extractingByKey(FirefoxOptions.FIREFOX_OPTIONS).asInstanceOf(MAP)
357+
.extractingByKey("args").asInstanceOf(LIST)
358+
.containsExactly("verbose", "silent");
359+
}
360+
361+
@Test
362+
public void mergingOptionsMergesPreferences() {
363+
FirefoxOptions one = new FirefoxOptions()
364+
.addPreference("opt1", "val1")
365+
.addPreference("opt2", "val2");
366+
FirefoxOptions two = new FirefoxOptions()
367+
.addPreference("opt2", "val4")
368+
.addPreference("opt3", "val3");
369+
FirefoxOptions merged = one.merge(two);
370+
371+
assertThat(merged.asMap()).asInstanceOf(MAP)
372+
.extractingByKey(FirefoxOptions.FIREFOX_OPTIONS).asInstanceOf(MAP)
373+
.extractingByKey("prefs").asInstanceOf(MAP)
374+
.containsEntry("opt1", "val1")
375+
.containsEntry("opt2", "val4")
376+
.containsEntry("opt3", "val3");
344377
}
345378

346379
private static class JreSystemProperty {

0 commit comments

Comments
 (0)