search

SteveGilham / altcover

Cross-platform coverage gathering and processing tool set for dotnet/.Net Framework and Mono
MIT License
498 stars 18 forks source link

MSBuild Tasks Support for Multi-Targeting Test Projects #196

Closed UtkarshPhilips closed 11 months ago

UtkarshPhilips commented 1 year ago

I've been recently migrating a few of my unittest projects, for a netstandard2.0 main project, to multi-target net48 and net6.0 for assuring compatibility. Since I'm using the Altcover Prepare and Collect targets, I'd love to see them support multi-targeting projects out-of-the-box.

My current setup:

<!-- Main Project (Example.csproj) -->
<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <Title>Example</Title>
    </PropertyGroup>

    <PropertyGroup>
        <TargetFramework>netstandard2.0</TargetFramework>
        <OutputType>Library</OutputType>
    </PropertyGroup>

    <ItemGroup>
        <PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
    </ItemGroup>

</Project>
<!-- Test Project (Example.Tests.csproj) -->
<Project Sdk="Microsoft.NET.Sdk" DefaultTargets="Coverage">

    <PropertyGroup>
        <Title>Example Tests</Title>
    </PropertyGroup>

    <PropertyGroup>
        <TargetFrameworks>net6.0;net48</TargetFrameworks>
        <OutputType>Library</OutputType>
        <TestProjectType>UnitTest</TestProjectType>
    </PropertyGroup>

    <ItemGroup>
        <ProjectReference Include="..\Example\Example.csproj" />
    </ItemGroup>

    <ItemGroup>
        <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.7.2" />
        <PackageReference Include="altcover" Version="8.6.68" />
        <PackageReference Include="ReportGenerator" Version="5.1.25" />
    </ItemGroup>

    <ItemGroup>
        <UnitTestAssemblies Include="$(AssemblyName).dll" />
    </ItemGroup>

    <PropertyGroup>
        <CoverageDirectory>$(MSBuildProjectDirectory)\Coverage</CoverageDirectory>
    </PropertyGroup>

    <Target Name="Coverage" DependsOnTargets="Restore;Build">
        <PropertyGroup>
            <InstrumentationDirectory>$(MSBuildProjectDirectory)\$(OutputPath)\__Instrumented</InstrumentationDirectory>
            <CoverageFilePath>$(CoverageDirectory)\AltCoverResults.xml</CoverageFilePath>
            <VSTestPath>$(MSBuildToolsPath)\..\..\..\..\Common7\IDE\CommonExtensions\Microsoft\TestWindow\vstest.console.exe</VSTestPath>
        </PropertyGroup>

        <ItemGroup>
            <CallContext Include="0" />
            <CallContext Include="[Fact]" />
            <TestWorkingDirectories Include="$(MSBuildProjectDirectory)\$(OutputPath)" />
            <InstrumentationDirectories Include="$(InstrumentationDirectory)" />
            <ReportFiles Include="$(CoverageFilePath)" />
        </ItemGroup>

        <AltCover.Prepare AssemblyFilter="?KMHID$" InputDirectories="@(TestWorkingDirectories)" OutputDirectories="@(InstrumentationDirectories)" Report="$(CoverageFilePath)" CallContext="@(CallContext)" />
        <Exec Command="&quot;$(VSTestPath)&quot; @(UnitTestAssemblies -> '&quot;$(InstrumentationDirectory)\%(filename)%(extension)&quot;', ' ') /ResultsDirectory:&quot;$(CoverageDirectory)&quot;" />
        <AltCover.Collect RecorderDirectory="%(InstrumentationDirectories.Identity)" />
        <ReportGenerator ProjectDirectory="$(MSBuildProjectDirectory)" ReportFiles="@(ReportFiles)" TargetDirectory="$(CoverageDirectory)" ReportTypes="MarkdownSummaryGithub" />
    </Target>

</Project>

Running the coverage target on my test project gives me the following output:

C:\Projects\ExampleSln\Example\Example.Tests.csproj(38,9): error MSB4036: The "AltCover.Prepare" task was not found. Check the following: 1.) The name of the task in the project file is the same as the name of the task class. 2.) The task class is "public" and implements the Microsoft.Build.Framework.ITask interface. 3.) The task is correctly declared with <UsingTask> in the project file, or in the *.tasks files located in the "C:\Program Files\Microsoft Visual Studio\2022\Community\MSBuild\Current\Bin\amd64" directory. [C:\Projects\ExampleSln\Example\Example.Tests.csproj]

Looking into the generated obj\Example.Tests.csproj.nuget.g.targets file, I can see the relevant import as follows:

<?xml version="1.0" encoding="utf-8" standalone="no"?>
<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <ImportGroup Condition=" '$(TargetFramework)' == 'net48' AND '$(ExcludeRestorePackageImports)' != 'true' ">
    <Import Project="$(NuGetPackageRoot)microsoft.net.test.sdk\17.7.2\build\net462\Microsoft.NET.Test.Sdk.targets" Condition="Exists('$(NuGetPackageRoot)microsoft.net.test.sdk\17.7.2\build\net462\Microsoft.NET.Test.Sdk.targets')" />
    <Import Project="$(NuGetPackageRoot)altcover\8.6.68\build\netstandard2.0\AltCover.targets" Condition="Exists('$(NuGetPackageRoot)altcover\8.6.68\build\netstandard2.0\AltCover.targets')" />
  </ImportGroup>
  <ImportGroup Condition=" '$(TargetFramework)' == 'net6.0' AND '$(ExcludeRestorePackageImports)' != 'true' ">
    <Import Project="$(NuGetPackageRoot)microsoft.net.test.sdk\17.7.2\build\netcoreapp3.1\Microsoft.NET.Test.Sdk.targets" Condition="Exists('$(NuGetPackageRoot)microsoft.net.test.sdk\17.7.2\build\netcoreapp3.1\Microsoft.NET.Test.Sdk.targets')" />
    <Import Project="$(NuGetPackageRoot)altcover\8.6.68\build\netstandard2.0\AltCover.targets" Condition="Exists('$(NuGetPackageRoot)altcover\8.6.68\build\netstandard2.0\AltCover.targets')" />
  </ImportGroup>
</Project>

Since I have no TargetFramework set in outer build due to multi-targeting. I suspected, the Prepare and Collect targets were not imported.

To test this, I manually imported the targets in the csproj and they started to work right away.

I found a relevant issue raised in MSBuild for cross-targeting: https://github.com/dotnet/msbuild/issues/1860

According to the resolution from the issue, it seems moving targets to buildCrossTargeting allows them to be used in cross-targeting projects.

Is this a viable path for AltCover as well? If not, can the imports be fixed without explicitly importing the targets file?

Thanks

SteveGilham commented 1 year ago

For multi-targeted projects, out-of-the-box support is currently provided through project-level target injection via NuGet, and relies on that being picked up under dotnet test as a calling framework to provide the $TargetFrameworks and current $TargetFramework. Those values are used, where appropriate, just to add a label to the coverage output file.

Not knowing how you are invoking the Prepare and Collect tasks, it is difficult to advise in detail; but from what you have provided, if you are using AltCover.targets across the board, it might as well be imported unconditionally, leaving any resolution of $TargetFrameworks and $TargetFramework to the point of use; and as you are quoting the path to the targets file, if you are indeed simply explicitly invoking those tasks, you might as well cut out the indirection, and include them directly

  <UsingTask TaskName="AltCover.Prepare"
        AssemblyFile="$(NuGetPackageRoot)altcover\8.6.68\lib\netstandard2.0/AltCover.Engine.dll" />
  <UsingTask TaskName="AltCover.Collect"
        AssemblyFile="$(NuGetPackageRoot)altcover\8.6.68\lib\netstandard2.0/AltCover.Engine.dll" />

as there is nothing there that depends on the target framework

SteveGilham commented 1 year ago

Thank you for this report - it has taught me something about NuGet documentation. It seems the only references to buildCrossTargeting and buildMultiTargeting to be found are in various GitHub issues that I would otherwise have never had reason to investigate.

This is a pre-release build that moves the .targets and .props file up out of the netstandard20 folder. That it built means that it passed all my test cases, including multi-targeting, but I would like you to try it as well to see if it avoids having to use the workrounds posted above.

altcover.8.6.85-pre-g4b6a1b95f1.nupkg.zip

SteveGilham commented 11 months ago

After much experimenting, I am forced to advise that this is not fixable without breaking all other MSBuild (dotnet test) integration. Specifically, while a .nuget.g.targets file can have an extra 

  <ImportGroup Condition=" '$(TargetFramework)' == '' AND '$(ExcludeRestorePackageImports)' != 'true' ">
    ...
    <Import Project="$(NuGetPackageRoot)altcover\8.6.68\build\netstandard2.0\AltCover.targets" Condition="Exists('$(NuGetPackageRoot)altcover\8.6.68\buildMultiTargeting\AltCover.targets')" />
  </ImportGroup>

clause, pointing at a different .targets file, the whole .nuget.g.targets only gets actioned once when it exists, but the existing multi-targeting for dotnet test relies on it being actioned per-project.

I'm afraid that it has to come down to manually adding something like this to the project file -

  <UsingTask TaskName="AltCover.Prepare"
        AssemblyFile="$(NuGetPackageRoot)altcover\$(AltCoverVersion)\lib\netstandard2.0/AltCover.Engine.dll" />
  <UsingTask TaskName="AltCover.Collect"
        AssemblyFile="$(NuGetPackageRoot)altcover\$(AltCoverVersion)\lib\netstandard2.0/AltCover.Engine.dll" />

where $(AltCoverVersion) is set in one place to be updated per new release.

SteveGilham commented 11 months ago

Labelled "wontfix" but truly it's "can't fix". Sorry.