Skip to content
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

Add validation for command-line arguments related to new dotnet test #47616

Merged
merged 3 commits into from
Mar 15, 2025

Conversation

Youssef1313
Copy link
Member

  • Adding validation to command-line options that are mutually exclusive.
  • Cleans up the logic around architecture

@Copilot Copilot bot review requested due to automatic review settings March 14, 2025 21:20
@Youssef1313 Youssef1313 requested a review from a team as a code owner March 14, 2025 21:20
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Request triage from a team member labels Mar 14, 2025

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request introduces command‑line validation for mutually exclusive options and refactors the handling of build options in the dotnet test commands. Key changes include:

  • Adding a new ValidateMutuallyExclusiveOptions method in the ValidationUtility class.
  • Refactoring TestApplication and related classes to use a BuildOptions instance instead of a List of arguments.
  • Updating MSBuildHandler and TestModulesFilterHandler methods to align with the new BuildOptions usage.

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Cli/dotnet/commands/dotnet-test/ValidationUtility.cs Introduces mutually exclusive options validation logic.
src/Cli/dotnet/commands/dotnet-test/TestApplication.cs Replaces _args with BuildOptions and adjusts argument construction logic.
src/Cli/dotnet/commands/dotnet-test/TestingPlatformCommand.cs Updates invocation and parameter passing to support the new validation and BuildOptions usage.
src/Cli/dotnet/commands/dotnet-test/MSBuildHandler.cs Removes redundant BuildOptions parameters and uses the class-level BuildOptions consistently.
src/Cli/dotnet/commands/dotnet-test/TestModulesFilterHandler.cs Adjusts constructor and TestApplication instantiation to align with BuildOptions changes.
Comments suppressed due to low confidence (2)

src/Cli/dotnet/commands/dotnet-test/TestApplication.cs:92

  • The updated condition now includes a check for !isDll, which may bypass using 'dotnet run' unexpectedly when running non-dll tests. Please verify that this logic change aligns with the intended behavior.
if (testOptions.HasFilterMode || !isDll || !IsArchitectureSpecified(testOptions))

src/Cli/dotnet/commands/dotnet-test/MSBuildHandler.cs:137

  • Ensure that using the class-level _buildOptions in GetProjectsProperties is safe and that _buildOptions is always properly initialized to avoid potential null reference or state inconsistency issues.
(IEnumerable<TestModule> projects, bool restored) = GetProjectsProperties(solutionOrProjectFilePath, isSolution);
@Youssef1313 Youssef1313 merged commit 50b4f3e into main Mar 15, 2025
39 checks passed
@Youssef1313 Youssef1313 deleted the dev/ygerges/validation branch March 15, 2025 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants