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

Use dotnet run in case of --arch in dotnet test #46446

Conversation

mariam-abdulla
Copy link
Member

@mariam-abdulla mariam-abdulla commented Jan 31, 2025

This pull request includes several changes to improve code readability and consistency in the dotnet-test command implementation. The most important changes involve making variables readonly where applicable, renaming variables for clarity, and refactoring methods to reduce redundancy.

Codebase Improvements:

  • Made _output and _executionIds readonly in MSBuildHandler and TestApplication classes respectively to ensure immutability. [1] [2]
  • Renamed variables from modules to projects to better reflect their purpose in the MSBuildHandler and MSBuildUtility classes. [1] [2] [3] [4] [5]
  • Refactored RunAsync and related methods in TestApplication to use TestOptions instead of multiple parameters, simplifying method signatures and improving readability. [1] [2] [3] [4] [5]
  • Updated BuildOptions and TestOptions records in Options.cs to include new fields and improve clarity.
  • Modified GetProjectProperties in SolutionAndProjectUtility to accept globalProperties parameter, enhancing flexibility.

Relates to #45927

@mariam-abdulla mariam-abdulla requested a review from a team as a code owner January 31, 2025 15:09
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Request triage from a team member labels Jan 31, 2025
@mariam-abdulla mariam-abdulla enabled auto-merge (squash) January 31, 2025 15:12
@mariam-abdulla mariam-abdulla added Area-dotnet test and removed untriaged Request triage from a team member labels Jan 31, 2025
@mariam-abdulla mariam-abdulla changed the title Use dotnet run in case of arch Use dotnet run in case of --arch, otherwise dotnet exec Jan 31, 2025
@mariam-abdulla mariam-abdulla changed the title Use dotnet run in case of --arch, otherwise dotnet exec Use dotnet run in case of --arch in dotnet test Jan 31, 2025
Copy link
Member

@Evangelink Evangelink left a comment

Choose a reason for hiding this comment

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

I don't see tests added for architecture with and without filter. Are the tests already existing?

@mariam-abdulla
Copy link
Member Author

I don't see tests added for architecture with and without filter. Are the tests already existing?

Without filter, yes (we have RunTestProjectSolutionWithArchOption_ShouldReturnZeroAsExitCode)
With filter, we still need to discuss this part. So, I still haven't added tests for it.

@mariam-abdulla mariam-abdulla merged commit b4b2390 into main Feb 3, 2025
37 checks passed
@mariam-abdulla mariam-abdulla deleted the dev/mabdullah/use-dotnet-run-when-arch-is-passed-in-dotnet-test branch February 3, 2025 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants