DotNetCoreCLI@2 test does not use enhanced reporting for compile errors

[aolszowka]

When using DotNetCoreCLI@2 in test command mode you have an implicit restore/build step, if your build fails you will not get enhanced error logging in the build logs.

It is believed that this is due to failing to register the MSBuild Logger in cases that are not explicitly a build as seen here:

azure-pipelines-tasks/Tasks/DotNetCoreCLIV2/dotnetcore.ts

Lines 109 to 112 in d8dd680

	if (this.isBuildCommand()) {
	var loggerAssembly = path.join(__dirname, 'dotnet-build-helpers/Microsoft.TeamFoundation.DistributedTask.MSBuild.Logger.dll');
	dotnet.arg(`-dl:CentralLogger,\"${loggerAssembly}\"*ForwardingLogger,\"${loggerAssembly}\"`);
	}

Required Information

Question, Bug, or Feature?
Type: Bug

Enter Task Name: DotNetCoreCLI@2

Environment

• Server - Azure Pipelines
• Agent - Hosted

Pool: Azure Pipelines
Image: ubuntu-latest
Agent: Hosted Agent

Issue Description / Repro Steps

Consider the following:

azure-pipelines.yml

steps:
  - task: DotNetCoreCLI@2
    displayName: 'dotnet test'
    inputs:
      command: 'test'
      projects: '**/Test.csproj'
      arguments: '/p:CollectCoverage=true /p:CoverletOutputFormat=cobertura /p:CoverletOutput=./coverage/'
      publishTestResults: true

Test.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <IsPackable>false</IsPackable>
    <EnableNETAnalyzers>True</EnableNETAnalyzers>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="6.0.0">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.1.0" />
    <PackageReference Include="MSTest.TestAdapter" Version="2.2.8" />
    <PackageReference Include="MSTest.TestFramework" Version="2.2.8" />
    <PackageReference Include="coverlet.msbuild" Version="3.1.2">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
    <PackageReference Include="coverlet.collector" Version="3.1.2">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
  </ItemGroup>

</Project>

UnitTest1.cs

namespace DotNetCoreCLI_Bugs
{
    using Microsoft.VisualStudio.TestTools.UnitTesting;

    [TestClass]
    public class UnitTest1
    {
        [TestMethod]
        public void TestMethod1()
        {
            Random random = new Random();
            random.Next(); //Trigger CA5394
            Assert.AreEqual(true, true);
        }
    }
}

.editorconfig

[*.{cs,vb}]
dotnet_diagnostic.CA5394.severity = error

The above is a bare bones project that creates an MSTest based unit test that will trigger CA5394 and a bare bones YAML based pipeline that will attempt to invoke this using the DotNetCoreCLI@2 using the test command.

When running this in a private Azure DevOps Project I see the following:

At least it shows that it fails but it doesn't say why without digging into the verbose logs; even then it is not highlighted in red (I have boxed the first instance of the error in red):

Compare this to when you use build command (using this YAML):

steps:
  - task: DotNetCoreCLI@2
    displayName: 'dotnet build'
    inputs:
      command: 'build'
      projects: '**/Test.csproj'

You are shown right on the build summary page the error:

Along with on the detailed log this is kicked out:

It is believed that this is because the custom MSBuild Logger is being registered by the task when build is selected as indicated in the summary.

The behavior, in my opinion, should be identical in all cases for the best UX.

[BryceBarbara]

I've been dealing with the same problems. Thanks for reporting this and diving in deep. Hopefully we can get some attention on this soon enough

[github-actions]

This issue is stale because it has been open for 180 days with no activity. Remove the stale label or comment on the issue otherwise this will be closed in 5 days

[aolszowka]

Bumping to avoid closing due to Stale Issue; This is still present.

[bramborman]

I'm also missing the logger... I think it should be on for every dotnet command supporting it, being it restore, build, publish, test, etc...

This would be a quick fix IMHO, one would just need to investigate which commands do support this... I think I'll just look into this and open a PR.

We could also try to add the logger manually, either by finding the task definition location, or more easily manually download the NPM package containing the logger during the build, and then using that...

[aolszowka]

I'm also missing the logger... I think it should be on for every dotnet command supporting it, being it restore, build, publish, test, etc...

