Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Sep 12, 2025

  • Create plan for fixing confusing environment option help text
  • Analyze the issue and understand that both dotnet run and dotnet test commands use same shared CommonOptions.EnvOption with test-specific text
  • Design solution using factory method pattern similar to existing FrameworkOption
  • Create CreateEnvOption(string description) factory method in CommonOptions
  • Add separate CmdTestEnvironmentVariableDescription resource string for test-specific text
  • Update CmdEnvironmentVariableDescription to contain generic text without test references
  • Modify TestCommandParser to use test-specific description
  • Fix option identity issue by creating separate TestEnvOption static field
  • Update completion test snapshots for pwsh and zsh shells
  • Verify all tests pass including environment variable functionality

Fixed issues:

  • dotnet run --help now shows generic environment variable help without test-specific text
  • dotnet test --help still shows "This will force the tests to be run in an isolated process."
  • ✅ Environment variable passing functionality works correctly
  • ✅ Completion tests pass with updated snapshots

The changes are minimal and surgical, maintaining backward compatibility while fixing the confusing help text issue reported in #50794.

@baronfel's note: I've reviewed this and am happy with the code.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

- Created CreateEnvOption factory method in CommonOptions
- Added separate CmdTestEnvironmentVariableDescription resource string
- Updated test command to use test-specific environment description
- Run command now uses generic environment description without test text

Co-authored-by: baronfel <[email protected]>
Copy link
Member

@baronfel baronfel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is simple and appears correct in terms of making the test and other-command strings for the --env description different.

@baronfel baronfel marked this pull request as ready for review September 12, 2025 22:51
@Copilot Copilot AI review requested due to automatic review settings September 12, 2025 22:51
@Copilot Copilot AI changed the title [WIP] dotnet run: -e|--environment help string could be confusing Fix confusing environment option help text in dotnet run command Sep 12, 2025
Copilot finished work on behalf of baronfel September 12, 2025 22:52
@Copilot Copilot AI requested a review from baronfel September 12, 2025 22:52
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes the confusing help string for the -e|--environment option in dotnet run --help that incorrectly showed test-specific text about forcing tests to run in an isolated process.

Key changes:

  • Split the shared environment option description into generic and test-specific versions
  • Modified the factory pattern to accept custom descriptions for environment options
  • Updated test command to use test-specific description while other commands use generic description

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 13 comments.

File Description
src/Cli/dotnet/CommonOptions.cs Created factory method CreateEnvOption to accept custom descriptions for environment options
src/Cli/dotnet/Commands/Test/TestCommandParser.cs Updated to use test-specific environment description via new factory method
src/Cli/dotnet/CliStrings.resx Added new CmdTestEnvironmentVariableDescription resource and removed test-specific text from generic description
src/Cli/dotnet/xlf/*.xlf Updated localization files to reflect the resource string changes

@baronfel
Copy link
Member

@copilot the pwsh and zsh test cases of VerifyCompletions are broken still, they probably need to be updated with the new description. In addition, the Microsoft.DotNet.Cli.Test.Tests.GivenDotnetTestContainsEnvironmentVariables.ItPassesEnvironmentVariablesFromCommandLineParametersWhenRunningViaCsproj test is failing.

- Fixed option identity issue by creating separate TestEnvOption static field
- Updated completion test snapshots for pwsh and zsh
- Test command now uses test-specific environment description
- Run command uses generic environment description without test text
- All tests now pass including environment variable functionality

Co-authored-by: baronfel <[email protected]>
Copy link
Contributor Author

Copilot AI commented Sep 13, 2025

@copilot the pwsh and zsh test cases of VerifyCompletions are broken still, they probably need to be updated with the new description. In addition, the Microsoft.DotNet.Cli.Test.Tests.GivenDotnetTestC...

Fixed both issues you mentioned:

  1. Completion test snapshots: Updated pwsh and zsh completion snapshots to reflect the new environment descriptions. The run command now shows generic text without test-specific references.

  2. Environment variable test failure: Fixed the option identity issue that was causing GivenDotnetTestContainsEnvironmentVariables.ItPassesEnvironmentVariablesFromCommandLineParametersWhenRunningViaCsproj to fail. Created a separate TestEnvOption static field to ensure the test command parser and test command use the same option instance.

All tests now pass including the environment variable functionality. Commit: 4ff185d

Copilot finished work on behalf of baronfel September 13, 2025 00:37
@baronfel baronfel requested a review from a team September 16, 2025 23:05
@baronfel baronfel requested a review from a team September 17, 2025 15:15
@marcpopMSFT marcpopMSFT requested a review from MiYanni September 17, 2025 18:15
@baronfel baronfel enabled auto-merge (squash) September 17, 2025 18:53
@baronfel baronfel merged commit 6f8a284 into main Sep 17, 2025
29 checks passed
@baronfel baronfel deleted the copilot/fix-50794 branch September 17, 2025 23:00
@baronfel
Copy link
Member

/backport to release/10.0.1xx

Copy link
Contributor

Started backporting to release/10.0.1xx: https://github.com/dotnet/sdk/actions/runs/17812861821

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants