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

Integrate coverlet with VSTest (Microsoft Test Platform) #395

Closed
PBoraMSFT opened this issue May 3, 2019 · 45 comments
Closed

Integrate coverlet with VSTest (Microsoft Test Platform) #395

PBoraMSFT opened this issue May 3, 2019 · 45 comments
Labels
feature-request New feature request

Comments

@PBoraMSFT
Copy link

PBoraMSFT commented May 3, 2019

What would integrating Coverlet with Microsoft Test Platform mean:

  1. Coverlet based coverage solution is available as a data collector that does the instrumentation of the necessary modules before test execution and appropriate restore after.
  2. Authoring – When new test projects are created (dotnet mstest), references to the data collector package are added by default. This reduces adoption friction for customers.
  3. Test execution - Today Coverlet is invoked as a msbuild target and as such only works with dotnet test (and requires a csproj). With the data collector in bullet 1 becoming available, coverage can also be collected when tests are run on built binaries (dotnet vstest)

Edit: Fixed markdown typos. I was referring to bullet 1 in the above description, not the issue number.

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented May 3, 2019

With the data collector in 1 becoming available,

Can we see the implementation?
Could this integration help with this issue #329 ?We're trying to find a way to integrate with VSTest but with a good user experience(RunSettings file is not the best maybe)

Today Coverlet is invoked as a msbuild target and as such only works with dotnet test (and requires a csproj).

We have also dotnet tool, btw integration with dotnet vstest would be great.

We're working on improving/reorganize "internals", this should lead to "stable"(cross finger) interface/contracts usable "outside" as standalone api.
Maybe to integrate with VSTest we need to "refactor"(separate reports and engine) even more and ship coverlet.core.dll as nuget package, that is in line with current refactor plan.

cc: @tonerdo

@MarcoRossignoli MarcoRossignoli added the enhancement General enhancement request label May 3, 2019
@vagisha-nidhi
Copy link
Contributor

@MarcoRossignoli Adding implementation details.

Scenario Explanation :

  1. Integration of coverlet such that it works with dotnet test.
    a. Command line invocation will be : dotnet test --collect:"Xplat Code Coverage"
  2. There will be an outproc datacollector[CoverletDataCollector] with friendly name "Xplat Code Coverage" which will have a dependency on coverlet.core to get coverage data by means of coverlet.core APIs such as coverage.PrepareModules(), coverage.GetCoverageResult().
  3. CoverletDataCollector will be responsible for creating result attachments from CoverageResult object and send coverage result back to test platform using DataCollectionSink.
  4. CoverletDataCollector will be a separate Nuget package which can be consumed by .Net Core unit test projects.
  5. CoverletDataCollector will have a dependency on coverlet.core.
  6. Unit test projects templates will have a package reference to CoverletDataCollector.
<ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.9.0" />
    <PackageReference Include="MSTest.TestAdapter" Version="1.3.2" />
    <PackageReference Include="MSTest.TestFramework" Version="1.3.2" />
    <PackageReference Include="CoverletDataCollector" Version="1.0.0" />   
  </ItemGroup>

Asks :

  1. CoverletDataCollector as a separate nuget package. We could contribute this to coverlet repo.
    This will have a dependency on coverlet.core.
  2. The issue Spurious 'Unable to read beyond end of stream' in CI while collecting results #210 will be addressed alongside as --collect:"Xplat Code Coverage" will add inproc datacollector to runsettings and get this can be picked up from
    CoverletDataCollector nuget.
  3. Getting [WIP] Implement a VSTest data collector to write coverage data to disk #329 PR to completion and getting the
    inprocdatacollector as part of CoverletDataCollector Nuget package.

@MarcoRossignoli
Copy link
Collaborator

Some questions(so you can save me/us some time on docs):

CoverletDataCollector will be responsible for creating result attachments from CoverageResult object and send coverage result back to test platform using DataCollectionSink.

What "results" should send?Coverage output file i.e. cobertura file etc...?

CoverletDataCollector will be a separate Nuget package which can be consumed by .Net Core unit test projects.

How versioning works?I mean if we publish collector package and we need to update(collector self or some transient dependencies) due to bug fix new features, is unit test projects template versionable?

