Skip to content

[dotnet] Simplify DriverService and DriverFinder #15028

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[dotnet] Simplify DriverService and DriverFinder #15028

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
145fbca
[dotnet] Simplify `DriverService`
RenderMichael Jan 1, 2025
ffac6d8
Merge branch 'trunk' into driver-service
RenderMichael Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 1 addition & 25 deletions dotnet/src/webdriver/Chromium/ChromiumDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ 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(GenerateDriverServiceCommandExecutor(service, options, commandTimeout), ConvertOptionsToCapabilities(options))
: base(service.CreateCommandExecutor(options, commandTimeout, searchForBrowserPath: true), ConvertOptionsToCapabilities(options))
{
this.optionsCapabilityName = options.CapabilityName;
}
Expand All @@ -141,30 +141,6 @@ protected static IReadOnlyDictionary<string, CommandInfo> ChromiumCustomCommands
get { return new ReadOnlyDictionary<string, CommandInfo>(chromiumCustomCommands); }
}

/// <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)
{
DriverFinder finder = new DriverFinder(options);
string fullServicePath = finder.GetDriverPath();
service.DriverServicePath = Path.GetDirectoryName(fullServicePath);
service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);
if (finder.HasBrowserPath())
{
options.BinaryLocation = finder.GetBrowserPath();
options.BrowserVersion = null;
}
}
return new DriverServiceCommandExecutor(service, commandTimeout);
}

/// <summary>
/// Gets or sets the <see cref="IFileDetector"/> responsible for detecting
/// sequences of keystrokes representing file paths and names.
Expand Down
52 changes: 38 additions & 14 deletions dotnet/src/webdriver/DriverFinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.IO;
using System.Text;

#nullable enable

namespace OpenQA.Selenium
{
/// <summary>
Expand All @@ -31,17 +34,18 @@ namespace OpenQA.Selenium
/// </summary>
public class DriverFinder
{
private DriverOptions options;
private Dictionary<string, string> paths = new Dictionary<string, string>();
private readonly DriverOptions options;
private readonly Dictionary<string, string> paths = new Dictionary<string, string>();
private const string BrowserPathKey = "browser_path";
private const string DriverPathKey = "driver_path";

/// <summary>
/// Initializes a new instance of the <see cref="DriverFinder"/> class.
/// </summary>
/// <exception cref="ArgumentNullException">If <paramref name="options"/> is <see langword="null"/>.</exception>
public DriverFinder(DriverOptions options)
{
this.options = options;
this.options = options ?? throw new ArgumentNullException(nameof(options));
}

/// <summary>
Expand All @@ -66,11 +70,33 @@ public string GetDriverPath()
return BinaryPaths()[DriverPathKey];
}

/// <summary>
/// Gets a value indicating whether a browser path exists, as retrieved by the Selenium Manager.
/// </summary>
/// <returns><see langword="true"/> if a browser path exists; otherwise, <see langword="false"/>.</returns>
public bool HasBrowserPath()
{
return !string.IsNullOrWhiteSpace(GetBrowserPath());
}

/// <summary>
/// Tries to get the browser path, as retrieved by Selenium Manager.
/// </summary>
/// <param name="browserPath">If the method returns <see langword="true"/>, the full browser path.</param>
/// <returns><see langword="true"/> if a browser path exists; otherwise, <see langword="false"/>.</returns>
public bool TryGetBrowserPath([NotNullWhen(true)] out string? browserPath)
{
string? path = GetBrowserPath();
if (!string.IsNullOrWhiteSpace(path))
{
browserPath = path;
return true;
}

browserPath = null;
return false;
}

/// <summary>
/// Invokes Selenium Manager to get the binaries paths and validates if they exist.
/// </summary>
Expand All @@ -80,29 +106,27 @@ public bool HasBrowserPath()
/// <exception cref="NoSuchDriverException">If one of the paths does not exist.</exception>
private Dictionary<string, string> BinaryPaths()
{
if (paths.ContainsKey(DriverPathKey) && !string.IsNullOrWhiteSpace(paths[DriverPathKey]))
if (paths.TryGetValue(DriverPathKey, out string? cachedDriverPath)
&& !string.IsNullOrWhiteSpace(cachedDriverPath))
{
return paths;
}

Dictionary<string, string> binaryPaths = SeleniumManager.BinaryPaths(CreateArguments());
string driverPath = binaryPaths[DriverPathKey];
string browserPath = binaryPaths[BrowserPathKey];
if (File.Exists(driverPath))
{
paths.Add(DriverPathKey, driverPath);
}
else
if (!File.Exists(driverPath))
{
throw new NoSuchDriverException($"The driver path is not a valid file: {driverPath}");
}
if (File.Exists(browserPath))
{
paths.Add(BrowserPathKey, browserPath);
}
else

if (!File.Exists(browserPath))
{
throw new NoSuchDriverException($"The browser path is not a valid file: {browserPath}");
}

paths.Add(DriverPathKey, driverPath);
paths.Add(BrowserPathKey, browserPath);
Copy link
Contributor Author

@RenderMichael RenderMichael Jan 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best to add both after exceptions, because paths is a shared state which can be corrupted if used wrong.

return paths;
}

Expand Down
24 changes: 24 additions & 0 deletions dotnet/src/webdriver/DriverService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,5 +428,29 @@ private bool WaitForServiceInitialization()

return isInitialized;
}