Be extremely careful when adding the logger blindly, I think you touched on it in your last sentence but to be extremely clear on this: ensure that whatever solution you end up with does not result in a "chicken-and-egg" scenario. Consider the example of a custom logger being shipped as a NuGet Package Reference that won't be populated until a dotnet restore is completed.

[bramborman]

I'm also missing the logger... I think it should be on for every dotnet command supporting it, being it restore, build, publish, test, etc...

Be extremely careful when adding the logger blindly, I think you touched on it in your last sentence but to be extremely clear on this: ensure that whatever solution you end up with does not result in a "chicken-and-egg" scenario. Consider the example of a custom logger being shipped as a NuGet Package Reference that won't be populated until a dotnet restore is completed.

I think of it as a workaround that would be done in my pipeline only, and it would involve downloading the NPM package these build tasks are using, and extracting the DLL from that, basically doing the same as is done for the build command, but done manually.

But I'd of course rather have it built-in, but as this issue did not get any attention from the team and the fix seems pretty simple, I'll try opening a PR myself.

[bramborman]

Ah, nevermind... I once again lost hope in this task, as it does some black magic modifying the output paths I specify... I think we need V3 tasks for .NET... Some that would not do such unexpected and unwanted things like appending things to the output path...

As for the workaround, I found where the DLL is located (not in NPM package after all):

azure-pipelines-tasks/Tasks/DotNetCoreCLIV2/make.json

Line 5 in cccb1c6

"url": "https://vstsagenttools.blob.core.windows.net/tools/msbuildlogger/3/msbuildlogger.zip",

So guess we can download this ZIP file (the link has not changed for almost three years now), extract the DLL and append the -dl param ourselves...

We'd use it somehow like this:

dotnet publish -dl:CentralLogger,"LOGGER_PATH"*ForwardingLogger,"LOGGER_PATH"

I'm not gonna try this myself now - it's not verified it will work, and of course, it's not good to rely on some random link. One would need to do at least a proper error handling in case the link does not work anymore...

[bramborman]

We'd use it somehow like this:

dotnet publish -dl:CentralLogger,"LOGGER_PATH"*ForwardingLogger,"LOGGER_PATH"

I'm not gonna try this myself now - it's not verified it will work, and of course, it's not good to rely on some random link. One would need to do at least a proper error handling in case the link does not work anymore...

So in the end I decided to give it a try and succeeded. To prevent issues if the link stops working, I downloaded the ZIP file and saved it to an internal location. I append the -dl parameter to every call to the dotnet executable, being it restore, pack, build, etc., and it just works and extracts the warnings and errors to the summary of the build run.

[github-actions]

This issue is stale because it has been open for 180 days with no activity. Remove the stale label or comment on the issue otherwise this will be closed in 5 days

[aolszowka]

The issue is still outstanding and therefore not stale, its unclear if the Team has even triaged this?

[KirillOsenkov]

@max-zaytsev @DenisRumyantsev @anatolybolshakov

[aolszowka]

Good to see you again @KirillOsenkov maybe some of my outstanding bugs will get resolved or observed finally! Hope all is well on your end :)

[KirillOsenkov]

Good to see you too @aolszowka! Things are good. I’ve been thinking that the logger should be rewritten like this:

https://github.com/KirillOsenkov/MSBuildTools/tree/main/src/ErrorLogger

It doesn’t need to be central/distributed and I don’t think it needs to track project nesting, orphans etc for the logdetail stuff.

And you’re right, we need to pass the logger during restore as well of course.

[github-actions]

This issue is stale because it has been open for 180 days with no activity. Remove the stale label or comment on the issue otherwise this will be closed in 5 days

[aolszowka]

Hey There... Been Awhile...

This is still failing; I've got this in my Azure DevOps Playground Instance, I'd be more than happy to share this if it would get this addressed.

Here's a screenshot from today:

I get priorities are hard, layoffs, restructuring, attempting to do yet another issue reporting system to paper over the huge backlog yet again, etc...

While I'm here I might as well give another shout to one of my all time favorite developers (next to Raymond Chen) @KirillOsenkov hope all is well on that end, you are not recognized enough ;)

[KirillOsenkov]

Lol @aolszowka if anything, I love that this issue gives us a periodic opportunity to catch up :) I'll try to ping some folks internally to see if we can get attention to this issue.