[dotnet] Lazy-load Selenium manager binary location

[RenderMichael]

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

The location of the selenium manager binary is calculated when it is first accessed.

Motivation and Context

I ran into this while working on #14480

It is considered bad practice for static constructors to throw exceptions. If an exception is throws, the type is rendered unusable for the rest of the application lifecycle. The stack-trace of the exception is also much harder to follow, and the debugging experience is worsened. The fact that it throws TypeInitializationException threw me off initially, because I was testing AOT at the time.

Exceptions are expected on this flow, in exceptional cases, they are even thrown directly. With this change, the Stack trace changes:

Before:

After:

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement

---

Description

• Implemented lazy-loading for the Selenium Manager binary path to improve performance and avoid exceptions during static initialization.
• Replaced the static constructor with a method GetBinaryFullPath to determine the binary path dynamically.
• Enhanced error handling by providing more informative exception messages when the binary is not found or the platform is unsupported.

---

Changes walkthrough 📝

	Relevant files
Enhancement	SeleniumManager.cs Implement lazy-loading for Selenium Manager binary path    dotnet/src/webdriver/SeleniumManager.cs Introduced lazy-loading for the Selenium Manager binary path. Replaced static constructor with a method to determine binary path. Improved error handling for unsupported platforms. Enhanced exception messages for missing binaries. +11/-9

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[codiumai-pr-agent-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 14480 - Partially compliant Fully compliant requirements: Partially addresses Selenium Manager compatibility Not compliant requirements: Does not address W3C WebDriver, CDP, or W3C BiDi compatibility

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Thread Safety The lazy initialization of _binaryFullPath is not thread-safe. Consider using Lazy<T> or double-checked locking to ensure thread safety. Error Handling The error message when the binary is not found could be more informative. Consider adding suggestions for resolving the issue.

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Enhancement	Use a switch expression for platform-specific path assignments to improve code readability and maintainability Consider using a switch expression instead of multiple if-else statements for platform-specific path assignments. This can make the code more concise and easier to maintain. dotnet/src/webdriver/SeleniumManager.cs [50-66] -if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) +binaryFullPath = RuntimeInformation.OSPlatform switch { - 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"); -} -else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) -{ - binaryFullPath = Path.Combine(currentDirectory, "selenium-manager", "macos", "selenium-manager"); -} -else -{ - throw new PlatformNotSupportedException( - $"Selenium Manager doesn't support your runtime platform: {RuntimeInformation.OSDescription}"); -} + OSPlatform.Windows => Path.Combine(currentDirectory, "selenium-manager", "windows", "selenium-manager.exe"), + OSPlatform.Linux => Path.Combine(currentDirectory, "selenium-manager", "linux", "selenium-manager"), + OSPlatform.OSX => Path.Combine(currentDirectory, "selenium-manager", "macos", "selenium-manager"), + _ => throw new PlatformNotSupportedException($"Selenium Manager doesn't support your runtime platform: {RuntimeInformation.OSDescription}") +}; Apply this suggestion Suggestion importance[1-10]: 8 Why: This suggestion improves code readability and maintainability by replacing multiple if-else statements with a switch expression, which is more concise and easier to understand.	8
Best practice	Use a constant for frequently used string values to improve maintainability and reduce errors Consider using a constant or readonly field for the "selenium-manager" folder name to improve maintainability and reduce the risk of typos when used in multiple places. dotnet/src/webdriver/SeleniumManager.cs [52] -binaryFullPath = Path.Combine(currentDirectory, "selenium-manager", "windows", "selenium-manager.exe"); +private const string SeleniumManagerFolder = "selenium-manager"; +... +binaryFullPath = Path.Combine(currentDirectory, SeleniumManagerFolder, "windows", "selenium-manager.exe"); Suggestion importance[1-10]: 7 Why: Introducing a constant for the "selenium-manager" folder name enhances maintainability by reducing the risk of typos and making future changes easier, especially when the string is used in multiple places.	7

💡 Need additional feedback ? start a PR chat

[nvborisenko]

Thank you! It can be even simplified to avoid cunning logic, please see inner comment.

[RenderMichael]

Thanks for the feedback, the PR has been updated

[nvborisenko]

Thank you!

[RenderMichael]

Thank you @nvborisenko !

I dug into the record syntax that worked, and you were right, it was working. There may be some special support for records. However, the moment I added [JsonRequired], I got an exception:

I'm not familiar with the STJ esoterica here, but it looks like what we have now works. Maybe once v9 comes out, the internal serialization options could enable the feature flags I linked and prevent null reference exceptions even further.