[dotnet] Stream Selenium Manager output to internal logging#17024
[dotnet] Stream Selenium Manager output to internal logging#17024nvborisenko merged 35 commits intoSeleniumHQ:trunkfrom
Conversation
PR TypeEnhancement, Tests Description
|
| Relevant files | |||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Enhancement |
|
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
dotnet/src/webdriver/SeleniumManager.cs:310
- The generic catch block here wraps all exceptions, including
WebDriverExceptioninstances thrown above (for non-zero exit codes or timeouts), which means the detailed error messages you build are only available inInnerExceptionand the outer message is the more generic "Error starting process". This is a regression in error clarity and will make diagnosing Selenium Manager failures harder. Consider leaving existingWebDriverExceptions unwrapped (for example by using acatch (Exception ex) when (ex is not WebDriverException)filter) so callers see the rich process/timeout message directly, while still wrapping unexpected exceptions.
if (process.ExitCode != 0)
{
var exceptionMessageBuilder = new StringBuilder($"Selenium Manager process exited abnormally with {process.ExitCode} code: {process.StartInfo.FileName} {arguments}");
if (!string.IsNullOrWhiteSpace(stdOutputBuilder.ToString()))
{
exceptionMessageBuilder.AppendLine();
exceptionMessageBuilder.AppendLine("--- Standard Output ---");
exceptionMessageBuilder.Append(stdOutputBuilder);
exceptionMessageBuilder.AppendLine("--- End Standard Output ---");
}
if (!string.IsNullOrWhiteSpace(errOutputBuilder.ToString()))
{
exceptionMessageBuilder.AppendLine();
exceptionMessageBuilder.AppendLine("--- Error Output ---");
exceptionMessageBuilder.Append(errOutputBuilder);
exceptionMessageBuilder.AppendLine("--- End Error Output ---");
}
throw new WebDriverException(exceptionMessageBuilder.ToString());
}
}
catch (Exception ex)
{
throw new WebDriverException($"Error starting process: {process.StartInfo.FileName} {arguments}", ex);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
dotnet/src/webdriver/DriverFinder.cs:136
- If
browserPathvalidation fails, the driver path has already been added topaths, leaving a partially populated cache that can cause subsequent calls to fail withKeyNotFoundExceptioninstead ofNoSuchDriverException. Consider only updating the cache after both paths have been validated (or clearing the cache on failure).
if (File.Exists(driverPath))
{
paths.Add(DriverPathKey, driverPath);
}
else
{
throw new NoSuchDriverException($"The driver path is not a valid file: {driverPath}");
}
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
The changes modernize how browser and driver paths are discovered, enhance logging capabilities, and simplify the interface for driver discovery.
🔗 Related Issues
Contributes to #13989
💥 What does this PR do?
BinaryPathsmethod and argument-string construction with a newDiscoverBrowserAPI that takes structured options, builds arguments internally, and returns aDiscoveryResultobject. This makes the API clearer, more robust, and easier to extend.DriverFinderand now use the newSeleniumManager.DiscoverBrowsermethod for driver and browser path resolution.DriverFinderto use local constants for key names instead of referencing them fromSeleniumManager.ILogger,ILogContext) to support emitting log messages with explicit timestamps, and updated implementations accordingly.🔄 Types of changes