From 5ff67fc2da04f660c0281fdca7116d7bd7be7e39 Mon Sep 17 00:00:00 2001 From: Alexei Barantsev Date: Wed, 22 Feb 2017 19:51:46 +0300 Subject: [PATCH] Refactoring constructor that accepts desired capabilities. Need more test, though --- .../selenium/firefox/FirefoxDriver.java | 47 ++++++----- .../selenium/firefox/FirefoxOptions.java | 79 +++++++++++++++++-- .../selenium/firefox/FirefoxDriverTest.java | 15 +++- .../selenium/firefox/MarionetteTest.java | 20 ++++- 4 files changed, 127 insertions(+), 34 deletions(-) diff --git a/java/client/src/org/openqa/selenium/firefox/FirefoxDriver.java b/java/client/src/org/openqa/selenium/firefox/FirefoxDriver.java index e319e65497b4d..95f128af3dc9d 100644 --- a/java/client/src/org/openqa/selenium/firefox/FirefoxDriver.java +++ b/java/client/src/org/openqa/selenium/firefox/FirefoxDriver.java @@ -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(); } @@ -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 map = (Map) 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) { @@ -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 map = (Map) 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); diff --git a/java/client/src/org/openqa/selenium/firefox/FirefoxOptions.java b/java/client/src/org/openqa/selenium/firefox/FirefoxOptions.java index 32c5601d3b674..c72ebee57a30c 100644 --- a/java/client/src/org/openqa/selenium/firefox/FirefoxOptions.java +++ b/java/client/src/org/openqa/selenium/firefox/FirefoxOptions.java @@ -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; @@ -65,6 +67,7 @@ public class FirefoxOptions { private Map stringPrefs = new HashMap<>(); private Level logLevel = null; private Boolean legacy; + private DesiredCapabilities desiredCapabilities = new DesiredCapabilities(); /** INTERNAL ONLY: DO NOT USE */ static FirefoxOptions fromJsonMap(Map map) throws IOException { @@ -143,7 +146,7 @@ public boolean isLegacy() { } public FirefoxOptions setBinary(FirefoxBinary binary) { - this.binary = checkNotNull(binary); + this.binary = binary; return this; } @@ -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 @@ -210,7 +220,52 @@ 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); } @@ -218,15 +273,20 @@ public DesiredCapabilities addTo(DesiredCapabilities capabilities) { 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."); } } @@ -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(); diff --git a/java/client/test/org/openqa/selenium/firefox/FirefoxDriverTest.java b/java/client/test/org/openqa/selenium/firefox/FirefoxDriverTest.java index bb2442c94a197..5128fd2d7dc65 100644 --- a/java/client/test/org/openqa/selenium/firefox/FirefoxDriverTest.java +++ b/java/client/test/org/openqa/selenium/firefox/FirefoxDriverTest.java @@ -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; @@ -83,7 +84,7 @@ public void quitDriver() { } @Test - public void canStartDriverWithNoParameters() throws InterruptedException { + public void canStartDriverWithNoParameters() { localDriver = new FirefoxDriver(); verifyItIsLegacy(localDriver); } @@ -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); @@ -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; diff --git a/java/client/test/org/openqa/selenium/firefox/MarionetteTest.java b/java/client/test/org/openqa/selenium/firefox/MarionetteTest.java index 042bd36f94266..a5d153de8d293 100644 --- a/java/client/test/org/openqa/selenium/firefox/MarionetteTest.java +++ b/java/client/test/org/openqa/selenium/firefox/MarionetteTest.java @@ -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; @@ -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; @@ -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); @@ -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) @@ -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); @@ -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);