Skip to content

Commit

Permalink
Refactoring constructor that accepts desired capabilities. Need more …
Browse files Browse the repository at this point in the history
…test, though
  • Loading branch information
barancev committed Mar 2, 2017
1 parent ec80d11 commit 5ff67fc
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 34 deletions.
47 changes: 26 additions & 21 deletions java/client/src/org/openqa/selenium/firefox/FirefoxDriver.java
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public static final class SystemProperty {

// TODO: make it public as soon as it's fully implemented
FirefoxDriver(FirefoxOptions options) {
super(toExecutor(options), options.addTo(new DesiredCapabilities()), null);
super(toExecutor(options), options.toDesiredCapabilities(), null);
//binary = options.getBinary();
}

Expand Down Expand Up @@ -155,7 +155,30 @@ public FirefoxDriver(FirefoxBinary binary, FirefoxProfile profile) {
}

public FirefoxDriver(Capabilities desiredCapabilities) {
this(getBinary(desiredCapabilities), null, desiredCapabilities);
this(getFirefoxOptions(desiredCapabilities).addDesiredCapabilities(desiredCapabilities));
}

private static FirefoxOptions getFirefoxOptions(Capabilities capabilities) {
FirefoxOptions options = new FirefoxOptions();
if (capabilities != null) {
Object rawOptions = capabilities.getCapability(FIREFOX_OPTIONS);
if (rawOptions != null) {
if (rawOptions instanceof Map) {
try {
@SuppressWarnings("unchecked")
Map<String, Object> map = (Map<String, Object>) rawOptions;
rawOptions = FirefoxOptions.fromJsonMap(map);
} catch (IOException e) {
throw new WebDriverException(e);
}
}
if (rawOptions != null && !(rawOptions instanceof FirefoxOptions)) {
throw new WebDriverException("Firefox option was set, but is not a FirefoxOption: " + rawOptions);
}
options = (FirefoxOptions) rawOptions;
}
}
return options;
}

public FirefoxDriver(Capabilities desiredCapabilities, Capabilities requiredCapabilities) {
Expand Down Expand Up @@ -210,25 +233,7 @@ private static Capabilities addProfileTo(Capabilities capabilities, FirefoxProfi
toReturn.setCapability(FirefoxDriver.PROFILE, profile);

// geckodriver
FirefoxOptions options = new FirefoxOptions();
if (capabilities != null) {
Object rawOptions = capabilities.getCapability(FIREFOX_OPTIONS);
if (rawOptions != null) {
if (rawOptions instanceof Map) {
try {
@SuppressWarnings("unchecked")
Map<String, Object> map = (Map<String, Object>) rawOptions;
rawOptions = FirefoxOptions.fromJsonMap(map);
} catch (IOException e) {
throw new WebDriverException(e);
}
}
if (rawOptions != null && !(rawOptions instanceof FirefoxOptions)) {
throw new WebDriverException("Firefox option was set, but is not a FirefoxOption: " + rawOptions);
}
options = (FirefoxOptions) rawOptions;
}
}
FirefoxOptions options = getFirefoxOptions(capabilities);
options.setProfileSafely(profile);
toReturn.setCapability(FIREFOX_OPTIONS, options);

Expand Down
79 changes: 72 additions & 7 deletions java/client/src/org/openqa/selenium/firefox/FirefoxOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@
import com.google.gson.JsonObject;
import com.google.gson.JsonPrimitive;

import org.openqa.selenium.Capabilities;
import org.openqa.selenium.WebDriverException;
import org.openqa.selenium.firefox.internal.ProfilesIni;
import org.openqa.selenium.remote.DesiredCapabilities;

import java.io.IOException;
Expand Down Expand Up @@ -65,6 +67,7 @@ public class FirefoxOptions {
private Map<String, String> stringPrefs = new HashMap<>();
private Level logLevel = null;
private Boolean legacy;
private DesiredCapabilities desiredCapabilities = new DesiredCapabilities();

/** INTERNAL ONLY: DO NOT USE */
static FirefoxOptions fromJsonMap(Map<String, Object> map) throws IOException {
Expand Down Expand Up @@ -143,7 +146,7 @@ public boolean isLegacy() {
}

public FirefoxOptions setBinary(FirefoxBinary binary) {
this.binary = checkNotNull(binary);
this.binary = binary;
return this;
}

Expand All @@ -169,7 +172,14 @@ public FirefoxOptions setProfile(FirefoxProfile profile) {
}

public FirefoxProfile getProfile() {
return Optional.ofNullable(profile).orElse(new FirefoxProfile());
if (profile != null) {
return profile;
}

// if (requiredCapabilities != null && requiredCapabilities.getCapability(PROFILE) != null) {
// raw = requiredCapabilities.getCapability(PROFILE);
// }
return extractProfile(desiredCapabilities);
}

// Confusing API. Keeping package visible only
Expand Down Expand Up @@ -210,23 +220,73 @@ public FirefoxOptions setLogLevel(Level logLevel) {
return this;
}

public DesiredCapabilities addTo(DesiredCapabilities capabilities) {
public FirefoxOptions addDesiredCapabilities(Capabilities desiredCapabilities) {
this.desiredCapabilities.merge(desiredCapabilities);

FirefoxProfile suggestedProfile = extractProfile(desiredCapabilities);
if (suggestedProfile != null) {
if (!booleanPrefs.isEmpty() || !intPrefs.isEmpty() || !stringPrefs.isEmpty()) {
throw new IllegalStateException(
"Unable to determine if preferences set on this option " +
"are the same as the profile in the capabilities");
}
if (profile != null && !suggestedProfile.equals(profile)) {
throw new IllegalStateException(
"Profile has been set on both the capabilities and these options, but they're " +
"different. Unable to determine which one you want to use.");
}
profile = suggestedProfile;
}

return this;
}

private FirefoxProfile extractProfile(Capabilities desiredCapabilities) {
if (desiredCapabilities == null) {
return null;
}
Object raw = desiredCapabilities.getCapability(PROFILE);
if (raw == null) {
return null;

} else if (raw instanceof FirefoxProfile) {
return (FirefoxProfile) raw;

} else if (raw instanceof String) {
try {
return FirefoxProfile.fromJson((String) raw);
} catch (IOException e) {
throw new WebDriverException(e);
}
}

return null;
}

public Capabilities toDesiredCapabilities() {
DesiredCapabilities capabilities = new DesiredCapabilities(desiredCapabilities);

if (isLegacy()) {
capabilities.setCapability(FirefoxDriver.MARIONETTE, false);
}

Object priorBinary = capabilities.getCapability(BINARY);
if (binary != null && priorBinary != null && !binary.equals(priorBinary)) {
throw new IllegalStateException(
"Binary already set in capabilities, but is different from the one in these options");
"Binary already set in capabilities, but is different from the one in these options");
}

Object priorProfile = capabilities.getCapability(PROFILE);
if (priorProfile != null) {
if (!priorProfile.equals(profile)) {
if (!booleanPrefs.isEmpty() || !intPrefs.isEmpty() || !stringPrefs.isEmpty()) {
throw new IllegalStateException(
"Profile has been set on both the capabilities and these options, but they're " +
"different. Unable to determine which one you want to use.");
"Unable to determine if preferences set on this option " +
"are the same as the profile in the capabilities");
}
if (profile != null && !priorProfile.equals(profile)) {
throw new IllegalStateException(
"Profile has been set on both the capabilities and these options, but they're " +
"different. Unable to determine which one you want to use.");
}
}

Expand All @@ -244,6 +304,11 @@ public DesiredCapabilities addTo(DesiredCapabilities capabilities) {
return capabilities;
}

public DesiredCapabilities addTo(DesiredCapabilities capabilities) {
capabilities.merge(toDesiredCapabilities());
return capabilities;
}

public JsonElement toJson() throws IOException {
JsonObject options = new JsonObject();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.openqa.selenium.Capabilities;
import org.openqa.selenium.Dimension;
import org.openqa.selenium.JavascriptExecutor;
import org.openqa.selenium.UnexpectedAlertBehaviour;
import org.openqa.selenium.testing.NeedsFreshDriver;
import org.openqa.selenium.ParallelTestRunner;
import org.openqa.selenium.ParallelTestRunner.Worker;
Expand Down Expand Up @@ -83,7 +84,7 @@ public void quitDriver() {
}

@Test
public void canStartDriverWithNoParameters() throws InterruptedException {
public void canStartDriverWithNoParameters() {
localDriver = new FirefoxDriver();
verifyItIsLegacy(localDriver);
}
Expand All @@ -97,7 +98,7 @@ public void canStartDriverWithSpecifiedBinary() throws IOException {
}

@Test
public void canStartDriverWithSpecifiedProfile() throws IOException {
public void canStartDriverWithSpecifiedProfile() {
FirefoxProfile profile = new FirefoxProfile();
profile.setPreference("browser.startup.page", 1);
profile.setPreference("browser.startup.homepage", pages.xhtmlTestPage);
Expand All @@ -118,6 +119,16 @@ public void canStartDriverWithSpecifiedBinaryAndProfile() throws IOException {
verify(binary).startFirefoxProcess(any());
}

@Test
public void canPassCapabilities() {
DesiredCapabilities capabilities = new DesiredCapabilities();
capabilities.setCapability(CapabilityType.PAGE_LOAD_STRATEGY, "none");
localDriver = new FirefoxDriver(capabilities);
verifyItIsLegacy(localDriver);
assertEquals(
localDriver.getCapabilities().getCapability(CapabilityType.PAGE_LOAD_STRATEGY), "none");
}

private static class ConnectionCapturingDriver extends FirefoxDriver {
public ExtensionConnection keptConnection;

Expand Down
20 changes: 16 additions & 4 deletions java/client/test/org/openqa/selenium/firefox/MarionetteTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.openqa.selenium.firefox;

import static org.junit.Assert.assertEquals;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.never;
Expand All @@ -27,6 +28,7 @@

import org.junit.After;
import org.junit.Test;
import org.openqa.selenium.remote.CapabilityType;
import org.openqa.selenium.remote.DesiredCapabilities;
import org.openqa.selenium.testing.Ignore;
import org.openqa.selenium.testing.JUnit4TestBase;
Expand Down Expand Up @@ -68,7 +70,7 @@ public void canStartDriverWithSpecifiedBinary() throws IOException {
}

@Test
public void canStartDriverWithSpecifiedProfile() throws IOException {
public void canStartDriverWithSpecifiedProfile() {
FirefoxProfile profile = new FirefoxProfile();
profile.setPreference("browser.startup.page", 1);
profile.setPreference("browser.startup.homepage", pages.xhtmlTestPage);
Expand All @@ -91,7 +93,17 @@ public void canStartDriverWithSpecifiedBinaryAndProfile() throws IOException {
}

@Test
public void shouldUseFirefoxOptions() throws InterruptedException {
public void canPassCapabilities() {
DesiredCapabilities capabilities = new DesiredCapabilities();
capabilities.setCapability(CapabilityType.PAGE_LOAD_STRATEGY, "none");
localDriver = new FirefoxDriver(capabilities);
verifyItIsMarionette(localDriver);
assertEquals(
localDriver.getCapabilities().getCapability(CapabilityType.PAGE_LOAD_STRATEGY), "none");
}

@Test
public void canSetPreferencesInFirefoxOptions() {
DesiredCapabilities caps = new FirefoxOptions()
.addPreference("browser.startup.page", 1)
.addPreference("browser.startup.homepage", pages.xhtmlTestPage)
Expand All @@ -103,7 +115,7 @@ public void shouldUseFirefoxOptions() throws InterruptedException {
}

@Test
public void canSetProfileInFirefoxOptions() throws InterruptedException {
public void canSetProfileInFirefoxOptions() {
FirefoxProfile profile = new FirefoxProfile();
profile.setPreference("browser.startup.page", 1);
profile.setPreference("browser.startup.homepage", pages.xhtmlTestPage);
Expand All @@ -117,7 +129,7 @@ public void canSetProfileInFirefoxOptions() throws InterruptedException {
}

@Test
public void canSetProfileInCapabilities() throws InterruptedException {
public void canSetProfileInCapabilities() {
FirefoxProfile profile = new FirefoxProfile();
profile.setPreference("browser.startup.page", 1);
profile.setPreference("browser.startup.homepage", pages.xhtmlTestPage);
Expand Down

0 comments on commit 5ff67fc

Please sign in to comment.