Fix CI validation and publish desktop artifacts#9
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces the old msbuild-based CI workflow with dotnet-native commands, makes the browser UI test suite runnable without manual driver setup by auto-downloading ChromeDriver, and adds desktop artifact publishing for macOS, Windows, and Linux.
Changes:
- Replaced
msbuildCI jobs withdotnet build/test/publishcommands, added quality, unit, coverage, UI test, and desktop artifact jobs - Implemented automatic ChromeDriver download and caching in
BrowserAutomationBootstrapso UI tests run without manualUNO_UITEST_DRIVER_PATHsetup - Updated package versions, coverage runsettings, governance docs, and architecture docs to reflect the new CI contract
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
Replaced msbuild jobs with dotnet-native quality/test/coverage/UI/desktop-artifact jobs |
.github/steps/install_dependencies/action.yml |
Switched to setup-dotnet@v4 with global-json-file instead of explicit version |
DotPilot.UITests/BrowserAutomationBootstrap.cs |
Added automatic ChromeDriver download, version detection, and caching |
DotPilot.UITests/TestBase.cs |
Added explicit --window-size selenium argument, always pass driver path |
DotPilot.UITests/BrowserAutomationBootstrapTests.cs |
Added environment save/restore in setup/teardown, updated test for non-null driver path |
DotPilot/Services/Endpoints/DebugHandler.cs |
Refactored to avoid warnings: #if DEBUG around logger field |
Directory.Packages.props |
Bumped coverlet, NUnit, test SDK, adapter, and logger versions |
global.json |
Pinned SDK version to 10.0.103 |
DotPilot.Tests/coverlet.runsettings |
New runsettings to scope coverage and exclude generated artifacts |
AGENTS.md, DotPilot/AGENTS.md, DotPilot.Tests/AGENTS.md, DotPilot.UITests/AGENTS.md |
Updated governance docs for new CI contract and terminology |
docs/Architecture.md |
Updated architecture docs for CI artifacts and UI test terminology |
ui-tests-mandatory.plan.md, ci-pr-validation.plan.md |
Task planning documents |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| { | ||
| ArgumentNullException.ThrowIfNull(logger); | ||
| #if DEBUG | ||
| _logger = logger; |
There was a problem hiding this comment.
In Release builds, the logger parameter is validated via ThrowIfNull but never stored or used, which will produce a compiler warning (CS9113 or similar for unused parameter, or in this case the local is simply discarded). Consider suppressing the warning explicitly or restructuring so the parameter isn't accepted at all in Release, to avoid confusion for future maintainers.
| _logger = logger; | |
| _logger = logger; | |
| #else | |
| _ = logger; |
| - name: Setup .NET SDK from global.json | ||
| uses: actions/setup-dotnet@v4 | ||
| with: | ||
| global-json-file: global.json |
There was a problem hiding this comment.
The desktop_artifacts job only runs actions/setup-dotnet but skips the shared install_dependencies composite action that the other jobs use. That action also runs uno-check to install Uno Platform workloads, which are likely required for dotnet publish with net10.0-desktop. Without workloads installed, this job may fail on CI. Consider using the shared install_dependencies action here as well, or at minimum adding a workload install step.
Summary
Verification