/// <summary>
/// Uses DriverFinder to set Service attributes if necessary when creating the command executor
/// </summary>
/// <param name="options">The <see cref="DriverOptions"/> to be used with the driver.</param>
/// <param name="commandTimeout">The maximum amount of time to wait for each command.</param>
/// <param name="searchForBrowserPath">Whether to search for browser binaries.</param>
/// <returns>A command executor.</returns>
internal DriverServiceCommandExecutor CreateCommandExecutor(DriverOptions options, TimeSpan commandTimeout, bool searchForBrowserPath)
{
if (DriverServicePath == null)
{
DriverFinder finder = new DriverFinder(options);
string fullServicePath = finder.GetDriverPath();
DriverServicePath = Path.GetDirectoryName(fullServicePath);
DriverServiceExecutableName = Path.GetFileName(fullServicePath);
if (searchForBrowserPath && finder.TryGetBrowserPath(out string browserPath))
{
options.BinaryLocation = browserPath;
Copy link
Member

@nvborisenko nvborisenko Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting binary location for IE is not supported (at runtime).

Copy link
Contributor Author

@RenderMichael RenderMichael Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the bool searchForBrowserPath set to false on IE and Safari, so this should not be a problem. https://github.com/SeleniumHQ/selenium/pull/15028/files#diff-628f59035c50241ea1a690b5e923fccb569486eea8d138a6a1c08b9bbf78258dR145

options.BrowserVersion = null;
}
}
return new DriverServiceCommandExecutor(this, commandTimeout);
}
}
}
26 changes: 1 addition & 25 deletions dotnet/src/webdriver/Firefox/FirefoxDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,36 +188,12 @@ 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(GenerateDriverServiceCommandExecutor(service, options, commandTimeout), ConvertOptionsToCapabilities(options))
: base(service.CreateCommandExecutor(options, commandTimeout, searchForBrowserPath: true), 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)
{
DriverFinder finder = new DriverFinder(options);
string fullServicePath = finder.GetDriverPath();
service.DriverServicePath = Path.GetDirectoryName(fullServicePath);
service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);
if (finder.HasBrowserPath())
{
options.BinaryLocation = finder.GetBrowserPath();
options.BrowserVersion = null;
}
}
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
Expand Down
21 changes: 1 addition & 20 deletions dotnet/src/webdriver/IE/InternetExplorerDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,29 +142,10 @@ 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(GenerateDriverServiceCommandExecutor(service, options, commandTimeout), ConvertOptionsToCapabilities(options))
: base(service.CreateCommandExecutor(options, commandTimeout, searchForBrowserPath: false), 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)
{
DriverFinder finder = new DriverFinder(options);
string fullServicePath = finder.GetDriverPath();
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.
Expand Down
21 changes: 1 addition & 20 deletions dotnet/src/webdriver/Safari/SafariDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,32 +145,13 @@ 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(GenerateDriverServiceCommandExecutor(service, options, commandTimeout), ConvertOptionsToCapabilities(options))
: base(service.CreateCommandExecutor(options, commandTimeout, searchForBrowserPath: false), 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)
{
DriverFinder finder = new DriverFinder(options);
string fullServicePath = finder.GetDriverPath();
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;"
Expand Down