Add support for flat launch settings #49769
Conversation
| [InlineData(TestingConstants.Release)] | ||
| [Theory] | ||
| public void RunTestProjectWithTestsAndLaunchSettings_ShouldReturnExitCodeSuccess(string configuration) | ||
| [Theory, CombinatorialData] |
There was a problem hiding this comment.
Happy to see we have an updated test for dotnet test :)
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for “flat” launch settings files (e.g. MyApp.run.json) alongside the existing launchSettings.json.
Key changes:
- Updated CLI logic in
RunCommand,CommonRunHelpers, andLaunchSettingsManagerto locate and apply both legacy and flat launch settings. - Expanded tests in
dotnet test,dotnet run, anddotnet-watchsuites to cover flat launch settings; updated completion snapshots. - Updated resource strings and localization artifacts to reference both file patterns.
Reviewed Changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Cli/dotnet/Commands/Run/CommonRunHelpers.cs | Added helpers to build paths for both launch settings. |
| src/Cli/dotnet/Commands/Run/RunCommand.cs | Updated launch settings lookup logic, error reporting. |
| src/Cli/dotnet/Commands/Run/LaunchSettings/LaunchSettingsManager.cs | Changed TryApplyLaunchSettings to accept a file path. |
| src/Cli/dotnet/Commands/Test/SolutionAndProjectUtility.cs | Kept test utility in sync with new lookup signature. |
| test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTests.cs | Extended test combinatorics for flat vs legacy profiles. |
| test/dotnet.Tests/CommandTests/Run/*.cs | Added run.json-based launch profile tests. |
| test/dotnet-watch.Tests/Process/LaunchSettingsProfileTest.cs | Adapted to use flat launch settings path. |
| test/dotnet.Tests/CompletionTests/snapshots/{zsh,pwsh}/DotnetCliSnapshotTests.VerifyCompletions.* | Updated completion descriptions for --no-launch-profile. |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.*.xlf and CliCommandStrings.resx | Updated message strings to mention flat launch settings. |
Comments suppressed due to low confidence (2)
test/dotnet.Tests/CommandTests/Run/GivenDotnetRunBuildsVbProj.cs:32
- There is no
testInstancevariable defined in this test. It appears you meant to use thetestProjectDirectoryvariable for the working directory. Please update the references accordingly so the test compiles.
{Path.Join(testInstance.Path, "My Project", "launchSettings.json")}
test/dotnet.Tests/CommandTests/Run/GivenDotnetRunBuildsCsProj.cs:382
- The variable
testInstanceis not defined here; the test usestestProjectDirectoryearlier. Please replacetestInstance.Pathwith the correct test directory variable so the test compiles and runs.
{Path.Join(testInstance.Path, "Properties", "launchSettings.json")}
| @@ -36,4 +36,10 @@ public static Dictionary<string, string> GetGlobalPropertiesFromArgs(string[] ar | |||
| } | |||
| return globalProperties; | |||
| } | |||
|
|
|||
| public static string GetPropertiesLaunchSettingsPath(string directoryPath, string propertiesDirectoryName) | |||
There was a problem hiding this comment.
[nitpick] The file path helpers use both Path.Combine and Path.Join in this class. Consider unifying on one approach (e.g. Path.Combine) for consistency and clarity.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
The existing helper used Path.Combine and I opted to preserve that to avoid a break.
| Reporter.Error.WriteLine(string.Format(CliCommandStrings.RunCommandExceptionCouldNotLocateALaunchSettingsFile, launchProfile, $""" | ||
| {launchSettingsPath} | ||
| {runJsonPath} | ||
| """).Red()); |
There was a problem hiding this comment.
[nitpick] Error messages elsewhere use .Bold().Red() for emphasis before writing to the reporter. For consistency, consider adding .Bold() here as well.
| """).Red()); | |
| """).Bold().Red()); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
This is not a hard error (in fact, the command continues to run in spite of the error), so I feel like bolding it is not necessary.
| <source>Do not attempt to use launchSettings.json to configure the application.</source> | ||
| <target state="translated">請勿嘗試使用 launchSettings.json 設定該應用程式。</target> | ||
| <note /> | ||
| <source>Do not attempt to use launchSettings.json or [app].run.json to configure the application.</source> |
There was a problem hiding this comment.
Manual edits to .xlf localization files should be avoided. Please update the source strings in the .resx and regenerate all .xlf files via the MSBuild /t:UpdateXlf target to ensure consistency.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
I didn't modify those, the build did this.
|
@jjonescz Quick question, the new |
Both. Also |
Yeah I was asking for the behavior of |
|
dotnet-watch changes LGTM |
|
@MiYanni @RikkiGibson @jaredpar for reviews, thanks |
Resolves #48200.