Getting #329 PR to completion and getting the
inprocdatacollector as part of CoverletDataCollector Nuget package.

It's not clear to me why/when use InProcess collector vs out of process one(public class NewDataCollector : DataCollector if I understood correctly), and how out of process can work.
Coverlet need to flush hits files before calculate coverage...at the moment we rely on exit events that seem to lead to issue(corrupted file) when for delay reason VSTest runtime decide to kill process. If we need to call that flush(UnloadModule) method we need to be in-process, do we need to implement some interprocess comunication for out of process collector?

@tonerdo
Copy link
Collaborator

tonerdo commented May 3, 2019

Thanks for elaborating @vagisha-nidhi. I'm happy you're looking to make Coverlet part of the out of the box experience, we definitely intend to work through this with you. Your implementation details seem fine to me. However, I have a few of questions:

Command line invocation will be : dotnet test --collect:"Xplat Code Coverage"

How will users pass extra arguments like coverage format, threshold etc.

CoverletDataCollector as a separate nuget package.

Can this be called coverlet.collector instead? Just to keep it consistent with existing package names

For my learning benefit

What's the difference between an in-proc and out-of-proc datacollector? How does one create both? Which one do we need to make this integration work?

@tonerdo
Copy link
Collaborator

tonerdo commented May 3, 2019

@MarcoRossignoli

What "results" should send?Coverage output file i.e. cobertura file etc...?

I believe it'll be the coverage output file. See here: https://github.com/Microsoft/vstest-docs/blob/master/docs/extensions/datacollector.md#datacollectionsink

@cltshivash
Copy link

Some questions(so you can save me/us some time on docs):

CoverletDataCollector will be responsible for creating result attachments from CoverageResult object and send coverage result back to test platform using DataCollectionSink.

What "results" should send?Coverage output file i.e. cobertura file etc...?

It will be the coverage output file.

CoverletDataCollector will be a separate Nuget package which can be consumed by .Net Core unit test projects.

How versioning works?I mean if we publish collector package and we need to update(collector self or some transient dependencies) due to bug fix new features, is unit test projects template versionable?

There's no versioning for the test template. If a new collector package is pushed, the template will be updated with the newer version. The existing users can update the nuget package to get the latest changes.

Getting #329 PR to completion and getting the
inprocdatacollector as part of CoverletDataCollector Nuget package.

It's not clear to me why/when use InProcess collector vs out of process one(public class NewDataCollector : DataCollector if I understood correctly), and how out of process can work.
Coverlet need to flush hits files before calculate coverage...at the moment we rely on exit events that seem to lead to issue(corrupted file) when for delay reason VSTest runtime decide to kill process. If we need to call that flush(UnloadModule) method we need to be in-process, do we need to implement some interprocess comunication for out of process collector?

We will need both the collectors. The inproc collector (which runs in the testhost process) is needed for the flushing of the hit files to be reliable and not dependent on the exit handler. The outproc collector is needed to setup the test session (instrument the dlls)

@cltshivash
Copy link

Thanks @tonerdo and @MarcoRossignoli for reviewing the enhancement and the comments. We are excited to work with you to enable this integration as it would address the code coverage ask in a seamless way with dotnet test/vstest.

Thanks for elaborating @vagisha-nidhi. I'm happy you're looking to make Coverlet part of the out of the box experience, we definitely intend to work through this with you. Your implementation details seem fine to me. However, I have a few of questions:

Command line invocation will be : dotnet test --collect:"Xplat Code Coverage"

How will users pass extra arguments like coverage format, threshold etc.

There are options to pass runsettings information through cmdline. https://github.com/Microsoft/vstest-docs/blob/master/docs/configure.md. This doesn't work well for datacollector settings under which the coverage format, threshold would fall under. As of now these options would need a runsettings file to specify a value other than the default. We will look addressing the gap here.

CoverletDataCollector as a separate nuget package.

Can this be called coverlet.collector instead? Just to keep it consistent with existing package names

Yes.

For my learning benefit

What's the difference between an in-proc and out-of-proc datacollector? How does one create both? Which one do we need to make this integration work?

