Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AltCover.Prepare copies wrong FSharp.Core into Output directory #68

Closed
matthid opened this issue Aug 26, 2019 · 6 comments
Closed

AltCover.Prepare copies wrong FSharp.Core into Output directory #68

matthid opened this issue Aug 26, 2019 · 6 comments

Comments

@matthid
Copy link

matthid commented Aug 26, 2019

I'm not quite sure why but our build started failing and I can basically pin it down to the AltCover.Prepare task copying a wrong FSharp.Core.dll into the output directory. This issue either appeared when updating packages or when changing the target framework from net461 to netcoreapp2.2

Basically we included AltCover like this (in a separate targets file we include):

  <Target Name="CollectExpectoCoverage" DependsOnTargets="Restore;BuildProject">
    <!-- Altcover fails if _Reports folder doesn't exist -->
    <MakeDir
          Directories="$(OutputPath)/_Reports"/>
    <ItemGroup>
        <InputDirectory Include="$(OutputPath)" />
        <FileFilter Include="OurCompany+" />
        <OutputDirectory Include="$(OutputPath)/__Saved$([System.DateTime]::UtcNow.ToString().Replace(':','-').Replace('/','-').Replace(' ','+'))" />
    </ItemGroup>
    <AltCover.Prepare
      InputDirectories="@(InputDirectory)"
      OutputDirectories="@(OutputDirectory)"
      FileFilter="@(FileFilter)"
      XmlReport="$(ProjectDir)coverage.xml"
      OpenCover="true"
      />
    <Delete Files="$(OutputPath)/FSharp.Core.dll" /><!-- Workaround this issue -->
    <Exec Command="dotnet run -c $(Configuration) -f $(TargetFramework) --no-build -- --summary" ContinueOnError="WarnAndContinue"/>
    <PropertyGroup>
      <DotNetRunHasFailed Condition=" '$(MSBuildLastTaskResult)' != 'true' ">true</DotNetRunHasFailed>
      <DotNetRunHasFailed Condition=" '$(MSBuildLastTaskResult)' == 'true' ">false</DotNetRunHasFailed>
    </PropertyGroup>
    <AltCover.Collect RecorderDirectory="$(OutputPath)" />
    <Error
            Text="previous dotnet run failed."
            Condition="'$(DotNetRunHasFailed)' == 'true'" />
  </Target>

and call it like this:

dotnet build -c Release /t:CollectExpectoCoverage 

Result (when you delete the Workaround line):

  Instrumenting files in C:\proj\MyCompany.TestProject\source\Server.Tests\bin\Release\netcoreapp2.2\
     => C:\proj\MyCompany.TestProject\source\Server.Tests\bin\Release\netcoreapp2.2\__Saved2019-08-26+16-18-46\MyCompany.TestProject.Server.dll
     => C:\proj\MyCompany.TestProject\source\Server.Tests\bin\Release\netcoreapp2.2\__Saved2019-08-26+16-18-46\MyCompany.TestProject.Server.Tests.dll

  Coverage Report: C:\proj\MyCompany.TestProject\source\Server.Tests\coverage.xml


      C:\proj\MyCompany.TestProject\source\Server.Tests\bin\Release\netcoreapp2.2\MyCompany.TestProject.Server.dll
                  <=  MyCompany.TestProject.Server, Version=0.0.1.0, Culture=neutral, PublicKeyToken=null
      C:\proj\MyCompany.TestProject\source\Server.Tests\bin\Release\netcoreapp2.2\MyCompany.TestProject.Server.Tests.dll
                  <=  MyCompany.TestProject.Server.Tests, Version=0.0.1.0, Culture=neutral, PublicKeyToken=null

  Unhandled Exception:
     Cannot print exception string because Exception.ToString() failed.
C:\proj\MyCompany.TestProject\source\Coverage.targets(27,5): warning MSB3073: The command "dotnet run -c Release -f netcoreapp2.2 --no-build -- --summary" exited with code -532462766. [C:\proj\MyCompany.TestProject\source\Server.Tests\MyCompany.TestProject.Server.Tests.fsproj]
C:\proj\MyCompany.TestProject\source\Coverage.targets(32,5): warning : A total of 0 visits recorded [C:\proj\MyCompany.TestProject\source\Server.Tests\MyCompany.TestProject.Server.Tests.fsproj]
  Coverage statistics flushing took 3.68 seconds
  Visited Classes 0 of 664 (0)
  Visited Methods 0 of 1074 (0)
  Visited Points 0 of 2616 (0)
  Visited Branches 0 of 1812 (0)

  ==== Alternative Results (includes all methods including those without corresponding source) ====
  Alternative Visited Classes 0 of 676 (0)
  Alternative Visited Methods 0 of 1242 (0)
C:\proj\MyCompany.TestProject\source\Coverage.targets(33,5): error : previous dotnet run failed. [C:\proj\MyCompany.TestProject\source\Server.Tests\MyCompany.TestProject.Server.Tests.fsproj]

Build FAILED.

Note: A regular "build" doesn't copy any FSharp.Core into the output directory, we have the nuget package 4.6.2 referenced and 4.5.0 is copied to the directory.

Note2: To Debug

  Unhandled Exception:
     Cannot print exception string because Exception.ToString() failed.

I run the project with the debugger in the above state and the issue is a regular exception about unable to find the correct assembly. The bad reporting is not related to AltCover (which I initially thought) but can be reproduced by just running dotnet run --no-build in the above state.

@SteveGilham
Copy link
Owner

I think I see the issue here -- the defensive copying of FSharp.Core is based on .net framework world thinking; there needs to be a check for whether any of the assemblies being instrumented already reference it out of the nuget cache.

Also, I need to fix the <!-- Altcover fails if _Reports folder doesn't exist -->

@matthid
Copy link
Author

matthid commented Aug 26, 2019

Let me know if you need additional info, currently I think this should be straightforward to reproduce but maybe not

Regarding the defensive copy: If it isn't there we shouldn't copy it and if it is there we should copy only the exact same version.
Also: If we decide to copy it - we should never copy a version older than the latest reference / error out in that situation.

@SteveGilham
Copy link
Owner

Pre-release build 6.0.703-pre available for testing. Should address both issues, the unnecessary copying of FSharp.Core, and the need to create the report directory in advance.

@matthid
Copy link
Author

matthid commented Aug 26, 2019

Wow that was fast :) Will probably take me a bit to test

@SteveGilham
Copy link
Owner

The change is in the new release, too.

@matthid
Copy link
Author

matthid commented Aug 28, 2019

Indeed it seems to solve the problem, thanks for the fast release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants