Skip to content

Commit e88bf72

Browse files
titusfortnertitusfortner
authored
[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
1 parent f3d7062 commit e88bf72

26 files changed

+180
-57
lines changed

‎dotnet/src/webdriver/Chrome/ChromeDriver.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public ChromeDriver()
8383
/// </summary>
8484
/// <param name="options">The <see cref="ChromeOptions"/> to be used with the Chrome driver.</param>
8585
public ChromeDriver(ChromeOptions options)
86-
: this(ChromeDriverService.CreateDefaultService(options), options, RemoteWebDriver.DefaultCommandTimeout)
86+
: this(ChromeDriverService.CreateDefaultService(), options, RemoteWebDriver.DefaultCommandTimeout)
8787
{
8888
}
8989

‎dotnet/src/webdriver/Chrome/ChromeDriverService.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
// limitations under the License.
1717
// </copyright>
1818

19+
using System;
1920
using System.IO;
2021
using OpenQA.Selenium.Chromium;
2122
using OpenQA.Selenium.Internal;
@@ -46,14 +47,15 @@ private ChromeDriverService(string executablePath, string executableFileName, in
4647
/// <returns>A ChromeDriverService that implements default settings.</returns>
4748
public static ChromeDriverService CreateDefaultService()
4849
{
49-
return CreateDefaultService(new ChromeOptions());
50+
return new ChromeDriverService(null, null, PortUtilities.FindFreePort());
5051
}
5152

5253
/// <summary>
5354
/// Creates a default instance of the ChromeDriverService.
5455
/// </summary>
5556
/// /// <param name="options">Browser options used to find the correct ChromeDriver binary.</param>
5657
/// <returns>A ChromeDriverService that implements default settings.</returns>
58+
[Obsolete("CreateDefaultService() now evaluates options in Driver constructor")]
5759
public static ChromeDriverService CreateDefaultService(ChromeOptions options)
5860
{
5961
string fullServicePath = DriverFinder.FullPath(options);

‎dotnet/src/webdriver/Chromium/ChromiumDriver.cs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
using System;
2020
using System.Collections.Generic;
2121
using System.Collections.ObjectModel;
22+
using System.IO;
2223
using OpenQA.Selenium.DevTools;
2324
using OpenQA.Selenium.Remote;
2425

@@ -125,11 +126,28 @@ public class ChromiumDriver : WebDriver, ISupportsLogs, IDevTools
125126
/// <param name="options">The <see cref="ChromiumOptions"/> to be used with the ChromiumDriver.</param>
126127
/// <param name="commandTimeout">The maximum amount of time to wait for each command.</param>
127128
protected ChromiumDriver(ChromiumDriverService service, ChromiumOptions options, TimeSpan commandTimeout)
128-
: base(new DriverServiceCommandExecutor(service, commandTimeout), ConvertOptionsToCapabilities(options))
129+
: base(GenerateDriverServiceCommandExecutor(service, options, commandTimeout), ConvertOptionsToCapabilities(options))
129130
{
130131
this.optionsCapabilityName = options.CapabilityName;
131132
}
132133

134+
/// <summary>
135+
/// Uses DriverFinder to set Service attributes if necessary when creating the command executor
136+
/// </summary>
137+
/// <param name="service"></param>
138+
/// <param name="commandTimeout"></param>
139+
/// <param name="options"></param>
140+
/// <returns></returns>
141+
private static ICommandExecutor GenerateDriverServiceCommandExecutor(DriverService service, DriverOptions options, TimeSpan commandTimeout)
142+
{
143+
if (service.DriverServicePath == null) {
144+
string fullServicePath = DriverFinder.FullPath(options);
145+
service.DriverServicePath = Path.GetDirectoryName(fullServicePath);
146+
service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);
147+
}
148+
return new DriverServiceCommandExecutor(service, commandTimeout);
149+
}
150+
133151
protected static IReadOnlyDictionary<string, CommandInfo> ChromiumCustomCommands
134152
{
135153
get { return new ReadOnlyDictionary<string, CommandInfo>(chromiumCustomCommands); }

‎dotnet/src/webdriver/DriverService.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
using System.Net;
2424
using System.Net.Http;
2525
using System.Threading.Tasks;
26-
using OpenQA.Selenium.Internal;
2726
using OpenQA.Selenium.Remote;
2827

2928
namespace OpenQA.Selenium

‎dotnet/src/webdriver/Edge/EdgeDriver.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public EdgeDriver()
5353
/// </summary>
5454
/// <param name="options">The <see cref="EdgeOptions"/> to be used with the Edge driver.</param>
5555
public EdgeDriver(EdgeOptions options)
56-
: this(EdgeDriverService.CreateDefaultService(options), options)
56+
: this(EdgeDriverService.CreateDefaultService(), options)
5757
{
5858
}
5959

‎dotnet/src/webdriver/Edge/EdgeDriverService.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
// limitations under the License.
1717
// </copyright>
1818

19+
using System;
1920
using System.IO;
2021
using OpenQA.Selenium.Chromium;
2122
using OpenQA.Selenium.Internal;
@@ -55,14 +56,15 @@ public bool UseVerboseLogging
5556
/// <returns>A EdgeDriverService that implements default settings.</returns>
5657
public static EdgeDriverService CreateDefaultService()
5758
{
58-
return CreateDefaultService(new EdgeOptions());
59+
return new EdgeDriverService(null, null, PortUtilities.FindFreePort());
5960
}
6061

6162
/// <summary>
6263
/// Creates a default instance of the EdgeDriverService.
6364
/// </summary>
6465
/// <param name="options">Browser options used to find the correct MSEdgeDriver binary.</param>
6566
/// <returns>A EdgeDriverService that implements default settings.</returns>
67+
[Obsolete("CreateDefaultService() now evaluates options in Driver constructor")]
6668
public static EdgeDriverService CreateDefaultService(EdgeOptions options)
6769
{
6870
string fullServicePath = DriverFinder.FullPath(options);

‎dotnet/src/webdriver/Firefox/FirefoxDriver.cs

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ public FirefoxDriver()
123123
/// </summary>
124124
/// <param name="options">The <see cref="FirefoxOptions"/> to be used with the Firefox driver.</param>
125125
public FirefoxDriver(FirefoxOptions options)
126-
: this(CreateService(options), options, RemoteWebDriver.DefaultCommandTimeout)
126+
: this(FirefoxDriverService.CreateDefaultService(), options, RemoteWebDriver.DefaultCommandTimeout)
127127
{
128128
}
129129

@@ -186,12 +186,29 @@ public FirefoxDriver(FirefoxDriverService service, FirefoxOptions options)
186186
/// <param name="options">The <see cref="FirefoxOptions"/> to be used with the Firefox driver.</param>
187187
/// <param name="commandTimeout">The maximum amount of time to wait for each command.</param>
188188
public FirefoxDriver(FirefoxDriverService service, FirefoxOptions options, TimeSpan commandTimeout)
189-
: base(new DriverServiceCommandExecutor(service, commandTimeout), ConvertOptionsToCapabilities(options))
189+
: base(GenerateDriverServiceCommandExecutor(service, options, commandTimeout), ConvertOptionsToCapabilities(options))
190190
{
191191
// Add the custom commands unique to Firefox
192192
this.AddCustomFirefoxCommands();
193193
}
194194

195+
/// <summary>
196+
/// Uses DriverFinder to set Service attributes if necessary when creating the command executor
197+
/// </summary>
198+
/// <param name="service"></param>
199+
/// <param name="commandTimeout"></param>
200+
/// <param name="options"></param>
201+
/// <returns></returns>
202+
private static ICommandExecutor GenerateDriverServiceCommandExecutor(DriverService service, DriverOptions options, TimeSpan commandTimeout)
203+
{
204+
if (service.DriverServicePath == null) {
205+
string fullServicePath = DriverFinder.FullPath(options);
206+
service.DriverServicePath = Path.GetDirectoryName(fullServicePath);
207+
service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);
208+
}
209+
return new DriverServiceCommandExecutor(service, commandTimeout);
210+
}
211+
195212
/// <summary>
196213
/// Gets a read-only dictionary of the custom WebDriver commands defined for FirefoxDriver.
197214
/// 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
434451
return options.ToCapabilities();
435452
}
436453

437-
private static FirefoxDriverService CreateService(FirefoxOptions options)
438-
{
439-
return FirefoxDriverService.CreateDefaultService(options);
440-
}
441-
442454
private void AddCustomFirefoxCommands()
443455
{
444456
foreach (KeyValuePair<string, CommandInfo> entry in CustomCommandDefinitions)

‎dotnet/src/webdriver/Firefox/FirefoxDriverService.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ protected override string CommandLineArguments
208208
/// <returns>A FirefoxDriverService that implements default settings.</returns>
209209
public static FirefoxDriverService CreateDefaultService()
210210
{
211-
return CreateDefaultService(new FirefoxOptions());
211+
return new FirefoxDriverService(null, null, PortUtilities.FindFreePort());
212212
}
213213

214214

@@ -217,6 +217,7 @@ public static FirefoxDriverService CreateDefaultService()
217217
/// </summary>
218218
/// <param name="options">Browser options used to find the correct GeckoDriver binary.</param>
219219
/// <returns>A FirefoxDriverService that implements default settings.</returns>
220+
[Obsolete("CreateDefaultService() now evaluates options in Driver constructor")]
220221
public static FirefoxDriverService CreateDefaultService(FirefoxOptions options)
221222
{
222223
string fullServicePath = DriverFinder.FullPath(options);

‎dotnet/src/webdriver/IE/InternetExplorerDriver.cs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
// </copyright>
1818

1919
using System;
20+
using System.IO;
2021
using OpenQA.Selenium.Remote;
2122

2223
namespace OpenQA.Selenium.IE
@@ -75,7 +76,7 @@ public InternetExplorerDriver()
7576
/// </summary>
7677
/// <param name="options">The <see cref="InternetExplorerOptions"/> used to initialize the driver.</param>
7778
public InternetExplorerDriver(InternetExplorerOptions options)
78-
: this(InternetExplorerDriverService.CreateDefaultService(options), options)
79+
: this(InternetExplorerDriverService.CreateDefaultService(), options)
7980
{
8081
}
8182

@@ -140,10 +141,27 @@ public InternetExplorerDriver(InternetExplorerDriverService service, InternetExp
140141
/// <param name="options">The <see cref="InternetExplorerOptions"/> used to initialize the driver.</param>
141142
/// <param name="commandTimeout">The maximum amount of time to wait for each command.</param>
142143
public InternetExplorerDriver(InternetExplorerDriverService service, InternetExplorerOptions options, TimeSpan commandTimeout)
143-
: base(new DriverServiceCommandExecutor(service, commandTimeout), ConvertOptionsToCapabilities(options))
144+
: base(GenerateDriverServiceCommandExecutor(service, options, commandTimeout), ConvertOptionsToCapabilities(options))
144145
{
145146
}
146147

148+
/// <summary>
149+
/// Uses DriverFinder to set Service attributes if necessary when creating the command executor
150+
/// </summary>
151+
/// <param name="service"></param>
152+
/// <param name="commandTimeout"></param>
153+
/// <param name="options"></param>
154+
/// <returns></returns>
155+
private static ICommandExecutor GenerateDriverServiceCommandExecutor(DriverService service, DriverOptions options, TimeSpan commandTimeout)
156+
{
157+
if (service.DriverServicePath == null) {
158+
string fullServicePath = DriverFinder.FullPath(options);
159+
service.DriverServicePath = Path.GetDirectoryName(fullServicePath);
160+
service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);
161+
}
162+
return new DriverServiceCommandExecutor(service, commandTimeout);
163+
}
164+
147165
/// <summary>
148166
/// Gets or sets the <see cref="IFileDetector"/> responsible for detecting
149167
/// sequences of keystrokes representing file paths and names.

‎dotnet/src/webdriver/IE/InternetExplorerDriverService.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
// limitations under the License.
1717
// </copyright>
1818

19+
using System;
1920
using System.Globalization;
2021
using System.IO;
2122
using System.Text;
@@ -148,14 +149,15 @@ protected override string CommandLineArguments
148149
/// <returns>A InternetExplorerDriverService that implements default settings.</returns>
149150
public static InternetExplorerDriverService CreateDefaultService()
150151
{
151-
return CreateDefaultService(new InternetExplorerOptions());
152+
return new InternetExplorerDriverService(null, null, PortUtilities.FindFreePort());
152153
}
153154

154155
/// <summary>
155156
/// Creates a default instance of the InternetExplorerDriverService.
156157
/// </summary>
157158
/// <param name="options">Browser options used to find the correct IEDriver binary.</param>
158159
/// <returns>A InternetExplorerDriverService that implements default settings.</returns>
160+
[Obsolete("CreateDefaultService() now evaluates options in Driver constructor")]
159161
public static InternetExplorerDriverService CreateDefaultService(InternetExplorerOptions options)
160162
{
161163
string fullServicePath = DriverFinder.FullPath(options);

0 commit comments

Comments
 (0)