[dotnet] options do not belong in the service class (#12534)

* [dotnet] options do not belong in the service class

* [dotnet] put service parameters back in test code

dotnet/src/webdriver/Chrome/ChromeDriver.cs

@@ -83,7 +83,7 @@ public ChromeDriver()
         /// </summary>
         /// <param name="options">The <see cref="ChromeOptions"/> to be used with the Chrome driver.</param>
         public ChromeDriver(ChromeOptions options)
-            : this(ChromeDriverService.CreateDefaultService(options), options, RemoteWebDriver.DefaultCommandTimeout)
+            : this(ChromeDriverService.CreateDefaultService(), options, RemoteWebDriver.DefaultCommandTimeout)
         {
         }
 

dotnet/src/webdriver/Chrome/ChromeDriverService.cs

@@ -16,6 +16,7 @@
 // limitations under the License.
 // </copyright>
 
+using System;
 using System.IO;
 using OpenQA.Selenium.Chromium;
 using OpenQA.Selenium.Internal;
@@ -46,14 +47,15 @@ private ChromeDriverService(string executablePath, string executableFileName, in
         /// <returns>A ChromeDriverService that implements default settings.</returns>
         public static ChromeDriverService CreateDefaultService()
         {
-            return CreateDefaultService(new ChromeOptions());
+            return new ChromeDriverService(null, null, PortUtilities.FindFreePort());
         }
 
         /// <summary>
         /// Creates a default instance of the ChromeDriverService.
         /// </summary>
         /// /// <param name="options">Browser options used to find the correct ChromeDriver binary.</param>
         /// <returns>A ChromeDriverService that implements default settings.</returns>
+        [Obsolete("CreateDefaultService() now evaluates options in Driver constructor")]
         public static ChromeDriverService CreateDefaultService(ChromeOptions options)
         {
             string fullServicePath = DriverFinder.FullPath(options);

dotnet/src/webdriver/Chromium/ChromiumDriver.cs

@@ -19,6 +19,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
+using System.IO;
 using OpenQA.Selenium.DevTools;
 using OpenQA.Selenium.Remote;
 
@@ -125,11 +126,28 @@ public class ChromiumDriver : WebDriver, ISupportsLogs, IDevTools
         /// <param name="options">The <see cref="ChromiumOptions"/> to be used with the ChromiumDriver.</param>
         /// <param name="commandTimeout">The maximum amount of time to wait for each command.</param>
         protected ChromiumDriver(ChromiumDriverService service, ChromiumOptions options, TimeSpan commandTimeout)
-            : base(new DriverServiceCommandExecutor(service, commandTimeout), ConvertOptionsToCapabilities(options))
+            : base(GenerateDriverServiceCommandExecutor(service, options, commandTimeout), ConvertOptionsToCapabilities(options))
         {
             this.optionsCapabilityName = options.CapabilityName;
         }
 
+        /// <summary>
+        /// Uses DriverFinder to set Service attributes if necessary when creating the command executor
+        /// </summary>
+        /// <param name="service"></param>
+        /// <param name="commandTimeout"></param>
+        /// <param name="options"></param>
+        /// <returns></returns>
+        private static ICommandExecutor GenerateDriverServiceCommandExecutor(DriverService service, DriverOptions options, TimeSpan commandTimeout)
+        {
+            if (service.DriverServicePath == null) {
+                string fullServicePath = DriverFinder.FullPath(options);
+                service.DriverServicePath = Path.GetDirectoryName(fullServicePath);
+                service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);
+            }
+            return new DriverServiceCommandExecutor(service, commandTimeout);
+        }
+
         protected static IReadOnlyDictionary<string, CommandInfo> ChromiumCustomCommands
         {
             get { return new ReadOnlyDictionary<string, CommandInfo>(chromiumCustomCommands); }

dotnet/src/webdriver/DriverService.cs

@@ -23,7 +23,6 @@
 using System.Net;
 using System.Net.Http;
 using System.Threading.Tasks;
-using OpenQA.Selenium.Internal;
 using OpenQA.Selenium.Remote;
 
 namespace OpenQA.Selenium

dotnet/src/webdriver/Edge/EdgeDriver.cs

@@ -53,7 +53,7 @@ public EdgeDriver()
         /// </summary>
         /// <param name="options">The <see cref="EdgeOptions"/> to be used with the Edge driver.</param>
         public EdgeDriver(EdgeOptions options)
-            : this(EdgeDriverService.CreateDefaultService(options), options)
+            : this(EdgeDriverService.CreateDefaultService(), options)
         {
         }
 

dotnet/src/webdriver/Edge/EdgeDriverService.cs

@@ -16,6 +16,7 @@
 // limitations under the License.
 // </copyright>
 
+using System;
 using System.IO;
 using OpenQA.Selenium.Chromium;
 using OpenQA.Selenium.Internal;
@@ -55,14 +56,15 @@ public bool UseVerboseLogging
         /// <returns>A EdgeDriverService that implements default settings.</returns>
         public static EdgeDriverService CreateDefaultService()
         {
-            return CreateDefaultService(new EdgeOptions());
+            return new EdgeDriverService(null, null, PortUtilities.FindFreePort());
         }
 
         /// <summary>
         /// Creates a default instance of the EdgeDriverService.
         /// </summary>
         /// <param name="options">Browser options used to find the correct MSEdgeDriver binary.</param>
         /// <returns>A EdgeDriverService that implements default settings.</returns>
+        [Obsolete("CreateDefaultService() now evaluates options in Driver constructor")]
         public static EdgeDriverService CreateDefaultService(EdgeOptions options)
         {
             string fullServicePath = DriverFinder.FullPath(options);

dotnet/src/webdriver/Firefox/FirefoxDriver.cs

@@ -123,7 +123,7 @@ public FirefoxDriver()
         /// </summary>
         /// <param name="options">The <see cref="FirefoxOptions"/> to be used with the Firefox driver.</param>
         public FirefoxDriver(FirefoxOptions options)
-            : this(CreateService(options), options, RemoteWebDriver.DefaultCommandTimeout)
+            : this(FirefoxDriverService.CreateDefaultService(), options, RemoteWebDriver.DefaultCommandTimeout)
         {
         }
 
@@ -186,12 +186,29 @@ public FirefoxDriver(FirefoxDriverService service, FirefoxOptions options)
         /// <param name="options">The <see cref="FirefoxOptions"/> to be used with the Firefox driver.</param>
         /// <param name="commandTimeout">The maximum amount of time to wait for each command.</param>
         public FirefoxDriver(FirefoxDriverService service, FirefoxOptions options, TimeSpan commandTimeout)
-            : base(new DriverServiceCommandExecutor(service, commandTimeout), ConvertOptionsToCapabilities(options))
+            : base(GenerateDriverServiceCommandExecutor(service, options, commandTimeout), ConvertOptionsToCapabilities(options))
         {
             // Add the custom commands unique to Firefox
             this.AddCustomFirefoxCommands();
         }
 
+        /// <summary>
+        /// Uses DriverFinder to set Service attributes if necessary when creating the command executor
+        /// </summary>
+        /// <param name="service"></param>
+        /// <param name="commandTimeout"></param>
+        /// <param name="options"></param>
+        /// <returns></returns>
+        private static ICommandExecutor GenerateDriverServiceCommandExecutor(DriverService service, DriverOptions options, TimeSpan commandTimeout)
+        {
+            if (service.DriverServicePath == null) {
+                string fullServicePath = DriverFinder.FullPath(options);
+                service.DriverServicePath = Path.GetDirectoryName(fullServicePath);
+                service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);
+            }
+            return new DriverServiceCommandExecutor(service, commandTimeout);
+        }
+
         /// <summary>
         /// Gets a read-only dictionary of the custom WebDriver commands defined for FirefoxDriver.
         /// The keys of the dictionary are the names assigned to the command; the values are the
@@ -434,11 +451,6 @@ private static ICapabilities ConvertOptionsToCapabilities(FirefoxOptions options
             return options.ToCapabilities();
         }
 
-        private static FirefoxDriverService CreateService(FirefoxOptions options)
-        {
-            return FirefoxDriverService.CreateDefaultService(options);
-        }
-
         private void AddCustomFirefoxCommands()
         {
             foreach (KeyValuePair<string, CommandInfo> entry in CustomCommandDefinitions)

dotnet/src/webdriver/Firefox/FirefoxDriverService.cs

@@ -208,7 +208,7 @@ protected override string CommandLineArguments
         /// <returns>A FirefoxDriverService that implements default settings.</returns>
         public static FirefoxDriverService CreateDefaultService()
         {
-            return CreateDefaultService(new FirefoxOptions());
+            return new FirefoxDriverService(null, null, PortUtilities.FindFreePort());
         }
 
 
@@ -217,6 +217,7 @@ public static FirefoxDriverService CreateDefaultService()
         /// </summary>
         /// <param name="options">Browser options used to find the correct GeckoDriver binary.</param>
         /// <returns>A FirefoxDriverService that implements default settings.</returns>
+        [Obsolete("CreateDefaultService() now evaluates options in Driver constructor")]
         public static FirefoxDriverService CreateDefaultService(FirefoxOptions options)
         {
             string fullServicePath = DriverFinder.FullPath(options);

dotnet/src/webdriver/IE/InternetExplorerDriver.cs

@@ -17,6 +17,7 @@
 // </copyright>
 
 using System;
+using System.IO;
 using OpenQA.Selenium.Remote;
 
 namespace OpenQA.Selenium.IE
@@ -75,7 +76,7 @@ public InternetExplorerDriver()
         /// </summary>
         /// <param name="options">The <see cref="InternetExplorerOptions"/> used to initialize the driver.</param>
         public InternetExplorerDriver(InternetExplorerOptions options)
-            : this(InternetExplorerDriverService.CreateDefaultService(options), options)
+            : this(InternetExplorerDriverService.CreateDefaultService(), options)
         {
         }
 
@@ -140,10 +141,27 @@ public InternetExplorerDriver(InternetExplorerDriverService service, InternetExp
         /// <param name="options">The <see cref="InternetExplorerOptions"/> used to initialize the driver.</param>
         /// <param name="commandTimeout">The maximum amount of time to wait for each command.</param>
         public InternetExplorerDriver(InternetExplorerDriverService service, InternetExplorerOptions options, TimeSpan commandTimeout)
-            : base(new DriverServiceCommandExecutor(service, commandTimeout), ConvertOptionsToCapabilities(options))
+            : base(GenerateDriverServiceCommandExecutor(service, options, commandTimeout), ConvertOptionsToCapabilities(options))
         {
         }
 
+        /// <summary>
+        /// Uses DriverFinder to set Service attributes if necessary when creating the command executor
+        /// </summary>
+        /// <param name="service"></param>
+        /// <param name="commandTimeout"></param>
+        /// <param name="options"></param>
+        /// <returns></returns>
+        private static ICommandExecutor GenerateDriverServiceCommandExecutor(DriverService service, DriverOptions options, TimeSpan commandTimeout)
+        {
+            if (service.DriverServicePath == null) {
+                string fullServicePath = DriverFinder.FullPath(options);
+                service.DriverServicePath = Path.GetDirectoryName(fullServicePath);
+                service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);
+            }
+            return new DriverServiceCommandExecutor(service, commandTimeout);
+        }
+
         /// <summary>
         /// Gets or sets the <see cref="IFileDetector"/> responsible for detecting
         /// sequences of keystrokes representing file paths and names.

dotnet/src/webdriver/IE/InternetExplorerDriverService.cs

@@ -16,6 +16,7 @@
 // limitations under the License.
 // </copyright>
 
+using System;
 using System.Globalization;
 using System.IO;
 using System.Text;
@@ -148,14 +149,15 @@ protected override string CommandLineArguments
         /// <returns>A InternetExplorerDriverService that implements default settings.</returns>
         public static InternetExplorerDriverService CreateDefaultService()
         {
-            return CreateDefaultService(new InternetExplorerOptions());
+            return new InternetExplorerDriverService(null, null, PortUtilities.FindFreePort());
         }
 
         /// <summary>
         /// Creates a default instance of the InternetExplorerDriverService.
         /// </summary>
         /// <param name="options">Browser options used to find the correct IEDriver binary.</param>
         /// <returns>A InternetExplorerDriverService that implements default settings.</returns>
+        [Obsolete("CreateDefaultService() now evaluates options in Driver constructor")]
         public static InternetExplorerDriverService CreateDefaultService(InternetExplorerOptions options)
         {
             string fullServicePath = DriverFinder.FullPath(options);

dotnet/src/webdriver/Safari/SafariDriver.cs

@@ -18,6 +18,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.IO;
 using OpenQA.Selenium.Remote;
 
 namespace OpenQA.Selenium.Safari
@@ -79,7 +80,7 @@ public SafariDriver()
         /// </summary>
         /// <param name="options">The <see cref="SafariOptions"/> to use for this <see cref="SafariDriver"/> instance.</param>
         public SafariDriver(SafariOptions options)
-            : this(SafariDriverService.CreateDefaultService(options), options)
+            : this(SafariDriverService.CreateDefaultService(), options)
         {
         }
 
@@ -143,13 +144,30 @@ public SafariDriver(SafariDriverService service, SafariOptions options)
         /// <param name="options">The <see cref="SafariOptions"/> to be used with the Safari driver.</param>
         /// <param name="commandTimeout">The maximum amount of time to wait for each command.</param>
         public SafariDriver(SafariDriverService service, SafariOptions options, TimeSpan commandTimeout)
-            : base(new DriverServiceCommandExecutor(service, commandTimeout), ConvertOptionsToCapabilities(options))
+            : base(GenerateDriverServiceCommandExecutor(service, options, commandTimeout), ConvertOptionsToCapabilities(options))
         {
             this.AddCustomSafariCommand(AttachDebuggerCommand, HttpCommandInfo.PostCommand, "/session/{sessionId}/apple/attach_debugger");
             this.AddCustomSafariCommand(GetPermissionsCommand, HttpCommandInfo.GetCommand, "/session/{sessionId}/apple/permissions");
             this.AddCustomSafariCommand(SetPermissionsCommand, HttpCommandInfo.PostCommand, "/session/{sessionId}/apple/permissions");
         }
 
+        /// <summary>
+        /// Uses DriverFinder to set Service attributes if necessary when creating the command executor
+        /// </summary>
+        /// <param name="service"></param>
+        /// <param name="commandTimeout"></param>
+        /// <param name="options"></param>
+        /// <returns></returns>
+        private static ICommandExecutor GenerateDriverServiceCommandExecutor(DriverService service, DriverOptions options, TimeSpan commandTimeout)
+        {
+            if (service.DriverServicePath == null) {
+                string fullServicePath = DriverFinder.FullPath(options);
+                service.DriverServicePath = Path.GetDirectoryName(fullServicePath);
+                service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);
+            }
+            return new DriverServiceCommandExecutor(service, commandTimeout);
+        }
+
         /// <summary>
         /// This opens Safari's Web Inspector.
         /// If driver subsequently executes script of "debugger;"

dotnet/src/webdriver/Safari/SafariDriverService.cs

@@ -159,14 +159,15 @@ protected override bool IsInitialized
         /// <returns>A SafariDriverService that implements default settings.</returns>
         public static SafariDriverService CreateDefaultService()
         {
-            return CreateDefaultService(new SafariOptions());
+            return new SafariDriverService(null, null, PortUtilities.FindFreePort());
         }
 
         /// <summary>
         /// Creates a default instance of the SafariDriverService.
         /// </summary>
         /// <param name="options">Browser options used to find the correct GeckoDriver binary.</param>
         /// <returns>A SafariDriverService that implements default settings.</returns>
+        [Obsolete("CreateDefaultService() now evaluates options in Driver constructor")]
         public static SafariDriverService CreateDefaultService(SafariOptions options)
         {
             string fullServicePath = DriverFinder.FullPath(options);

dotnet/test/common/CustomDriverConfigs/DefaultSafariDriver.cs

@@ -5,20 +5,15 @@ namespace OpenQA.Selenium.Safari
     // constructor.
     public class DefaultSafariDriver : SafariDriver
     {
-        public DefaultSafariDriver()
-            : base(DefaultOptions)
-        {
-        }
-
         // Required for dynamic setting with `EnvironmentManager.Instance.CreateDriverInstance(options)`
         public DefaultSafariDriver(SafariOptions options)
             : base(options)
         {
         }
 
-        public static SafariOptions DefaultOptions
+        public DefaultSafariDriver(SafariDriverService service, SafariOptions options)
+            : base(service, options)
         {
-            get { return new SafariOptions(); }
         }
     }
 }

dotnet/test/common/CustomDriverConfigs/DevChannelChromeDriver.cs

@@ -13,6 +13,11 @@ public DevChannelChromeDriver(ChromeOptions options)
         {
         }
 
+        public DevChannelChromeDriver(ChromeDriverService service, ChromeOptions options)
+            : base(service, options)
+        {
+        }
+
         public static ChromeOptions DefaultOptions
         {
             get { return new ChromeOptions() { BrowserVersion = "dev" }; }

dotnet/test/common/CustomDriverConfigs/DevChannelEdgeDriver.cs

@@ -13,6 +13,11 @@ public DevChannelEdgeDriver(EdgeOptions options)
         {
         }
 
+        public DevChannelEdgeDriver(EdgeDriverService service, EdgeOptions options)
+            : base(service, options)
+        {
+        }
+
         public static EdgeOptions DefaultOptions
         {
             get { return new EdgeOptions() { BrowserVersion = "dev" }; }

dotnet/test/common/CustomDriverConfigs/EdgeInternetExplorerModeDriver.cs

@@ -16,6 +16,11 @@ public EdgeInternetExplorerModeDriver(InternetExplorerOptions options)
         {
         }
 
+        public EdgeInternetExplorerModeDriver(InternetExplorerDriverService service, InternetExplorerOptions options)
+            : base(service, options)
+        {
+        }
+
         public static InternetExplorerOptions DefaultOptions
         {
             get { return new InternetExplorerOptions() { RequireWindowFocus = true, UsePerProcessProxy = true, AttachToEdgeChrome = true }; }

dotnet/test/common/CustomDriverConfigs/NightlyChannelFirefoxDriver.cs

@@ -16,6 +16,11 @@ public NightlyChannelFirefoxDriver(FirefoxOptions options)
         {
         }
 
+        public NightlyChannelFirefoxDriver(FirefoxDriverService service, FirefoxOptions options)
+            : base(service, options)
+        {
+        }
+
         public static FirefoxOptions DefaultOptions
         {
             get { return new FirefoxOptions() { BrowserVersion = "nightly" }; }