[dotnet] Fix dev environment to run tests on Windows/MacOS

[nvborisenko]

User description

• Upgraded Runfiles nuget package
• Fixed transient resolved dependencies
• Fixed build target for appserver

Motivation and Context

Now I am able to use my IDE to execute tests:

• Windows with Visual Studio
• MacOS with Visual Studio Code

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

Bug fix, Enhancement

---

Description

• Updated Runfiles package to version 0.14.0.

• Refactored TestWebServer to improve IDE compatibility.

• Simplified dependency management by removing unnecessary packages.

• Fixed nullable warning in CallFunctionCommand.

---

Changes walkthrough 📝

	Relevant files
Enhancement	TestWebServer.cs Refactor `TestWebServer` for IDE compatibility                      dotnet/test/common/Environment/TestWebServer.cs Refactored TestWebServer to use Runfiles for appserver path resolution. Removed unused variables and redundant code. Adjusted process arguments for better IDE compatibility. Simplified error handling for missing appserver path. +16/-60  WebDriver.Common.Tests.csproj Update dependencies and build target in project file          dotnet/test/common/WebDriver.Common.Tests.csproj Updated Runfiles package to version 0.14.0. Adjusted Bazel build target for appserver. +2/-2
Bug fix	CallFunctionCommand.cs Fix nullable warning in `CallFunctionCommand`                        dotnet/src/webdriver/BiDi/Modules/Script/CallFunctionCommand.cs Fixed nullable warning in Arguments property. +1/-1
Dependencies	paket.dependencies Simplify and update dependencies                                                  dotnet/paket.dependencies Updated Runfiles package to version 0.14.0. Removed unnecessary dependencies to simplify management. +2/-13
Additional files	paket.nuget.bzl +13/-31  NuGet.Config +0/-6

---

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 🔍

(Review updated until commit aebbdc6)

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 Commented Code Commented out code for IDE path resolution should be removed or implemented properly to ensure IDE compatibility works as intended //var baseDirectory = AppContext.BaseDirectory; //standaloneAppserverPath = Path.Combine(baseDirectory, "../../../../../../bazel-bin/java/test/org/openqa/selenium/environment/appserver"); Error Handling The catch block for FileNotFoundException is empty which could silently fail. Consider adding proper error handling or logging. catch (FileNotFoundException) { //var baseDirectory = AppContext.BaseDirectory; //standaloneAppserverPath = Path.Combine(baseDirectory, "../../../../../../bazel-bin/java/test/org/openqa/selenium/environment/appserver"); }

[qodo-merge-pro]

PR Code Suggestions ✨

Latest suggestions up to aebbdc6
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Validate required parameter Add validation for the port parameter before using it in process arguments to ensure it's not null or empty. dotnet/test/common/Environment/TestWebServer.cs [68] -string processArguments = $"{port}"; +string processArguments = string.IsNullOrEmpty(port) ? throw new ArgumentException("Port cannot be null or empty") : $"{port}"; Apply this suggestion Suggestion importance[1-10]: 8 __ Why: The port parameter is critical for the web server functionality, and validating it early prevents potential runtime errors and provides clearer error messages. This is an important defensive programming practice.	Medium
	Include jar extension in build target The Bazel build command should include the jar extension to ensure the correct artifact is built and available for tests dotnet/test/common/WebDriver.Common.Tests.csproj [37] -<Exec Command="bazel build //java/test/org/openqa/selenium/environment:appserver" WorkingDirectory="../../.." /> +<Exec Command="bazel build //java/test/org/openqa/selenium/environment:appserver_deploy.jar" WorkingDirectory="../../.." /> Apply this suggestion Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that removing '_deploy.jar' from the build target could prevent the proper Java artifact from being built, which is critical for test execution.	Medium
	Prevent null reference exception Add null check for webserverProcess before accessing its properties in the error message construction to prevent potential NullReferenceException. dotnet/test/common/Environment/TestWebServer.cs [127] -string errorMessage = string.Format("Could not start the test web server in {0} seconds.\nWorking directory: {1}\nProcess Args: {2}\nstdout: {3}\nstderr: {4}", timeout.TotalSeconds, projectRootPath, processArguments, output, error); +string errorMessage = string.Format("Could not start the test web server in {0} seconds.\nWorking directory: {1}\nProcess Args: {2}\nstdout: {3}\nstderr: {4}", timeout.TotalSeconds, projectRootPath, processArguments ?? "N/A", output ?? "N/A", error ?? "N/A"); Apply this suggestion Suggestion importance[1-10]: 7 __ Why: Adding null checks for output and error variables is important to prevent potential NullReferenceException when constructing error messages, which could mask the actual error causing the test web server to fail.	Medium
