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

Exit if restore/build failure in dotnet test #47450

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mariam-abdulla
Copy link
Member

@mariam-abdulla mariam-abdulla commented Mar 11, 2025

This pull request includes changes to the GetProj and GetProjectsFromProject methods in the MSBuildUtility.cs file to improve the handling of build or restore failures. The most important changes include adding early return statements when the build or restore process fails, ensuring that an empty array is returned in such cases.

Changes to build or restore handling:

Relates to #45927
Fixes microsoft/testfx#5203

@Copilot Copilot bot review requested due to automatic review settings March 11, 2025 11:04
@mariam-abdulla mariam-abdulla requested a review from a team as a code owner March 11, 2025 11:04
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-dotnet test untriaged Request triage from a team member labels Mar 11, 2025
@mariam-abdulla mariam-abdulla requested a review from nohwnd March 11, 2025 11:04

Choose a reason for hiding this comment

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

PR Overview

This PR ensures that dotnet test exits early if a restore/build failure occurs in order to prevent further processing when the build state is invalid.

  • Added early exit in GetProjectsSolution if build/restore fails
  • Added early return in GetProjectsFromProject on build/restore failure
  • Removed aggregated status update using projects' existence to determine the build state

Reviewed Changes

File Description
src/Cli/dotnet/commands/dotnet-test/MSBuildUtility.cs Early exit on build/restore failure and removal of additional built state checks

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

Comments suppressed due to low confidence (2)

src/Cli/dotnet/commands/dotnet-test/MSBuildUtility.cs:32

  • The removal of the projects emptiness check changes the behavior compared to before. Confirm that the build status should rely solely on the initial build/restore result rather than also considering whether any projects were found.
isBuiltOrRestored |= !projects.IsEmpty;

src/Cli/dotnet/commands/dotnet-test/MSBuildUtility.cs:45

  • With the early return on restore/build failure, the check using projects.Any() has been removed. Verify that not updating the isBuiltOrRestored flag based on the presence of projects is the intended behavior.
IEnumerable<TestModule> projects = SolutionAndProjectUtility.GetProjectProperties(projectFilePath, GetGlobalProperties(buildOptions.BuildProperties), new ProjectCollection());
@mariam-abdulla mariam-abdulla removed the untriaged Request triage from a team member label Mar 11, 2025
@mariam-abdulla mariam-abdulla enabled auto-merge (squash) March 11, 2025 11:37
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.

new dotnet test does not handle build failure when TargetFramework is used with multiple tfms
1 participant