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.