The inproc data collector runs in the testhost process executing the tests (testhost*.exe/ testhost.dll). The outproc collector would run in a separate process (datacollector.exe/datacollector.dll). We would need the inproc collector to remove the dependency on the exit handler to flush the hits and would need the outproc collector to setup the test run session (instrument dlls) and restore.

@MarcoRossignoli
Copy link
Collaborator

To start to work on it could be good if some of you(MS guys) write a documentation guide under Documentation/VSTestIntegration.md where we(coverlet guys) can see how is the user experience with dotnet test/vstest and setting files for all types of parameters coverlet uses.
This would be useful for two reason, first we'll have some user documentation ready and we can understand how good/bad is, second allow us to understand if we need to rethink someting, for instance if the tracker generation is ok ASIS.
What do you think @tonerdo cc @petli to keep him up to date.

@vagisha-nidhi
Copy link
Contributor

@MarcoRossignoli That would be a great start. I'll shortly raise a PR on the repo with the documentation. We can discuss the next steps further.

@ViktorHofer
Copy link
Contributor

@vagisha-nidhi What is missing here and when will we be able to try this out with dotnet test?

@MarcoRossignoli
Copy link
Collaborator

FYI @ViktorHofer #410 (comment)

@vagisha-nidhi
Copy link
Contributor

@MarcoRossignoli @tonerdo dotnet sdk version 2.2.300 has been released. You can download it from https://dotnet.microsoft.com/download/dotnet-core/2.2 and try it out. Then we should be able to release the nuget.

@MarcoRossignoli
Copy link
Collaborator

