[dotnet] test Selenium Manager #16775
Open
+161
−26
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
selenium-ci
added
C-dotnet
Contributor
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 |
|||||||||||||||||||||||||||||||
Contributor
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||
RenderMichael
approved these changes
Dec 22, 2025
Co-authored-by: Michael Render <[email protected]>
Contributor
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Member
|
In my opinion this is bad precedent. What we test... We should test that binding "invokes external dependency with proper parameters" and properly parse response. That's it. Testing... how to test... i don't see real unit tests. No clue. |
Member
Author
|
Well, needing unit tests in dotnet is a completely separate issue. 😂 Even then, this should be part of a slightly bigger change, and applied across the bindings. |
Member
Author
|
But also, something is weird on Windows... SM is not seeing the environment variables I'm passing in. Not sure if it is how the test is wired up, or an issue with how .NET is calling it. Once I figure out why, then we'll know if a unit test would have caught it. 😄 |
Member
I don't mind having one integration test to invoke SM executing on GHA. +1 |
nvborisenko
reviewed
Dec 24, 2025
Comment on lines
+106
to
+117
| var service = CreateServiceForCurrentBrowser(driverPath); | ||
|
|
||
| try | ||
| { | ||
| service.Start(); | ||
| Assert.That(service.ServiceUrl, Is.Not.Null); | ||
| Assert.That(service.ServiceUrl.IsAbsoluteUri, Is.True); | ||
| } | ||
| finally | ||
| { | ||
| service.Dispose(); | ||
| } |
nvborisenko
reviewed
Dec 24, 2025
Comment on lines
+49
to
+53
| string homeDirectory = System.Environment.GetFolderPath(System.Environment.SpecialFolder.UserProfile); | ||
| if (!string.IsNullOrWhiteSpace(homeDirectory)) | ||
| { | ||
| return Path.Combine(homeDirectory, ".cache", "selenium"); | ||
| } |
Member
There was a problem hiding this comment.
This is responsibility of SM, dotnet should not know about this folder.
Member
Author
There was a problem hiding this comment.
This is a way for us to see that it has in fact been downloaded and not just using the current installation. The contract is SE_CACHE_PATH which we check first. This is currently failing in windows, and again, depending on why would validate the need for this test. 😄
We could also set SE_CACHE_PATH as part of the test which would exercise the contract that way. That might be the right way to do this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
User description
🔗 Related Issues
.NET equivalent of #16756
💥 What does this PR do?
🔧 Implementation Notes
This could be split out in each of the browser directories instead of all handled in one test
💡 Additional Considerations
PR Type
Tests, Enhancement
Description
Add Selenium Manager tests for .NET across Windows, macOS, and Ubuntu
Verify driver and browser path discovery via DriverFinder
Validate driver service startup with discovered paths
Configure CI workflow with OS-specific caching and test matrix
Skip Safari and IE browsers as they don't require Selenium Manager
Diagram Walkthrough
File Walkthrough
SeleniumManagerTest.cs
New Selenium Manager test suite for .NETdotnet/test/common/SeleniumManagerTest.cs
DriverFinder.GetDriverPath()andGetBrowserPath()methodsDriverService.Start()with discovered driver pathsIgnoreBrowserattributesci-dotnet.yml
Add CI workflow for Selenium Manager tests.github/workflows/ci-dotnet.yml
manager-testsjob running on Windows, macOS, and Ubuntuhandling
falseto OS-specific keysBUILD.bazel
Update test suite configurationdotnet/test/common/BUILD.bazel