[java] Deprecating methods that use FirefoxBinary as well.

We need to do that first so we can properly remove FirefoxBinary

Author: diemol

java/src/org/openqa/selenium/firefox/Executable.java

@@ -26,7 +26,13 @@
 import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.internal.Require;
 
-/** Wrapper around Firefox executable. */
+/**
+ * Wrapper around Firefox executable.
+ *
+ * @deprecated Class {@link FirefoxBinary} will be removed in the future. Functionality of this
+ *     class is already covered by Selenium Manager.
+ */
+@Deprecated
 class Executable {
 
   private final File binary;
@@ -57,6 +63,12 @@ public String getVersion() {
     return version;
   }
 
+  /**
+   * @deprecated Use {@link FirefoxOptions#setBinary(Path)} or {@link
+   *     FirefoxOptions#setBinary(String)} instead. Class {@link Executable} will also be removed in
+   *     the future.
+   */
+  @Deprecated
   public FirefoxBinary.Channel getChannel() {
     if (channel == null) {
       loadChannelPref();

java/src/org/openqa/selenium/firefox/FirefoxBinary.java

@@ -48,7 +48,10 @@
 @Deprecated
 public class FirefoxBinary {
 
-  /** Enumerates Firefox channels, according to https://wiki.mozilla.org/RapidRelease */
+  /**
+   * Enumerates Firefox channels, according to <a
+   * href="https://wiki.mozilla.org/RapidRelease">...</a>
+   */
   public enum Channel {
     ESR("esr"),
     RELEASE("release"),

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

@@ -152,11 +152,23 @@ public FirefoxOptions configureFromEnv() {
   /**
    * Constructs a {@link FirefoxBinary} and returns that to be used, and because of this is only
    * useful when actually starting firefox.
+   *
+   * @deprecated This method is deprecated and will be removed in a future version. Selenium Manager
+   *     handles this for you.
    */
+  @Deprecated
   public FirefoxBinary getBinary() {
     return getBinaryOrNull().orElseGet(FirefoxBinary::new);
   }
 
+  /**
+   * Sets the path to the Firefox binary to use. This is useful when you have multiple versions of
+   * Firefox installed on your machine.
+   *
+   * @deprecated This method is deprecated and will be removed in a future version. Use {@link
+   *     #setBinary(Path)} or {@link #setBinary(String)} instead.
+   */
+  @Deprecated
   public FirefoxOptions setBinary(FirefoxBinary binary) {
     Require.nonNull("Binary", binary);
     addArguments(binary.getExtraOptions());
@@ -173,6 +185,14 @@ public FirefoxOptions setBinary(String path) {
     return setFirefoxOption(Keys.BINARY, path);
   }
 
+  /**
+   * Returns the binary as a {@link FirefoxBinary} if it was set, or an empty {@link Optional} if
+   * not.
+   *
+   * @deprecated This method is deprecated and will be removed in a future version. Selenium Manager
+   *     handles this for you.}
+   */
+  @Deprecated
   public Optional<FirefoxBinary> getBinaryOrNull() {
     Object binary = firefoxOptions.get(Keys.BINARY.key());
     if (!(binary instanceof String)) {

java/test/org/openqa/selenium/firefox/FirefoxDriverTest.java

@@ -21,11 +21,8 @@
 import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
 import static org.junit.jupiter.api.Assertions.fail;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
-import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.spy;
-import static org.mockito.Mockito.verify;
 import static org.openqa.selenium.WaitingConditions.elementValueToEqual;
 import static org.openqa.selenium.firefox.FirefoxAssumptions.assumeDefaultBrowserLocationUsed;
 import static org.openqa.selenium.remote.CapabilityType.ACCEPT_INSECURE_CERTS;
@@ -43,7 +40,6 @@
 import org.mockito.ArgumentMatchers;
 import org.openqa.selenium.By;
 import org.openqa.selenium.Capabilities;
-import org.openqa.selenium.Dimension;
 import org.openqa.selenium.HasCapabilities;
 import org.openqa.selenium.ImmutableCapabilities;
 import org.openqa.selenium.JavascriptExecutor;
@@ -96,17 +92,6 @@ public void canStartDriverWithNoParameters() {
         .isEqualTo("firefox");
   }
 
-  @Test
-  @NoDriverBeforeTest
-  public void canStartDriverWithSpecifiedBinary() {
-    FirefoxBinary binary = spy(new FirefoxBinary());
-    FirefoxOptions options = new FirefoxOptions().setBinary(binary);
-
-    localDriver = new WebDriverBuilder().get(options);
-
-    verify(binary, atLeastOnce()).getPath();
-  }
-
   @Test
   void shouldGetMeaningfulExceptionOnBrowserDeath() throws Exception {
     RemoteWebDriver driver2 = (RemoteWebDriver) new WebDriverBuilder().get();
@@ -175,20 +160,6 @@ public void shouldBeAbleToStartANewInstanceEvenWithVerboseLogging() {
     new FirefoxDriver(service, (FirefoxOptions) FIREFOX.getCapabilities()).quit();
   }
 
-  @Test
-  @NoDriverBeforeTest
-  public void shouldBeAbleToPassCommandLineOptions() {
-    FirefoxBinary binary = new FirefoxBinary();
-    binary.addCommandLineOptions("-width", "800", "-height", "600");
-
-    localDriver = new WebDriverBuilder().get(new FirefoxOptions().setBinary(binary));
-    Dimension size = localDriver.manage().window().getSize();
-    assertThat(size.width).isGreaterThanOrEqualTo(800);
-    assertThat(size.width).isLessThan(850);
-    assertThat(size.height).isGreaterThanOrEqualTo(600);
-    assertThat(size.height).isLessThan(650);
-  }
-
   @Test
   @NoDriverBeforeTest
   public void canPassCapabilities() {

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

@@ -17,9 +17,7 @@
 
 package org.openqa.selenium.firefox;
 
-import static java.nio.file.StandardOpenOption.DELETE_ON_CLOSE;
 import static java.util.Collections.emptyMap;
-import static java.util.Collections.singleton;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatNoException;
@@ -28,7 +26,6 @@
 import static org.assertj.core.api.InstanceOfAssertFactories.MAP;
 import static org.assertj.core.api.InstanceOfAssertFactories.STRING;
 import static org.openqa.selenium.PageLoadStrategy.EAGER;
-import static org.openqa.selenium.firefox.FirefoxDriver.SystemProperty.BROWSER_BINARY;
 import static org.openqa.selenium.firefox.FirefoxDriver.SystemProperty.BROWSER_PROFILE;
 import static org.openqa.selenium.firefox.FirefoxDriverLogLevel.DEBUG;
 import static org.openqa.selenium.firefox.FirefoxDriverLogLevel.ERROR;
@@ -40,11 +37,9 @@
 import com.google.common.collect.ImmutableMap;
 import java.io.File;
 import java.io.IOException;
-import java.io.OutputStream;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.nio.file.attribute.PosixFilePermission;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
@@ -56,7 +51,6 @@
 import org.openqa.selenium.ImmutableCapabilities;
 import org.openqa.selenium.MutableCapabilities;
 import org.openqa.selenium.PageLoadStrategy;
-import org.openqa.selenium.Platform;
 import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.internal.Require;
 import org.openqa.selenium.testing.TestUtilities;
@@ -91,43 +85,6 @@ void binaryPathNeedNotExist() {
     new FirefoxOptions().setBinary("does/not/exist");
   }
 
-  @Test
-  void shouldKeepRelativePathToBinaryAsIs() {
-    String path = String.join(File.separator, "some", "path");
-    FirefoxOptions options = new FirefoxOptions().setBinary(path);
-    assertThat(options.getBinary())
-        .extracting(FirefoxBinary::getFile)
-        .extracting(String::valueOf)
-        .isEqualTo(path);
-  }
-
-  @Test
-  void shouldKeepWindowsDriveLetterInPathToBinary() {
-    FirefoxOptions options = new FirefoxOptions().setBinary("F:\\some\\path");
-    assertThat(options.getBinary())
-        .extracting(FirefoxBinary::getFile)
-        .extracting(String::valueOf)
-        .isEqualTo("F:\\some\\path");
-  }
-
-  @Test
-  void shouldKeepWindowsNetworkFileSystemRootInPathToBinary() {
-    FirefoxOptions options = new FirefoxOptions().setBinary("\\\\server\\share\\some\\path");
-    assertThat(options.getBinary())
-        .extracting(FirefoxBinary::getFile)
-        .extracting(String::valueOf)
-        .isEqualTo("\\\\server\\share\\some\\path");
-  }
-
-  @Test
-  void shouldKeepAFirefoxBinaryAsABinaryIfSetAsOne() throws IOException {
-    File fakeExecutable = Files.createTempFile("firefox", ".exe").toFile();
-    fakeExecutable.deleteOnExit();
-    FirefoxBinary binary = new FirefoxBinary(fakeExecutable);
-    FirefoxOptions options = new FirefoxOptions().setBinary(binary);
-    assertThat(options.getBinary().getFile()).isEqualTo(binary.getFile());
-  }
-
   @Test
   void stringBasedBinaryRemainsAbsoluteIfSetAsAbsolute() {
     Map<String, Object> json = new FirefoxOptions().setBinary("/i/like/cheese").asMap();
@@ -147,28 +104,6 @@ void pathBasedBinaryRemainsAbsoluteIfSetAsAbsolute() {
         .containsEntry("binary", path);
   }
 
-  @Test
-  void shouldPickUpBinaryFromSystemPropertyIfSet() throws IOException {
-    JreSystemProperty property = new JreSystemProperty(BROWSER_BINARY);
-
-    Path binary = Files.createTempFile("firefox", ".exe");
-    try (OutputStream ignored = Files.newOutputStream(binary, DELETE_ON_CLOSE)) {
-      Files.write(binary, "".getBytes());
-      if (!TestUtilities.getEffectivePlatform().is(Platform.WINDOWS)) {
-        Files.setPosixFilePermissions(binary, singleton(PosixFilePermission.OWNER_EXECUTE));
-      }
-      property.set(binary.toString());
-      FirefoxOptions options = new FirefoxOptions().configureFromEnv();
-
-      FirefoxBinary firefoxBinary =
-          options.getBinaryOrNull().orElseThrow(() -> new AssertionError("No binary"));
-
-      assertThat(firefoxBinary.getPath()).isEqualTo(binary.toString());
-    } finally {
-      property.reset();
-    }
-  }
-
   @Test
   void shouldPickUpProfileFromSystemProperty() {
     FirefoxProfile defaultProfile = new ProfilesIni().getProfile("default");