Skip to content
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

[dotnet] Simplify and nullable annotate DriverFinder #15232

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
+64 −46
Open

[dotnet] Simplify and nullable annotate DriverFinder #15232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions dotnet/src/webdriver/Chromium/ChromiumDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,9 @@ private static ICommandExecutor GenerateDriverServiceCommandExecutor(DriverServi
string fullServicePath = finder.GetDriverPath();
service.DriverServicePath = Path.GetDirectoryName(fullServicePath);
service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);
if (finder.HasBrowserPath())
if (finder.TryGetBrowserPath(out string browserPath))
{
options.BinaryLocation = finder.GetBrowserPath();
options.BinaryLocation = browserPath;
options.BrowserVersion = null;
}
}
Expand Down
68 changes: 43 additions & 25 deletions dotnet/src/webdriver/DriverFinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@
// </copyright>

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 +33,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 SeleniumManagerPaths? paths;
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 @@ -52,7 +55,7 @@ public DriverFinder(DriverOptions options)
/// </returns>
public string GetBrowserPath()
{
return BinaryPaths()[BrowserPathKey];
return BinaryPaths().BrowserPath;
}

/// <summary>
Expand All @@ -63,47 +66,62 @@ public string GetBrowserPath()
/// </returns>
public string GetDriverPath()
{
return BinaryPaths()[DriverPathKey];
return BinaryPaths().DriverPath;
}

/// <summary>
/// Gets whether there is a browser path for the given browser on this platform.
/// </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>
/// <returns>
/// A Dictionary with the validated browser and driver path.
/// </returns>
/// <exception cref="NoSuchDriverException">If one of the paths does not exist.</exception>
private Dictionary<string, string> BinaryPaths()
private SeleniumManagerPaths BinaryPaths()
{
if (paths.ContainsKey(DriverPathKey) && !string.IsNullOrWhiteSpace(paths[DriverPathKey]))
if (paths is not null)
{
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
{
throw new NoSuchDriverException($"The driver path is not a valid file: {driverPath}");
}
if (File.Exists(browserPath))

SeleniumManagerPaths binaryPaths = SeleniumManager.BinaryPaths(CreateArguments());
if (!File.Exists(binaryPaths.DriverPath))
{
paths.Add(BrowserPathKey, browserPath);
throw new NoSuchDriverException($"The driver path is not a valid file: {binaryPaths.DriverPath}");
}
else

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

return paths = binaryPaths;
}

/// <summary>
Expand All @@ -123,7 +141,7 @@ private string CreateArguments()
argsBuilder.AppendFormat(CultureInfo.InvariantCulture, " --browser-version {0}", options.BrowserVersion);
}

string browserBinary = options.BinaryLocation;
string? browserBinary = options.BinaryLocation;
if (!string.IsNullOrEmpty(browserBinary))
{
argsBuilder.AppendFormat(CultureInfo.InvariantCulture, " --browser-path \"{0}\"", browserBinary);
Expand Down
4 changes: 2 additions & 2 deletions dotnet/src/webdriver/Firefox/FirefoxDriver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,9 @@ private static ICommandExecutor GenerateDriverServiceCommandExecutor(DriverServi
string fullServicePath = finder.GetDriverPath();
service.DriverServicePath = Path.GetDirectoryName(fullServicePath);
service.DriverServiceExecutableName = Path.GetFileName(fullServicePath);
if (finder.HasBrowserPath())
if (finder.TryGetBrowserPath(out string browserPath))
{
options.BinaryLocation = finder.GetBrowserPath();
options.BinaryLocation = browserPath;
options.BrowserVersion = null;
}
}
Expand Down
34 changes: 17 additions & 17 deletions dotnet/src/webdriver/SeleniumManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public static class SeleniumManager
/// <returns>
/// An array with two entries, one for the driver path, and another one for the browser path.
/// </returns>
public static Dictionary<string, string> BinaryPaths(string arguments)
public static SeleniumManagerPaths BinaryPaths(string arguments)
{
StringBuilder argsBuilder = new StringBuilder(arguments);
argsBuilder.Append(" --language-binding csharp");
Expand All @@ -91,19 +91,14 @@ public static Dictionary<string, string> BinaryPaths(string arguments)
}

var smCommandResult = RunCommand(_lazyBinaryFullPath.Value, argsBuilder.ToString());
Dictionary<string, string> binaryPaths = new()
{
{ "browser_path", smCommandResult.BrowserPath },
{ "driver_path", smCommandResult.DriverPath }
};

if (_logger.IsEnabled(LogEventLevel.Trace))
{
_logger.Trace($"Driver path: {binaryPaths["driver_path"]}");
_logger.Trace($"Browser path: {binaryPaths["browser_path"]}");
_logger.Trace($"Driver path: {smCommandResult.DriverPath}");
_logger.Trace($"Browser path: {smCommandResult.BrowserPath}");
}

return binaryPaths;
return smCommandResult;
}

/// <summary>
Expand All @@ -114,7 +109,7 @@ public static Dictionary<string, string> BinaryPaths(string arguments)
/// <returns>
/// the standard output of the execution.
/// </returns>
private static ResultResponse RunCommand(string fileName, string arguments)
private static SeleniumManagerPaths RunCommand(string fileName, string arguments)
{
Process process = new Process();
process.StartInfo.FileName = _lazyBinaryFullPath.Value;
Expand Down Expand Up @@ -208,18 +203,23 @@ private static ResultResponse RunCommand(string fileName, string arguments)
}
}

internal sealed record SeleniumManagerResponse(IReadOnlyList<LogEntryResponse> Logs, ResultResponse Result)
internal sealed record SeleniumManagerResponse(IReadOnlyList<LogEntryResponse> Logs, SeleniumManagerPaths Result)
{
public sealed record LogEntryResponse(string Level, string Message);
}

public sealed record ResultResponse
(
[property: JsonPropertyName("driver_path")]
/// <summary>
/// Response from Selenium Manager about the paths and their locations.
/// </summary>
/// <param name="DriverPath">The path to the driver binary.</param>
/// <param name="BrowserPath">The path to the browser binary, or <see cref="string.Empty"/> if none exists.</param>
public sealed record SeleniumManagerPaths
(
[property: JsonPropertyName("driver_path")]
string DriverPath,
[property: JsonPropertyName("browser_path")]
[property: JsonPropertyName("browser_path")]
string BrowserPath
);
}
);

[JsonSerializable(typeof(SeleniumManagerResponse))]
[JsonSourceGenerationOptions(PropertyNameCaseInsensitive = true)]
Expand Down
Loading