From d274da7a79eaed4a67cca131ab88a7013ec49e0c Mon Sep 17 00:00:00 2001 From: Michael Render Date: Wed, 23 Oct 2024 22:58:20 -0400 Subject: [PATCH 1/8] [dotnet] Lazy-load Selenium manager binary location --- dotnet/src/webdriver/SeleniumManager.cs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/dotnet/src/webdriver/SeleniumManager.cs b/dotnet/src/webdriver/SeleniumManager.cs index d57b13f278180..2465d1afb002c 100644 --- a/dotnet/src/webdriver/SeleniumManager.cs +++ b/dotnet/src/webdriver/SeleniumManager.cs @@ -36,27 +36,28 @@ public static class SeleniumManager { private static readonly ILogger _logger = Log.GetLogger(typeof(SeleniumManager)); - private static readonly string BinaryFullPath = Environment.GetEnvironmentVariable("SE_MANAGER_PATH"); + private static string? _binaryFullPath; + private static string BinaryFullPath => _binaryFullPath ??= GetBinaryFullPath(); private static readonly JsonSerializerOptions _serializerOptions = new() { PropertyNameCaseInsensitive = true, TypeInfoResolver = SeleniumManagerSerializerContext.Default }; - static SeleniumManager() + private static string GetBinaryFullPath() { - - if (BinaryFullPath == null) + string? binaryFullPath = Environment.GetEnvironmentVariable("SE_MANAGER_PATH"); + if (binaryFullPath == null) { var currentDirectory = AppContext.BaseDirectory; if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - BinaryFullPath = Path.Combine(currentDirectory, "selenium-manager", "windows", "selenium-manager.exe"); + binaryFullPath = Path.Combine(currentDirectory, "selenium-manager", "windows", "selenium-manager.exe"); } else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) { - BinaryFullPath = Path.Combine(currentDirectory, "selenium-manager", "linux", "selenium-manager"); + binaryFullPath = Path.Combine(currentDirectory, "selenium-manager", "linux", "selenium-manager"); } else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) { - BinaryFullPath = Path.Combine(currentDirectory, "selenium-manager", "macos", "selenium-manager"); + binaryFullPath = Path.Combine(currentDirectory, "selenium-manager", "macos", "selenium-manager"); } else { @@ -65,10 +66,11 @@ static SeleniumManager() } } - if (!File.Exists(BinaryFullPath)) + if (!File.Exists(binaryFullPath)) { - throw new WebDriverException($"Unable to locate or obtain Selenium Manager binary at {BinaryFullPath}"); + throw new WebDriverException($"Unable to locate or obtain Selenium Manager binary at {binaryFullPath}"); } + return binaryFullPath; } /// From f056d1544ef9777c805f9e0abda4ca818286f72c Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 24 Oct 2024 11:48:17 -0400 Subject: [PATCH 2/8] PR suggestions --- dotnet/src/webdriver/SeleniumManager.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/dotnet/src/webdriver/SeleniumManager.cs b/dotnet/src/webdriver/SeleniumManager.cs index 2465d1afb002c..34d993edae0db 100644 --- a/dotnet/src/webdriver/SeleniumManager.cs +++ b/dotnet/src/webdriver/SeleniumManager.cs @@ -36,12 +36,9 @@ public static class SeleniumManager { private static readonly ILogger _logger = Log.GetLogger(typeof(SeleniumManager)); - private static string? _binaryFullPath; - private static string BinaryFullPath => _binaryFullPath ??= GetBinaryFullPath(); - private static readonly JsonSerializerOptions _serializerOptions = new() { PropertyNameCaseInsensitive = true, TypeInfoResolver = SeleniumManagerSerializerContext.Default }; - private static string GetBinaryFullPath() + private static readonly Lazy _lazyBinaryFullPath = new(() => { string? binaryFullPath = Environment.GetEnvironmentVariable("SE_MANAGER_PATH"); if (binaryFullPath == null) @@ -71,7 +68,7 @@ private static string GetBinaryFullPath() throw new WebDriverException($"Unable to locate or obtain Selenium Manager binary at {binaryFullPath}"); } return binaryFullPath; - } + }); /// /// Determines the location of the browser and driver binaries. @@ -90,7 +87,7 @@ public static Dictionary BinaryPaths(string arguments) argsBuilder.Append(" --debug"); } - var smCommandResult = RunCommand(BinaryFullPath, argsBuilder.ToString()); + var smCommandResult = RunCommand(_lazyBinaryFullPath.Value, argsBuilder.ToString()); Dictionary binaryPaths = new() { { "browser_path", smCommandResult.BrowserPath }, @@ -117,7 +114,7 @@ public static Dictionary BinaryPaths(string arguments) private static SeleniumManagerResponse.ResultResponse RunCommand(string fileName, string arguments) { Process process = new Process(); - process.StartInfo.FileName = BinaryFullPath; + process.StartInfo.FileName = _lazyBinaryFullPath.Value; process.StartInfo.Arguments = arguments; process.StartInfo.UseShellExecute = false; process.StartInfo.CreateNoWindow = true; From bbf234d5750587baa32aaef842e7b56cc01689bd Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 24 Oct 2024 12:57:39 -0400 Subject: [PATCH 3/8] Enable NRT for `SeleniumManager` --- dotnet/src/webdriver/SeleniumManager.cs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/dotnet/src/webdriver/SeleniumManager.cs b/dotnet/src/webdriver/SeleniumManager.cs index 34d993edae0db..b5b8183bb9874 100644 --- a/dotnet/src/webdriver/SeleniumManager.cs +++ b/dotnet/src/webdriver/SeleniumManager.cs @@ -26,6 +26,8 @@ using System.Text.Json; using System.Text.Json.Serialization; +#nullable enable + namespace OpenQA.Selenium { /// @@ -77,7 +79,7 @@ public static class SeleniumManager /// /// An array with two entries, one for the driver path, and another one for the browser path. /// - public static Dictionary BinaryPaths(string arguments) + public static Dictionary BinaryPaths(string arguments) { StringBuilder argsBuilder = new StringBuilder(arguments); argsBuilder.Append(" --language-binding csharp"); @@ -88,7 +90,7 @@ public static Dictionary BinaryPaths(string arguments) } var smCommandResult = RunCommand(_lazyBinaryFullPath.Value, argsBuilder.ToString()); - Dictionary binaryPaths = new() + Dictionary binaryPaths = new() { { "browser_path", smCommandResult.BrowserPath }, { "driver_path", smCommandResult.DriverPath } @@ -182,7 +184,8 @@ private static SeleniumManagerResponse.ResultResponse RunCommand(string fileName try { - jsonResponse = JsonSerializer.Deserialize(output, _serializerOptions); + jsonResponse = JsonSerializer.Deserialize(output, _serializerOptions) + ?? throw new WebDriverException("SeleniumManagerResponse was null"); } catch (Exception ex) { @@ -217,36 +220,35 @@ private static SeleniumManagerResponse.ResultResponse RunCommand(string fileName } } - return jsonResponse.Result; + return jsonResponse.Result ?? throw new WebDriverException("Selenium manager response's Result was null"); } } internal class SeleniumManagerResponse { - public IReadOnlyList Logs { get; set; } + public IReadOnlyList? Logs { get; set; } - public ResultResponse Result { get; set; } + public ResultResponse? Result { get; set; } public class LogEntryResponse { - public string Level { get; set; } + public string? Level { get; set; } - public string Message { get; set; } + public string? Message { get; set; } } public class ResultResponse { [JsonPropertyName("driver_path")] - public string DriverPath { get; set; } + public string? DriverPath { get; set; } [JsonPropertyName("browser_path")] - public string BrowserPath { get; set; } + public string? BrowserPath { get; set; } } } [JsonSerializable(typeof(SeleniumManagerResponse))] internal partial class SeleniumManagerSerializerContext : JsonSerializerContext { - } } From dd1af5ef16781022ee5cd3237cf3da12fdcbfd44 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 24 Oct 2024 13:03:49 -0400 Subject: [PATCH 4/8] fix last hypothetical null reference exception in selenium manager --- dotnet/src/webdriver/SeleniumManager.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dotnet/src/webdriver/SeleniumManager.cs b/dotnet/src/webdriver/SeleniumManager.cs index b5b8183bb9874..64dc9210ce97a 100644 --- a/dotnet/src/webdriver/SeleniumManager.cs +++ b/dotnet/src/webdriver/SeleniumManager.cs @@ -196,7 +196,7 @@ private static SeleniumManagerResponse.ResultResponse RunCommand(string fileName { foreach (var entry in jsonResponse.Logs) { - switch (entry.Level) + switch (entry?.Level) { case "WARN": if (_logger.IsEnabled(LogEventLevel.Warn)) @@ -226,7 +226,7 @@ private static SeleniumManagerResponse.ResultResponse RunCommand(string fileName internal class SeleniumManagerResponse { - public IReadOnlyList? Logs { get; set; } + public IReadOnlyList? Logs { get; set; } public ResultResponse? Result { get; set; } From ba6d5b75d202e5f99d70f98a669282150733cbc4 Mon Sep 17 00:00:00 2001 From: Nikolay Borisenko <22616990+nvborisenko@users.noreply.github.com> Date: Thu, 24 Oct 2024 20:21:53 +0300 Subject: [PATCH 5/8] Do not use nullable json properties --- dotnet/src/webdriver/SeleniumManager.cs | 38 +++++++++---------------- 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/dotnet/src/webdriver/SeleniumManager.cs b/dotnet/src/webdriver/SeleniumManager.cs index 64dc9210ce97a..5ba3205efb3ef 100644 --- a/dotnet/src/webdriver/SeleniumManager.cs +++ b/dotnet/src/webdriver/SeleniumManager.cs @@ -25,6 +25,7 @@ using System.Text; using System.Text.Json; using System.Text.Json.Serialization; +using static OpenQA.Selenium.SeleniumManagerResponse; #nullable enable @@ -69,6 +70,7 @@ public static class SeleniumManager { throw new WebDriverException($"Unable to locate or obtain Selenium Manager binary at {binaryFullPath}"); } + return binaryFullPath; }); @@ -79,7 +81,7 @@ public static class SeleniumManager /// /// An array with two entries, one for the driver path, and another one for the browser path. /// - public static Dictionary BinaryPaths(string arguments) + public static Dictionary BinaryPaths(string arguments) { StringBuilder argsBuilder = new StringBuilder(arguments); argsBuilder.Append(" --language-binding csharp"); @@ -90,7 +92,7 @@ public static class SeleniumManager } var smCommandResult = RunCommand(_lazyBinaryFullPath.Value, argsBuilder.ToString()); - Dictionary binaryPaths = new() + Dictionary binaryPaths = new() { { "browser_path", smCommandResult.BrowserPath }, { "driver_path", smCommandResult.DriverPath } @@ -113,7 +115,7 @@ public static class SeleniumManager /// /// the standard output of the execution. /// - private static SeleniumManagerResponse.ResultResponse RunCommand(string fileName, string arguments) + private static ResultResponse RunCommand(string fileName, string arguments) { Process process = new Process(); process.StartInfo.FileName = _lazyBinaryFullPath.Value; @@ -184,8 +186,7 @@ private static SeleniumManagerResponse.ResultResponse RunCommand(string fileName try { - jsonResponse = JsonSerializer.Deserialize(output, _serializerOptions) - ?? throw new WebDriverException("SeleniumManagerResponse was null"); + jsonResponse = JsonSerializer.Deserialize(output, _serializerOptions)!; } catch (Exception ex) { @@ -196,7 +197,7 @@ private static SeleniumManagerResponse.ResultResponse RunCommand(string fileName { foreach (var entry in jsonResponse.Logs) { - switch (entry?.Level) + switch (entry.Level) { case "WARN": if (_logger.IsEnabled(LogEventLevel.Warn)) @@ -220,35 +221,24 @@ private static SeleniumManagerResponse.ResultResponse RunCommand(string fileName } } - return jsonResponse.Result ?? throw new WebDriverException("Selenium manager response's Result was null"); + return jsonResponse.Result; } } - internal class SeleniumManagerResponse + internal record SeleniumManagerResponse(IReadOnlyList Logs, ResultResponse Result) { - public IReadOnlyList? Logs { get; set; } - - public ResultResponse? Result { get; set; } - - public class LogEntryResponse - { - public string? Level { get; set; } + public record LogEntryResponse(string Level, string Message); - public string? Message { get; set; } - } - - public class ResultResponse + public record ResultResponse(string DriverPath, string BrowserPath) { [JsonPropertyName("driver_path")] - public string? DriverPath { get; set; } + public string DriverPath { get; } = DriverPath; [JsonPropertyName("browser_path")] - public string? BrowserPath { get; set; } + public string BrowserPath { get; } = BrowserPath; } } [JsonSerializable(typeof(SeleniumManagerResponse))] - internal partial class SeleniumManagerSerializerContext : JsonSerializerContext - { - } + internal partial class SeleniumManagerSerializerContext : JsonSerializerContext; } From fe728c61e2dfb98133c731b0646cd2562823560c Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 24 Oct 2024 13:33:47 -0400 Subject: [PATCH 6/8] fix ResultResponse deserizalization --- dotnet/src/webdriver/SeleniumManager.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/dotnet/src/webdriver/SeleniumManager.cs b/dotnet/src/webdriver/SeleniumManager.cs index 5ba3205efb3ef..b1bdad1712212 100644 --- a/dotnet/src/webdriver/SeleniumManager.cs +++ b/dotnet/src/webdriver/SeleniumManager.cs @@ -229,13 +229,12 @@ internal record SeleniumManagerResponse(IReadOnlyList Logs, Re { public record LogEntryResponse(string Level, string Message); - public record ResultResponse(string DriverPath, string BrowserPath) + public record ResultResponse( + [property: JsonPropertyName("driver_path")] + string DriverPath, + [property: JsonPropertyName("browser_path")] + string BrowserPath) { - [JsonPropertyName("driver_path")] - public string DriverPath { get; } = DriverPath; - - [JsonPropertyName("browser_path")] - public string BrowserPath { get; } = BrowserPath; } } From b98732242dc247a8ea4b231a51a596f919bb9ef5 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 24 Oct 2024 13:34:57 -0400 Subject: [PATCH 7/8] Fix ResultResponse again --- dotnet/src/webdriver/SeleniumManager.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/dotnet/src/webdriver/SeleniumManager.cs b/dotnet/src/webdriver/SeleniumManager.cs index b1bdad1712212..1aae49ae981e4 100644 --- a/dotnet/src/webdriver/SeleniumManager.cs +++ b/dotnet/src/webdriver/SeleniumManager.cs @@ -229,12 +229,13 @@ internal record SeleniumManagerResponse(IReadOnlyList Logs, Re { public record LogEntryResponse(string Level, string Message); - public record ResultResponse( - [property: JsonPropertyName("driver_path")] - string DriverPath, - [property: JsonPropertyName("browser_path")] - string BrowserPath) + public record ResultResponse { + [JsonPropertyName("driver_path")] + public string? DriverPath { get; init; } + + [JsonPropertyName("browser_path")] + public string? BrowserPath { get; init; } } } From 0af8dd248d59426c8e6e80fe223052c3b56c7b87 Mon Sep 17 00:00:00 2001 From: Michael Render Date: Thu, 24 Oct 2024 13:39:23 -0400 Subject: [PATCH 8/8] Make ResultResponse properties required --- dotnet/src/webdriver/SeleniumManager.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dotnet/src/webdriver/SeleniumManager.cs b/dotnet/src/webdriver/SeleniumManager.cs index 1aae49ae981e4..e9060a8984e5e 100644 --- a/dotnet/src/webdriver/SeleniumManager.cs +++ b/dotnet/src/webdriver/SeleniumManager.cs @@ -232,10 +232,12 @@ public record LogEntryResponse(string Level, string Message); public record ResultResponse { [JsonPropertyName("driver_path")] - public string? DriverPath { get; init; } + [JsonRequired] + public string DriverPath { get; init; } = null!; [JsonPropertyName("browser_path")] - public string? BrowserPath { get; init; } + [JsonRequired] + public string BrowserPath { get; init; } = null!; } }