[release/13.5] Resolve dotnet paths in doctor checks - #19263
Jose Perez Rodriguez (joperezr) merged 6 commits into
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19263Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19263" |
There was a problem hiding this comment.
Pull request overview
Note
Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.
This PR updates CLI checks to invoke dotnet via a resolved executable path (including dotnet.exe on Windows), and adds tests to verify the resolved path is used.
Changes:
- Resolve the
dotnetexecutable path and pass it into process start info for SDK and workload checks - Introduce
IEnvironmentdependency and a process-start injectable delegate to improve testability - Add/adjust tests to assert the resolved
dotnetpath is used
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Utils/DeprecatedWorkloadCheckTests.cs | Adds a test asserting DeprecatedWorkloadCheck uses the resolved dotnet path |
| tests/Aspire.Cli.Tests/DotNetSdkInstallerTests.cs | Refactors installer construction in tests and adds a resolved-path assertion test |
| src/Aspire.Cli/Utils/EnvironmentChecker/DeprecatedWorkloadCheck.cs | Uses resolved dotnet path and injects environment + start-process delegate |
| src/Aspire.Cli/DotNet/DotNetSdkInstaller.cs | Adds executable path resolution and updates process start creation to use it |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| internal static string ResolveDotNetPath(IEnvironment environment) => | ||
| PathLookupHelper.ResolveExecutablePath(environment.IsWindows() ? "dotnet.exe" : "dotnet"); |
| /// Users with this workload installed may encounter conflicts or confusion. | ||
| /// </remarks> | ||
| internal sealed class DeprecatedWorkloadCheck(ILogger<DeprecatedWorkloadCheck> logger) : IEnvironmentCheck | ||
| internal sealed class DeprecatedWorkloadCheck(ILogger<DeprecatedWorkloadCheck> logger, IEnvironment environment) : IEnvironmentCheck |
| var processInfo = new ProcessStartInfo | ||
| { | ||
| FileName = "dotnet", | ||
| FileName = DotNetSdkInstaller.ResolveDotNetPath(environment), |
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 2 / 100 test projects · 5 jobs, from 4 changed files. Selected test projects (2 / 100)
Selected jobs (5)
How these were chosen — grouped by what changed📦 affected project 🧪 🧪 Job reasons
Selection computed for commit |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Good hardening improvement. Approved. |
b80778f
into
release/13.5
|
✅ No documentation update needed. Step 5 branch taken: "excluded → head_branch_is_backport, title_release_prefix, body_backport_marker, base_branch_is_release" This PR is a backport and is out of scope for docs generation per Step 5's exclusion rule.
Documentation for this change, if needed, should be authored against the original forward PR (#19232) on the default branch, not duplicated here for the backport. |
Backport of #19232 to release/13.5
/cc James Newton-King (@JamesNK)
Customer Impact
The .NET SDK and deprecated-workload checks can launch an unresolved
dotnetexecutable instead of the executable selected fromPATH, makingaspire doctorresults unreliable in affected environments.Testing
DotNetSdkInstallerTestsandDeprecatedWorkloadCheckTests: 32 passed. Mutation verification confirmed both new tests fail with baredotnetand pass with path resolution restored. Source PR CI passed.Risk
Low. The change is localized to executable resolution for two doctor checks and preserves their existing process lifecycle and cancellation behavior.
Regression?
Unknown - please confirm.