Improve MTP dotnet test error reporting#52911
Conversation
| // be slowing us down unnecessarily. | ||
| // Alternatively, if we can enqueue right after every project evaluation without waiting all evaluations to be done, we can enqueue early. | ||
| actionQueue = new TestApplicationActionQueue(degreeOfParallelism, buildOptions, testOptions, _output, OnHelpRequested); | ||
| if (!msBuildHandler.EnqueueTestApplications(actionQueue)) |
There was a problem hiding this comment.
This used to return false when we have any VSTest projects. After the changes in the PR, we will report the error having VSTest projects early in the RunMSBuild call above. So EnqueueTestApplications no longer need to return anything as it never fails.
There was a problem hiding this comment.
Pull request overview
This PR improves dotnet test (Microsoft Testing Platform / MTP) failure reporting by surfacing more specific MSBuild exit codes and adding a dedicated error when no test projects are discovered, along with corresponding localized strings.
Changes:
- Propagate MSBuild/restore exit codes through MTP project discovery to improve error output accuracy.
- Fail with a specific “No test projects were found.” error when project discovery yields zero test projects.
- Add the new string to
CliCommandStrings.resxand update allCliCommandStrings.*.xlflocalization files.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Cli/dotnet/Commands/Test/MTP/MicrosoftTestingPlatformTestCommand.cs | Adjusts enqueue flow to rely on RunMSBuild() for failure conditions (no longer checks enqueue return value). |
| src/Cli/dotnet/Commands/Test/MTP/MSBuildUtility.cs | Changes build/restore result from bool to int exit code and propagates it to callers. |
| src/Cli/dotnet/Commands/Test/MTP/MSBuildHandler.cs | Adds “no test projects found” error path; updates error reporting to include actual MSBuild exit code; changes enqueue method signature. |
| src/Cli/dotnet/Commands/CliCommandStrings.resx | Adds CmdTestNoTestProjectsFound resource (and includes unrelated header whitespace reformat). |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.cs.xlf | Adds localized entry for CmdTestNoTestProjectsFound. |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.de.xlf | Adds localized entry for CmdTestNoTestProjectsFound. |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.es.xlf | Adds localized entry for CmdTestNoTestProjectsFound. |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.fr.xlf | Adds localized entry for CmdTestNoTestProjectsFound. |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.it.xlf | Adds localized entry for CmdTestNoTestProjectsFound. |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.ja.xlf | Adds localized entry for CmdTestNoTestProjectsFound. |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.ko.xlf | Adds localized entry for CmdTestNoTestProjectsFound. |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.pl.xlf | Adds localized entry for CmdTestNoTestProjectsFound. |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.pt-BR.xlf | Adds localized entry for CmdTestNoTestProjectsFound. |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.ru.xlf | Adds localized entry for CmdTestNoTestProjectsFound. |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.tr.xlf | Adds localized entry for CmdTestNoTestProjectsFound. |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.zh-Hans.xlf | Adds localized entry for CmdTestNoTestProjectsFound. |
| src/Cli/dotnet/Commands/xlf/CliCommandStrings.zh-Hant.xlf | Adds localized entry for CmdTestNoTestProjectsFound. |
| private readonly BuildOptions _buildOptions = buildOptions; | ||
|
|
||
| private readonly ConcurrentBag<ParallelizableTestModuleGroupWithSequentialInnerModules> _testApplications = []; | ||
| private bool _areTestingPlatformApplications = true; |
There was a problem hiding this comment.
This used to track whether we have any VSTest test projects. Before the PR, this would be set to false during InitializeTestApplications which is called by RunMSBuild, and then checked in EnqueueTestApplications. Now all is aligned to happen in the same place.
| (IEnumerable<ParallelizableTestModuleGroupWithSequentialInnerModules> projects, int buildExitCode) = isSolution ? | ||
| MSBuildUtility.GetProjectsFromSolution(projectOrSolutionFilePath, _buildOptions) : | ||
| MSBuildUtility.GetProjectsFromProject(projectOrSolutionFilePath, _buildOptions); |
There was a problem hiding this comment.
GetProjectsProperties was mostly this single line. So I inlined it.
| (IEnumerable<ParallelizableTestModuleGroupWithSequentialInnerModules> projects, int buildExitCode) = isSolution ? | ||
| MSBuildUtility.GetProjectsFromSolution(projectOrSolutionFilePath, _buildOptions) : | ||
| MSBuildUtility.GetProjectsFromProject(projectOrSolutionFilePath, _buildOptions); |
There was a problem hiding this comment.
GetProjectsProperties was mostly this single line. So I inlined it.
| if (buildExitCode != 0) | ||
| { | ||
| Reporter.Error.WriteLine(string.Format(CliCommandStrings.CmdMSBuildProjectsPropertiesErrorDescription, ExitCode.GenericFailure)); | ||
| Reporter.Error.WriteLine(string.Format(CliCommandStrings.CmdMSBuildProjectsPropertiesErrorDescription, buildExitCode)); |
There was a problem hiding this comment.
We previously reported GenericFailure regardless of the actual build exit code because we only had a bool indicating success/failure. This is fixed now so that we flow the build exit code.
| if (!_areTestingPlatformApplications) | ||
| if (_testApplications.IsEmpty) | ||
| { | ||
| Reporter.Error.WriteLine(string.Format(CliCommandStrings.CmdTestNoTestProjectsFound)); |
There was a problem hiding this comment.
Now we have a separate error for when there are no test projects.
4f0bdcb to
1e14d90
Compare
|
Build analysis seems to be stuck. Let me close and re-open for a fresh build |
Shall we add some link to some doc to explain what is the condition for a project to be considered as test project?
We still see the |
Let's consider in a follow-up. Some of the details of our detection are too technical for normal users. We definitely want to balance the level of technical details with what end users are concerned about, and I expect we will be doing major updates to our documentation. So we can revisit after.
I agree and I never liked this message. I'm going to simplify it to |
Fixes #51802
For all screenshots, left is "dogfood" (this PR - the new behavior)
Comparison when given a project that is not a test project
The case of passing VSTest project
The case of having a build error
Same experience, except that we capture the real exit code of MSBuild in case it's something other than "1".