I'll do test ASAP, I found also an issue, on coverlet.core we use Newtownsoft.Json 9.x.x but this version has got a bug and doesn't use converter in a correct way...we didn't hit issue on test because seem that msbuil load version 10.x.x and not referenced one. What does vstest runner load?I'll upgrade to 10.x.x on next PR(I'm working on custom runner for instrumentation tests)

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented May 22, 2019

@vagisha-nidhi @tonerdo doing some tests:

  • ok for --collect:"XPlat Code Coverage" also report is ok.
C:\Users\Marco\Downloads\TMP\TMP\XUnitTestProject1
λ dotnet test --collect:"XPlat Code Coverage"
Test run for C:\Users\Marco\Downloads\TMP\TMP\XUnitTestProject1\bin\Debug\netcoreapp2.2\XUnitTestProject1.dll(.NETCoreApp,Version=v2.2)
Microsoft (R) Test Execution Command Line Tool Version 16.1.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...

Attachments:
  C:\Users\Marco\Downloads\TMP\TMP\XUnitTestProject1\TestResults\57f654f2-7b66-41d1-aa2d-7f0ba8b8df09\coverage.cobertura.xml

Test Run Successful.
Total tests: 1
     Passed: 1
 Total time: 2,2419 Seconds
  • issue with dotnet test --settings coverletArgs.runsettings
<?xml version="1.0" encoding="utf-8" ?>
<RunSettings>
  <DataCollectionRunSettings>
    <DataCollectors>
      <DataCollector friendlyName="XPlat code coverage">
        <Configuration>
          <Format>json</Format>
          <!--<MergeWith>/custom/path/result.json</MergeWith>
          <Exclude>[coverlet.*.tests?]*,[*]Coverlet.Core*</Exclude> --><!-- [Assembly-Filter]Type-Filter --><!--
          <Include>[coverlet.*]*,[*]Coverlet.Core*</Include> --><!-- [Assembly-Filter]Type-Filter --><!--
          <ExcludeByAttribute>Obsolete,GeneratedCodeAttribute,CompilerGeneratedAttribute</ExcludeByAttribute>
          <ExcludeByFile>../dir1/class1.cs,../dir2/*.cs,../dir3/**/*.cs,</ExcludeByFile> --><!-- Absolute or relative file paths --><!--
          <IncludeDirectory>../dir1/*,../dir2/,</IncludeDirectory>
          <SingleHit>false</SingleHit>
          <UseSourceLink>true</UseSourceLink>-->
        </Configuration>
      </DataCollector>
    </DataCollectors>
  </DataCollectionRunSettings>
</RunSettings>
C:\Users\Marco\Downloads\TMP\TMP\XUnitTestProject1
λ dotnet test --settings coverletArgs.runsettings
Test run for C:\Users\Marco\Downloads\TMP\TMP\XUnitTestProject1\bin\Debug\netcoreapp2.2\XUnitTestProject1.dll(.NETCoreApp,Version=v2.2)
Microsoft (R) Test Execution Command Line Tool Version 16.1.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
Data collector 'XPlat code coverage' message: Coverlet.Collector.Utilities.CoverletDataCollectorException: CoverletCoverageDataCollector: Failed to instrument modules ---> System.ArgumentException: Illegal characters in path.
Parameter name: path
   at System.IO.Path.GetFullPath(String path)
   at Coverlet.Core.Helpers.InstrumentationHelper.GetCoverableModules(String module, String[] directories, Boolean includeTestAssembly)
   at Coverlet.Core.Coverage.PrepareModules()
   at Coverlet.Collector.DataCollection.CoverageWrapper.PrepareModules(Coverage coverage)
   at Coverlet.Collector.DataCollection.CoverageManager.InstrumentModules()
   --- End of inner exception stack trace ---
   at Coverlet.Collector.DataCollection.CoverageManager.InstrumentModules()
   at Coverlet.Collector.DataCollection.CoverletCoverageCollector.OnSessionStart(Object sender, SessionStartEventArgs sessionStartEventArgs).

Test Run Successful.
Total tests: 1
     Passed: 1
 Total time: 1,8666 Seconds

Doing some investigation.

@vagisha-nidhi
Copy link
Contributor

@MarcoRossignoli Rooted down the cause. This is happening because of the * character in <IncludeDirectory>../dir1/*,../dir2/,</IncludeDirectory>. It fails to combine the path here https://github.com/tonerdo/coverlet/blob/master/src/coverlet.core/Helpers/InstrumentationHelper.cs#L47 considering * as an invalid character.

@MarcoRossignoli
Copy link
Collaborator

@vagisha-nidhi strange...I didn't passed that param...I'm using an empty on like

<?xml version="1.0" encoding="utf-8" ?>
<RunSettings>
  <DataCollectionRunSettings>
    <DataCollectors>
      <DataCollector friendlyName="XPlat code coverage">
        <Configuration>
          <Format>json</Format>
        </Configuration>
      </DataCollector>
    </DataCollectors>
  </DataCollectionRunSettings>
</RunSettings>

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented May 22, 2019

@vagisha-nidhi never mind...I used wrong settings, your analysis is correct.

@vagisha-nidhi
Copy link
Contributor

vagisha-nidhi commented May 22, 2019

@vagisha-nidhi never mind...I used wrong settings, your analysis it's correct.

@MarcoRossignoli Yeah. First of all, we should update the documentation so that nobody faces it if they are directly using the runsettings from what written in the documentation.

@vagisha-nidhi
Copy link
Contributor

@MarcoRossignoli Also, dotnet vstest scenario won't work with this release of sdk. It will be available with the next release.

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented May 22, 2019

@vagisha-nidhi let me do some other tests to undestand why that config worked in past...after that I'll update the docs.

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented May 22, 2019

My tests are ok, some logs

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

  <PropertyGroup>
    <TargetFramework>netcoreapp2.2</TargetFramework>

    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.1.0" />
    <PackageReference Include="coverlet.collector" Version="1.0.0" />
    <PackageReference Include="xunit" Version="2.4.1" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.4.1">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
  </ItemGroup>

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

</Project>
C:\Users\Marco\Downloads\TMP\TMP
λ dotnet --info
.NET Core SDK (reflecting any global.json):
 Version:   2.2.300
 Commit:    73efd5bd87

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.17763
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\2.2.300\

Host (useful for support):
  Version: 3.0.0-preview5-27626-15
  Commit:  61f30f5a23

.NET Core SDKs installed:
  2.1.202 [C:\Program Files\dotnet\sdk]
  2.1.504 [C:\Program Files\dotnet\sdk]
  2.1.505 [C:\Program Files\dotnet\sdk]
  2.1.600 [C:\Program Files\dotnet\sdk]
  2.1.601 [C:\Program Files\dotnet\sdk]
  2.1.602 [C:\Program Files\dotnet\sdk]
  2.1.700-preview-009597 [C:\Program Files\dotnet\sdk]
  2.1.700-preview-009601 [C:\Program Files\dotnet\sdk]
  2.1.700-preview-009618 [C:\Program Files\dotnet\sdk]
  2.2.104 [C:\Program Files\dotnet\sdk]
  2.2.203 [C:\Program Files\dotnet\sdk]
  2.2.204 [C:\Program Files\dotnet\sdk]
  2.2.300 [C:\Program Files\dotnet\sdk]
  3.0.100-preview5-011568 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.0-preview5-19227-01 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.0-preview5-27626-15 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.0.0-preview5-27626-15 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download

C:\Users\Marco\Downloads\TMP\TMP
λ dotnet test --collect:"XPlat Code Coverage"
Test run for C:\Users\Marco\Downloads\TMP\TMP\XUnitTestProject1\bin\Debug\netcoreapp2.2\XUnitTestProject1.dll(.NETCoreApp,Version=v2.2)
Microsoft (R) Test Execution Command Line Tool Version 16.1.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...

Attachments:
  C:\Users\Marco\Downloads\TMP\TMP\XUnitTestProject1\TestResults\45f3df8e-5657-4c91-b56e-98b196728d9e\coverage.cobertura.xml

Test Run Successful.
Total tests: 1
     Passed: 1
 Total time: 2,1578 Seconds

C:\Users\Marco\Downloads\TMP\TMP
λ dotnet test --settings coverletArgs.runsettings
Test run for C:\Users\Marco\Downloads\TMP\TMP\XUnitTestProject1\bin\Debug\netcoreapp2.2\XUnitTestProject1.dll(.NETCoreApp,Version=v2.2)
Microsoft (R) Test Execution Command Line Tool Version 16.1.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...

Attachments:
  C:\Users\Marco\Downloads\TMP\TMP\XUnitTestProject1\TestResults\481a7d4c-73e4-4b4a-8fe6-992e45fda824\coverage.json

Test Run Successful.
Total tests: 1
     Passed: 1
 Total time: 1,7576 Seconds

C:\Users\Marco\Downloads\TMP\TMP
λ
<?xml version="1.0" encoding="utf-8" ?>
<RunSettings>
  <DataCollectionRunSettings>
    <DataCollectors>
      <DataCollector friendlyName="XPlat code coverage">
        <Configuration>
          <Format>json</Format>
          <MergeWith>/custom/path/result.json</MergeWith>
          <Exclude>[coverlet.*.tests?]*,[*]Coverlet.Core*</Exclude>
          <Include>[coverlet.*]*,[*]Coverlet.Core*</Include>
          <ExcludeByAttribute>Obsolete,GeneratedCodeAttribute,CompilerGeneratedAttribute</ExcludeByAttribute>
          <ExcludeByFile>../dir1/class1.cs,../dir2/*.cs,../dir3/**/*.cs,</ExcludeByFile>
          <IncludeDirectory>../dir1/,../dir2/,</IncludeDirectory>
          <SingleHit>false</SingleHit>
          <UseSourceLink>true</UseSourceLink>
        </Configuration>
      </DataCollector>
    </DataCollectors>
  </DataCollectionRunSettings>
</RunSettings>

Simple test code:

namespace XUnitTestProject1
{
    public class UnitTest1
    {
        [Fact]
        public void Test1()
        {
            Class1 c = new Class1();
            c.Test();
        }
    }
}
...
namespace TMP
{
    public class Class1
    {
        public void Test()
        {

        }
    }
}

Report
image

I think that we could publish cc: @tonerdo @vagisha-nidhi

@vagisha-nidhi
Copy link
Contributor

vagisha-nidhi commented May 22, 2019

@MarcoRossignoli
Great! Once this is released, I can add its reference to the unit test csproj template.

@tonerdo
Copy link
Collaborator

tonerdo commented May 22, 2019

Awesome. Will make a release within 24 hours

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented May 22, 2019

@vagisha-nidhi before add reference I would like to do some tests with nuget package...only to be sure that all it's ok, for the sake.

@vagisha-nidhi
Copy link
Contributor

vagisha-nidhi commented May 22, 2019

@MarcoRossignoli Sure. Let me know about that.

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented May 22, 2019

@tonerdo just a moment maybe we missed one feature from collectors
@vagisha-nidhi do we need to notify threshold fails?
On msbuild and console we do it i.e.
https://github.com/tonerdo/coverlet/blob/master/src/coverlet.msbuild.tasks/CoverageResultTask.cs#L124
https://github.com/tonerdo/coverlet/blob/master/src/coverlet.console/Program.cs#L143

But no on collector https://github.com/tonerdo/coverlet/blob/master/src/coverlet.collector/DataCollection/CoverletCoverageCollector.cs#L139 we send only artifact.

@tonerdo if you want to setup and publish something, publish a beta version that we can test, and after we'll release official.

Bonus: we should setup a nighly build...I expect an increase in usage if MS will advertise this we should join beta of https://devblogs.microsoft.com/devops/pay-per-gb-azure-artifacts/
From post:
What’s next: public feeds
If you’re developing an open source project using a public Azure Repo or a repo on GitHub, you might want to share nightly or pre-release versions of your packages with your project team. Azure Artifacts public feeds will enable you to do just that, backed by the same scale and reliability guarantees as the private feeds you use for internal development. Interested in joining the preview? Get in touch (@alexmullans on Twitter).

@vagisha-nidhi
Copy link
Contributor

@MarcoRossignoli @tonerdo
Yeah we missed this. I can take this up as follow up enhancement. But since dotnet test --collect:"XPlat code coverage" is an entirely new switch and not going via any existing coverlet invocation points, the feature shouldn't impact customers in a negative way even if not present. If we are able to release a beta version also, we could get early feedback on what more needs to be incorporated.
Please let me know if you want me to pick this(the threshold feature) up before the release?

@vagisha-nidhi
Copy link
Contributor

@MarcoRossignoli
So I had a discussion regarding this. It might not be possible to just throw an exception and fail a test run in test platform using the datacollector. I will have to look deeper into it if we need any changes on the test platform side for failing a test run if a datacollector tells to. Aborting test run will not be a good option. I would not like to block the release based on this since it'll be good to have feedback if we want to incorporate more changes, and as already stated, since it is a new option, the implementation can go in stages.

@MarcoRossignoli
Copy link
Collaborator

@vagisha-nidhi @tonerdo if it's not "fully supported" by test plat at the moment I'm ok with release this version, we'll support it in future based on user feedback, we can release package!

@vagisha-nidhi
Copy link
Contributor

@tonerdo @MarcoRossignoli I think we can go ahead with releasing the nuget. I have done some testing from my side as well.

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented May 24, 2019

:shipit: Toni!

@tonerdo
Copy link
Collaborator

tonerdo commented May 24, 2019

@vagisha-nidhi @MarcoRossignoli do you think I should release it as a pre-release beta or is stable enough for a v1.0.0?

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented May 24, 2019

If we are able to release a beta version also, we could get early feedback on what more needs to be incorporated.

One step a time?1.0.0-beta1?
We can ask also to @ViktorHofer to give it a try on corefx!

Plan could be, beta1->nighly build(Toni I told you my idea offline) -> bug fix/new features -> nighly(for users)-> betaN or stable.

cc: @vagisha-nidhi

@vagisha-nidhi
Copy link
Contributor

If we are able to release a beta version also, we could get early feedback on what more needs to be incorporated.

One step a time?1.0.0-beta1?
We can ask also to @ViktorHofer to give it a try on corefx!

Plan could be, beta1->nighly build(Toni I told you my idea offline) -> bug fix/new features -> nighly(for users)-> betaN or stable.

cc: @vagisha-nidhi

@tonerdo @MarcoRossignoli A beta package is good enough for people to get their hands on. Plus we'll be able to know if there is any major issue that we should fix right away.

@vagisha-nidhi
Copy link
Contributor

@tonerdo @MarcoRossignoli
Is there an ETA on the release of coverlet.collector nuget? I would like to help if any contribution is needed from my end.

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented May 29, 2019

Green light to me we've nightly https://www.myget.org/feed/coverlet-dev/package/nuget/coverlet.collector (no perfect yet but enough in case of issue, we're fixing version to be more user friendly inside VS)
We can release a "pre-release" @tonerdo, current steps:
Now we're using https://github.com/AArnott/Nerdbank.GitVersioning (thank's @AArnott) first manual pre-release(for now we're working on AZP automation),
align to master current 0dd5624

git clean -fdx
dotnet pack coverlet.sln -c Release /p:PublicRelease=false
...
  Successfully created package 'F:\git\coverlet\build\Release\coverlet.collector.1.0.24-ge5998d8fbd.nupkg'. <- release
  Successfully created package 'F:\git\coverlet\build\Release\coverlet.msbuild.2.6.24-ge5998d8fbd.nupkg'.
  Successfully created package 'F:\git\coverlet\build\Release\coverlet.console.1.5.24-ge5998d8fbd.nupkg'.

https://docs.microsoft.com/en-us/nuget/create-packages/prerelease-packages#semantic-versioning

Pre-release versions are then denoted by appending a hyphen and a string after the patch number. 
Technically speaking, you can use any string after the hyphen and NuGet will treat the package as pre-release. 
NuGet then displays the full version number in the applicable UI, leaving consumers to interpret the meaning for themselves. <- for us is the HEAD commit

We could release also pre-release directly from master for simplicity and avoid to commit add and removal of -beta prefix, let's say non official release are all beta, @AArnott at the moment coverlet doesn't have servicing branches and doesn't release topic branches AFAIK, in future we'll think about it, but order in UI will be preserved thank's to calculated height and non-possible overlapping commit hash.
If there are objections on this schema we can talk please.
Read this great Andrew's doc https://github.com/AArnott/Nerdbank.GitVersioning/blob/publicReleaseDoc/doc/public_vs_stable.md

@clairernovotny
Copy link
Contributor

I'm hitting a missing method exception when I try to use it, details here: #456

@clairernovotny
Copy link
Contributor

One other thing -- from a docs perspective, the MergeWith parameter is confusing to me. What should the value be? An absolute path? Something relative? How/when do I use it?

I think that's an important parameter and it could help to make it more clear with some concrete examples.

@MarcoRossignoli
Copy link
Collaborator

One other thing -- from a docs perspective, the MergeWith parameter is confusing to me. What should the value be? An absolute path? Something relative? How/when do I use it?
I think that's an important parameter and it could help to make it more clear with some concrete examples.

@onovotny thank's for the feedback I opened a new issue #457, feel free to open new issue if find lack in documentations.

@vagisha-nidhi
Copy link
Contributor

@MarcoRossignoli @tonerdo
As specified in #110 (comment), the inproc dll name changes will flow into the next sdk release.

Also, to pick up inproc datacollector, we had changes in test host that comes via
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.1.0" />
This will only work with version>=16.1.0 which you can add to the documentation.
image

Never the less, this won't affect the outproc datacollector(only that it needs sdk>=2.2.300).

cc: @PBoraMSFT

@MarcoRossignoli
Copy link
Collaborator

Clear...recap to run in/out collectors we need to run >=2.2.300 SDK and reference test sdk >= 16.1.0

@vagisha-nidhi
Copy link
Contributor

vagisha-nidhi commented Jun 10, 2019

@MarcoRossignoli
Also, I have been trying out things with xunit/nunit and I found this #361 (comment). This would be a bad experience for any first user, if a test run crashes with --collect:"Xplat code coverage".

Should we add [xunit*]* to default exclude filter here https://github.com/tonerdo/coverlet/blob/master/src/coverlet.collector/Utilities/CoverletConstants.cs#L21?

cc: @cltshivash

@clairernovotny
Copy link
Contributor

clairernovotny commented Jun 10, 2019

Should we add [xunit*]* to default exclude filter here

I think the more 80% likely defaults and "right thing" should be defaulted, fwiw, so I'd say yes. I would also suggest it exclude *.test.dll/*.tests.dll assemblies as you almost never want to instrument and report on the unit test assembly itself.

@MarcoRossignoli
Copy link
Collaborator

Should we add [xunit*]* to default exclude filter here https://github.com/tonerdo/coverlet/blob/master/src/coverlet.collector/Utilities/CoverletConstants.cs#L21?

I'll move to new issue with my thoughts.

@MarcoRossignoli
Copy link
Collaborator

Done

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

No branches or pull requests

7 participants