[dotnet] Simplify testing driver factory

[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.

• Simplify DriverFactory and EnvironmentManager types in the test suite
• Add constructor to the testing driver types which take just a respective driver service

Motivation and Context

Taking some code changes out of #14662 which can be added without that feature.

Preparatory work for taking more changes from that PR.

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, Tests

---

Description

• Added constructors to driver classes for DriverService initialization.

• Refactored DriverFactory for improved readability and functionality.

• Simplified EnvironmentManager by introducing readonly fields and properties.

• Removed redundant methods and improved code consistency across files.

---

Changes walkthrough 📝

Relevant files

Enhancement

11 files DefaultSafariDriver.cs Added constructor for `SafariDriverService` initialization. +5/-0      DevChannelChromeDriver.cs Added constructor for `ChromeDriverService` initialization. +5/-0      DevChannelEdgeDriver.cs Added constructor for `EdgeDriverService` initialization. +5/-0      EdgeInternetExplorerModeDriver.cs Added constructor for `InternetExplorerDriverService` initialization. +5/-0      NightlyChannelFirefoxDriver.cs Added constructor for `FirefoxDriverService` initialization. +5/-0      SafariTechnologyPreviewDriver.cs Added constructor for `SafariDriverService` initialization. +5/-0      StableChannelChromeDriver.cs Added constructor for `ChromeDriverService` initialization. +5/-0      StableChannelEdgeDriver.cs Added constructor for `EdgeDriverService` initialization. +6/-0      StableChannelFirefoxDriver.cs Added constructor for `FirefoxDriverService` initialization. +5/-0      DriverFactory.cs Refactored `DriverFactory` for better readability and functionality. +25/-51  EnvironmentManager.cs Simplified `EnvironmentManager` with readonly fields and properties. +18/-68

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Null Reference The CreateDriverWithOptions method may return null if driver initialization fails, as the driver variable is only initialized in the final fallback case. This could lead to NullReferenceException. public IWebDriver CreateDriverWithOptions(Type driverType, DriverOptions driverOptions, bool enableLogging = false) { Console.WriteLine($"Creating new driver of {driverType} type..."); Browser browser = Browser.All; DriverService service = null; DriverOptions options = null; IWebDriver driver; if (typeof(ChromeDriver).IsAssignableFrom(driverType)) { browser = Browser.Chrome; options = GetDriverOptions<ChromeOptions>(driverType, driverOptions); var chromeOptions = (ChromeOptions)options; chromeOptions.AddArguments("--no-sandbox", "--disable-dev-shm-usage"); service = CreateService<ChromeDriverService>(); if (!string.IsNullOrEmpty(this.browserBinaryLocation)) { ((ChromeOptions)options).BinaryLocation = this.browserBinaryLocation; } if (enableLogging) { ((ChromiumDriverService)service).EnableVerboseLogging = true; } } else if (typeof(EdgeDriver).IsAssignableFrom(driverType)) { browser = Browser.Edge; options = GetDriverOptions<EdgeOptions>(driverType, driverOptions); var edgeOptions = (EdgeOptions)options; edgeOptions.AddArguments("--no-sandbox", "--disable-dev-shm-usage"); service = CreateService<EdgeDriverService>(); if (!string.IsNullOrEmpty(this.browserBinaryLocation)) { ((EdgeOptions)options).BinaryLocation = this.browserBinaryLocation; } if (enableLogging) { ((ChromiumDriverService)service).EnableVerboseLogging = true; } } else if (typeof(InternetExplorerDriver).IsAssignableFrom(driverType)) { browser = Browser.IE; options = GetDriverOptions<InternetExplorerOptions>(driverType, driverOptions); service = CreateService<InternetExplorerDriverService>(); if (enableLogging) { ((InternetExplorerDriverService)service).LoggingLevel = InternetExplorerDriverLogLevel.Trace; } } else if (typeof(FirefoxDriver).IsAssignableFrom(driverType)) { browser = Browser.Firefox; options = GetDriverOptions<FirefoxOptions>(driverType, driverOptions); service = CreateService<FirefoxDriverService>(); if (!string.IsNullOrEmpty(this.browserBinaryLocation)) { ((FirefoxOptions)options).BinaryLocation = this.browserBinaryLocation; } if (enableLogging) { ((FirefoxDriverService)service).LogLevel = FirefoxDriverLogLevel.Trace; } } else if (typeof(SafariDriver).IsAssignableFrom(driverType)) { browser = Browser.Safari; options = GetDriverOptions<SafariOptions>(driverType, driverOptions); service = CreateService<SafariDriverService>(); } if (!string.IsNullOrEmpty(this.driverPath) && service != null) { service.DriverServicePath = Path.GetDirectoryName(this.driverPath); service.DriverServiceExecutableName = Path.GetFileName(this.driverPath); } this.OnDriverLaunching(service, options); if (browser != Browser.All) { ConstructorInfo ctorInfo = driverType.GetConstructor([this.serviceTypes[browser], this.optionsTypes[browser]]); if (ctorInfo != null) { return (IWebDriver)ctorInfo.Invoke([service, options]); } } driver = (IWebDriver)Activator.CreateInstance(driverType);

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add null checks for parameters Add null check before invoking the driver constructor to prevent potential NullReferenceException if service or options are null dotnet/test/common/Environment/DriverFactory.cs [161-164] -if (ctorInfo != null) +if (ctorInfo != null && service != null && options != null) { return (IWebDriver)ctorInfo.Invoke([service, options]); } Apply this suggestion Suggestion importance[1-10]: 8 __ Why: The suggestion addresses a potential runtime NullReferenceException by adding necessary null checks before invoking the constructor. This is a critical safety improvement that prevents crashes.	Medium
	Add reflection error handling Add error handling around reflection invocation which can throw various exceptions at runtime dotnet/test/common/Environment/DriverFactory.cs [213-217] -MethodInfo createDefaultServiceMethod = typeof(TService).GetMethod("CreateDefaultService", BindingFlags.Public | BindingFlags.Static, null, [], null); -if (createDefaultServiceMethod != null && createDefaultServiceMethod.ReturnType == typeof(TService)) +try { + MethodInfo createDefaultServiceMethod = typeof(TService).GetMethod("CreateDefaultService", BindingFlags.Public | BindingFlags.Static, null, [], null); + if (createDefaultServiceMethod != null && createDefaultServiceMethod.ReturnType == typeof(TService)) + { + return (TService)createDefaultServiceMethod.Invoke(null, []); + } +} +catch (Exception ex) when (ex is TargetInvocationException or InvalidOperationException) { - return (TService)createDefaultServiceMethod.Invoke(null, []); + throw new InvalidOperationException($"Failed to create service for {typeof(TService).Name}", ex); } Apply this suggestion Suggestion importance[1-10]: 7 __ Why: Adding proper exception handling for reflection operations is important for robustness, as reflection can fail in various ways at runtime. The suggestion provides specific error handling with meaningful error messages.	Medium
Learned best practice	Add null validation checks for constructor parameters to prevent potential NullReferenceExceptions The constructor parameters driverPath and browserBinaryLocation should be validated for null values since they are used throughout the class. Add ArgumentNullException checks at the start of the constructor. dotnet/test/common/Environment/DriverFactory.cs [40-41] public DriverFactory(string driverPath, string browserBinaryLocation) { + ArgumentNullException.ThrowIfNull(driverPath, nameof(driverPath)); + ArgumentNullException.ThrowIfNull(browserBinaryLocation, nameof(browserBinaryLocation)); this.driverPath = driverPath; Apply this suggestion	Low

[nvborisenko]

Not sure it is in runtime. As I remember we always use ctor with service and options.

[RenderMichael]

I saw it was included in your PR #14662 so it would be simpler to include it here and shrink that PR.

If we don't use it, then that's OK. I think it doesn't hurt anything, and makes potential future refactorings simpler, no?

[nvborisenko]

I don't think code, which is not used, makes life easier.

[RenderMichael]

Fixed

[nvborisenko]

We should define a strategy how we want to introduce async methods. As soon as we have clear vision, then will proceed.

[RenderMichael]

Completely unused method

[nvborisenko]

Please ignore my last comment, wrong tab.

[RenderMichael]

No worries!

I removed the extra constructors on the testing driver implementations. Let me know if there's anything else this PR needs