Improve MSTEST0001 message

[gao-artur]

When building a solution with quite logging (-v q) we get a bunch of MSTEST0001 warnings without a reference to the project name. In a solution with dozens of test projects, it's very difficult to find out the projects to fix

CSC : warning MSTEST0001: Explicitly enable or disable tests parallelization (https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0001)
CSC : warning MSTEST0001: Explicitly enable or disable tests parallelization (https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0001)
CSC : warning MSTEST0001: Explicitly enable or disable tests parallelization (https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0001)
CSC : warning MSTEST0001: Explicitly enable or disable tests parallelization (https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0001)
CSC : warning MSTEST0001: Explicitly enable or disable tests parallelization (https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0001)

Please add a reported project name to the error message.

[Youssef1313]

@baronfel @rainersigwald This is a warning from Roslyn analyzer. Is it expected from MSBuild point of view that the project name/path isn't shown after the CSC warning?

[baronfel]

I'd be curious to see the binlogs that generate this diagnostic - if it's coming from a Roslyn analyzer then those should inherently belong to a project, so I'm interesting in what's doing the reporting of the diagnostic itself.

[rainersigwald]

My first thought is that the terminal logger uses indentation grouping on this sort of warning, rather than the old suffix project--I agree that a binlog would be super helpful.

[gao-artur]

Here is a new project created from the MSTest template without MSTestSettings.cs file + the binlog. Built with

dotnet build -bl -v q

TestProject11.zip

[gao-artur]

Interestingly, in the binlog the message contains the file path

CSC warning MSTEST0001: Explicitly enable or disable tests parallelization (https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0001) [C:\temp\TestProject11\TestProject11\TestProject11.csproj]

[gao-artur]

@rainersigwald, you were right. With a terminal logger disabled, the path is shown in the message

dotnet build -v q --tl:off

CSC : warning MSTEST0001: Explicitly enable or disable tests parallelization (https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0001) [C:\temp\TestProject11\TestProject11\TestProject11.cs proj]

[baronfel]

I think this might actually be something that we need Roslyn to dig into a bit. For comparison to the analyzer-reported diagnostic, here's a simple Warning I added to your test project in non-quiet mode:

➜  TestProject11 dotnet build
Restore complete (0.4s)
  TestProject11 net9.0 succeeded with 2 warning(s) (0.2s) → bin/Debug/net9.0/TestProject11.dll
    CSC : warning MSTEST0001: Explicitly enable or disable tests parallelization (https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0001)
    /Users/chethusk/Downloads/TestProject11/TestProject11.csproj(12,5): warning FAKE001: This is a fake diagnostic message for testing purposes.

Build succeeded with 2 warning(s) in 0.8s

and quiet mode:

➜  TestProject11 dotnet build -v q
CSC : warning MSTEST0001: Explicitly enable or disable tests parallelization (https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0001)
/Users/chethusk/Downloads/TestProject11/TestProject11.csproj(12,5): warning FAKE001: This is a fake diagnostic message for testing purposes.

It seems like the diagnostics reported from the Csc task get the CSC 'file' segment both in the binlog viewer and in the terminal logger renderer. Since we're just interpreting the results of individual Tasks calling LogWarning/LogError/etc here, I suspect that the Csc Task implementation needs to change a bit in order to give a user a more reasonable display.

[Evangelink]

@jjonescz Could you have a look?

[jjonescz]

To investigate I have created an analyzer which reports a diagnostic once with and once without a location. The csc outputs this:

warning MYDIAG: My message
C:\ConsoleApp1\Program.cs(19,1): warning MYDIAG: My message

The Csc build task then calls MSBuild's TaskLoggingHelper.LogMessageFromText on each line. That method seems to write CSC as the "origin" of the warning. Perhaps that should be fixed instead? Or is there a better MSBuild API csc should use?

[baronfel]

@jjonescz I'd suggest either using a text string that matches MSbuild's canonical error format (https://github.com/dotnet/msbuild/blob/main/src/Shared/TaskLoggingHelper.cs#L1354) or use the LogMessage/LogWarning/LogError overloads that have the billion separate parameters, like this one: https://github.com/dotnet/msbuild/blob/main/src/Shared/TaskLoggingHelper.cs#L352-L363

[rainersigwald]

Taking a step back here:

1. It looks like this analyzer doesn't report the diagnostic with a location. Is that intentional, because it's the whole project that is intended to be flagged? (@evangelink?)
2. There's a loss of important information in terminal-logger quiet mode. Compare

❯ dotnet build
Restore complete (4.0s)
  TestProject11 net9.0 succeeded with 1 warning(s) (2.3s) → bin\Debug\net9.0\TestProject11.dll
    CSC : warning MSTEST0001: Explicitly enable or disable tests parallelization (https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0001)

Build succeeded with 1 warning(s) in 7.2s

❯ rm -r .\obj\

❯ dotnet build -v:q
CSC : warning MSTEST0001: Explicitly enable or disable tests parallelization (https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0001)

The associated project TestProject11 is not mentioned anywhere in the latter. I think that's unreasonable even when the user has requested "quiet".

[Evangelink]

It looks like this analyzer doesn't report the diagnostic with a location. Is that intentional, because it's the whole project that is intended to be flagged? (@Evangelink?)

The analyzer is complaining about non-explicit choice of parallelization that is done via an assembly attribute so we don't have a good location to report hence the no-location.

[rainersigwald]

Filed https://github.com/dotnet/msbuild/issues/12929 to track the TL problem. I don't think there's a compiler issue here, and the test-layer "it's not specific to any C# source file but we can't know about it outside of a C# analyzer" problem seems reasonably solved as-is.

[baronfel]

Here's the TL formatting in quiet mode. This renders the warning and is using the following message properties that impact the rendering from what I can tell:

property	value
file	N/A
lineNumber	N/A
endLineNumber	N/A
columnNumber	N/A
endColumnNumber	N/A
subcategory	CSC
category	warning
code	MSTEST0001
message	Explicitly enable or disable tests parallelization (https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0001)
helpLink	N/A
helpKeyword	N/A

I think in this case, it's reasonable for the analyzer to report a diagnostic that includes

• a file that is the project
• no (end)lineNumber or (end)columnNumber because it's the whole project
• consider setting helpLink to your link - this will enable TL to make the diagnostic code a clickable link automatically

Assuming the analyzer-generated diagnostic contains those properties, the next question is if the translation that the Csc task is doing is preserving those properties. I suspect that we would get better integration here if, in general, the diagnostic/analyzer messages didn't directly use the canonical error format parser (because it doesn't support e.g. helpLink/helpKeyword) and instead had some kind of more capable parser/message communication protocol.

[baronfel]

Ah, @rainersigwald is also correct here - it's not about formatting the message, it's about TL dropping the 'project' context. I do think it's potentially interesting for Roslyn to consider logging diagnostics in a way that could preserve helpLink information, etc, but that's a separate question.