-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support --solution and --directory options in dotnet test #45859
Support --solution and --directory options in dotnet test #45859
Conversation
Co-authored-by: Amaury Levé <[email protected]>
…erties' of https://github.com/dotnet/sdk into dev/mabdullah/use-msbuild-apis-to-retrieve-project-properties
…ations is set to false
…roject-properties
Co-authored-by: Amaury Levé <[email protected]>
Co-authored-by: Amaury Levé <[email protected]>
Co-authored-by: Amaury Levé <[email protected]>
Co-authored-by: Amaury Levé <[email protected]>
Co-authored-by: Amaury Levé <[email protected]>
…roject-properties
Co-authored-by: Amaury Levé <[email protected]>
Co-authored-by: Amaury Levé <[email protected]>
…erties' of https://github.com/dotnet/sdk into dev/mabdullah/use-msbuild-apis-to-retrieve-project-properties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 9 out of 24 changed files in this pull request and generated 1 comment.
Files not reviewed (15)
- src/Cli/dotnet/commands/dotnet-test/LocalizableStrings.resx: Language not supported
- src/Cli/dotnet/commands/dotnet-test/xlf/LocalizableStrings.cs.xlf: Language not supported
- src/Cli/dotnet/commands/dotnet-test/xlf/LocalizableStrings.de.xlf: Language not supported
- src/Cli/dotnet/commands/dotnet-test/xlf/LocalizableStrings.es.xlf: Language not supported
- src/Cli/dotnet/commands/dotnet-test/xlf/LocalizableStrings.fr.xlf: Language not supported
- src/Cli/dotnet/commands/dotnet-test/xlf/LocalizableStrings.it.xlf: Language not supported
- src/Cli/dotnet/commands/dotnet-test/xlf/LocalizableStrings.ja.xlf: Language not supported
- src/Cli/dotnet/commands/dotnet-test/xlf/LocalizableStrings.ko.xlf: Language not supported
- src/Cli/dotnet/commands/dotnet-test/xlf/LocalizableStrings.pl.xlf: Language not supported
- src/Cli/dotnet/commands/dotnet-test/xlf/LocalizableStrings.pt-BR.xlf: Language not supported
- src/Cli/dotnet/commands/dotnet-test/BuiltInOptions.cs: Evaluated as low risk
- src/Cli/dotnet/commands/dotnet-test/TestingPlatformCommand.Help.cs: Evaluated as low risk
- src/Cli/dotnet/commands/dotnet-test/TestingPlatformOptions.cs: Evaluated as low risk
- src/Cli/dotnet/commands/dotnet-test/TestCommandParser.cs: Evaluated as low risk
- src/Cli/dotnet/commands/dotnet-test/TestApplication.cs: Evaluated as low risk
Comments suppressed due to low confidence (3)
src/Cli/dotnet/commands/dotnet-test/TestingPlatformCommand.cs:25
- Changing the Run method from async Task to int removes the ability to await asynchronous operations, which could lead to unhandled asynchronous operations and unexpected behavior.
public int Run(ParseResult parseResult)
src/Cli/dotnet/commands/dotnet-test/SolutionAndProjectUtility.cs:81
- [nitpick] The method GetProjectFilePaths is defined twice in the same file. Consider renaming one of the methods to avoid confusion.
private static string[] GetProjectFilePaths(string directory)
src/Cli/dotnet/commands/dotnet-test/SolutionAndProjectUtility.cs:24
- The new behavior introduced by the --solution option should be covered by tests to ensure it works as expected.
var possibleSolutionPaths = GetSolutionFilePaths(directory);
return false; | ||
} | ||
|
||
if (!File.Exists(filePath)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@baronfel do you have some helper around filesystem for mocking and more or is it fine to use this check directly?
src/Cli/dotnet/commands/dotnet-test/SolutionAndProjectUtility.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-test/SolutionAndProjectUtility.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-test/SolutionAndProjectUtility.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-test/SolutionAndProjectUtility.cs
Outdated
Show resolved
Hide resolved
Updating with latest main as we had a build break that just got fixed with 6b3a36d |
…https://github.com/dotnet/sdk into dev/mabdullah/support-solution-option-in-dotnet-test
Relates to #45927 |
This pull request includes several changes to improve and refactor the
dotnet-test
command in the .NET CLI. The changes focus on enhancing file path validation, updating localizable strings, and refactoring the MSBuild handler.Refactoring and Enhancements:
src/Cli/dotnet/commands/dotnet-test/MSBuildHandler.cs
: Refactored theRunMSBuild
method to handle project, solution, and directory paths more robustly. Added validation methods for build path options and file paths. [1] [2] [3] [4] [5] [6]src/Cli/dotnet/commands/dotnet-test/SolutionAndProjectUtility.cs
: Improved solution and project file path retrieval and validation. Updated methods to handle multiple file types and directories. [1] [2]Code Simplification:
src/Cli/dotnet/commands/dotnet-test/CliConstants.cs
: Added new constants for file extensions and separators, and removed hardcoded strings.src/Cli/dotnet/commands/dotnet-test/MSBuildHandler.cs
: Removed redundant constants and replaced them with values fromCliConstants
. [1] [2] [3]Localization Updates:
src/Cli/dotnet/commands/dotnet-test/LocalizableStrings.resx
: Added new error messages and descriptions for various command scenarios, such as invalid file extensions and multiple build path options. [1] [2]New Records:
src/Cli/dotnet/commands/dotnet-test/Options.cs
: Introduced new recordsBuildConfigurationOptions
andBuildPathsOptions
to encapsulate build configuration and path options.Removed Obsolete Code:
src/Cli/dotnet/commands/dotnet-test/BuiltInOptions.cs
: Removed the obsoleteBuiltInOptions
record.