Learned best practice	Add null parameter validation at the start of methods to prevent NullReferenceExceptions The projectRoot and config parameters in the constructor should be validated for null to prevent potential NullReferenceException when accessing their properties. Add null checks using ArgumentNullException.ThrowIfNull(). dotnet/test/common/Environment/TestWebServer.cs [41-47] public TestWebServer(string projectRoot, TestWebServerConfig config) { + ArgumentNullException.ThrowIfNull(projectRoot, nameof(projectRoot)); + ArgumentNullException.ThrowIfNull(config, nameof(config)); + this.projectRootPath = projectRoot; this.captureWebServerOutput = config.CaptureConsoleOutput; this.hideCommandPrompt = config.HideCommandPromptWindow; this.port = config.Port; } Apply this suggestion Suggestion importance[1-10]: 6	Low
Update

---

Previous suggestions

Suggestions up to commit bed777d

Category	Suggestion	Impact
Possible issue	Fix string format parameter order The error message string format is missing a placeholder for the project root path parameter. Add {2} placeholder in the error message format string to properly display the project root path. dotnet/test/common/Environment/TestWebServer.cs [78-83] throw new FileNotFoundException( string.Format( - "Test Appserver at {0} didn't exist. Project root is {2}. Please build it using something like {1}.", + "Test Appserver at {0} didn't exist. Project root is {1}. Please build it using something like {2}.", standaloneAppserverPath, - "bazel build //java/test/org/openqa/selenium/environment:appserver", - projectRootPath)); + projectRootPath, + "bazel build //java/test/org/openqa/selenium/environment:appserver")); Suggestion importance[1-10]: 8 __ Why: The suggestion fixes a critical bug where the string format parameters are in incorrect order, causing projectRootPath to be referenced as {2} but passed as the third argument. This could lead to runtime formatting errors or incorrect error messages.	Medium
	Add error handling for build The Bazel build command should include error handling to ensure the build process fails gracefully if the command execution fails. Add a condition to check the exit code. dotnet/test/common/WebDriver.Common.Tests.csproj [36-38] <Target Name="BuildTestWebServer" AfterTargets="AfterBuild"> - <Exec Command="bazel build //java/test/org/openqa/selenium/environment:appserver" WorkingDirectory="../../.." /> + <Exec Command="bazel build //java/test/org/openqa/selenium/environment:appserver" WorkingDirectory="../../.." IgnoreExitCode="false" /> </Target> Suggestion importance[1-10]: 7 __ Why: Adding explicit error handling with IgnoreExitCode="false" is a good practice to ensure build failures are properly caught and reported, improving the build process reliability.	Medium
General	Use safer dependency resolution strategy The strategy: min setting might cause compatibility issues by always selecting the minimum version that satisfies dependencies. Consider using max strategy for better stability. dotnet/paket.dependencies [1-3] group nuget framework: net8.0,netstandard2.0 -strategy: min +strategy: max Suggestion importance[1-10]: 8 __ Why: Changing from 'min' to 'max' strategy is important for stability as it reduces the risk of compatibility issues by using the latest compatible versions rather than minimum versions.	Medium

[nvborisenko]

@RenderMichael can you please check whether it is still working on your environment? Looking forward any feedback.

[RenderMichael]

This all works for me!

Can I ask what wasn't working before? Maybe it's because I built the Java bindings, but I had no issues. I didn't try macOS though.

[RenderMichael]

I opened the DevToolsProtocolGenerator solution to ensure the CDP generator builds, and it added some unignored files to the bin folder:

I'm not sure how it's related, but we might also need to update the .gitignore for this.

[nvborisenko]

I should not change build output path for DevTools generator in this PR. I will revert it back.

[nvborisenko]

Finally I did it working, supporting .NET cross-platform IDE to execute tests.

Why

I leaned that bazel build //java/test/org/openqa/selenium/environment:appserver actually generates shell script. This shell script works fine on Windows, but not in MacOS. The error is something about "RUNFILES". Then I tried building appserver target with --enable_runfiles argument, no success on MacOS.

How

Do not execute generated shell script by bazel. Use bazel run //java/test/org/openqa/selenium/environment:appserver, it is smart enough to prepare env vars.

Can be simplified?

Yes, it can! Python and Ruby don't have dependency to java web server, .net is happy to introduce own.

[RenderMichael]

Confirmed that the bin files from the screenshot are no longer an issue.

Love this!

[p0deje]

Looks good from a quick glance! I'd merge as long as RBE and .NET CIs are passing.

[nvborisenko]

Merging it. If somebody has issues to execute tests in IDE, please post new issue.