Coverage is inconsistent following Xunit upgrade. #700

JimBobSquarePants · 2020-01-22T14:31:17Z

Issue

Following upgrade to xunit 2.4.1 from 2.3.1 in the ImageSharp repo certain tests do not run and no coverage is calculated.

For example.

Baseline coverage 83.79%
https://github.com/SixLabors/ImageSharp/runs/398717930#step:11:424

Following upgrade 63.28%
https://github.com/SixLabors/ImageSharp/runs/402256993#step:11:399

As you can see from the hosted report an entire section of code has now been excluded as a result of non-running test.
https://codecov.io/gh/SixLabors/ImageSharp/tree/6f832aa77343e8d112d6ddd08765aa2e4d05519c/src/ImageSharp/ColorSpaces

This gets more odd though. I added some filtering to ensure the xunit references were added to the
test project only I got another change. (No tests have changed)

Now 64.8%
https://github.com/SixLabors/ImageSharp/runs/403076336#step:11:396

I'm stumped. A whole suite of tests are getting ignored suddenly and I cannot figure out why.

Example test.
https://github.com/SixLabors/ImageSharp/blob/4973f5f160e4768af344075fe4e3fef39be26d69/tests/ImageSharp.Tests/Colorspaces/Conversion/RgbAndHslConversionTest.cs#L34

Coverlet Versions

coverlet.collector 1.1.0
coverlet.msbuild 2.8.0

MarcoRossignoli · 2020-01-22T15:41:32Z

Some question,
Are you using msbuild integration(msbuild /p:CollectCoverage=true) or vstest integration(dotnet test --collect:"XPlat Code Coverage")?
Do you see difference in "number of test ran"?I mean xunit shows a different number of test found between two version?
Do you see difference in a particular tfm?(I'm working on a similar issue but only for net472 #578)
If you downgrade coverlet to 2.7.0 do you see the same behaviour?

JimBobSquarePants · 2020-01-23T04:42:39Z

Hi @MarcoRossignoli thanks for getting back so swiftly.

I'm using it via the following command.

 dotnet test -c Release -f netcoreapp3.1 /p:codecov=true

This calls our target file configured in the following manner.

  <!--Code coverage specific settings-->
  <!--https://github.com/tonerdo/coverlet-->
  <PropertyGroup Condition="'$(codecov)' == 'true' AND '$(IsTestProject)' == 'true'">
    <CollectCoverage>true</CollectCoverage>
    <UseSourceLink>true</UseSourceLink>
    <Include>[SixLabors.*]*</Include>
    <CoverletOutputFormat>lcov</CoverletOutputFormat>
    <CoverletOutputPath>$(MSBuildThisFileDirectory)..\</CoverletOutputPath>
    <CoverletOutput>$(CoverletOutputPath)$(AssemblyName).$(TargetFramework).lcov</CoverletOutput>
    <!--Used by coverlet due to reference issues-->
    <!--https://github.com/tonerdo/coverlet/blob/master/Documentation/KnowIssues.md#4-failed-to-resolve-assembly-during-instrumentation-->
    <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
  </PropertyGroup>

The post upgrade runs an additional two unit tests, non of which are related to the missing coverage results.

Running the same tests in Debug leads to a result of 48.25% which again, I know to be inaccurate. (I run in Release as it means 10min shorter build times.)
https://github.com/SixLabors/ImageSharp/runs/402949429#step:11:379

I just rand the same command locally targeting coverlet.msbuild 2.7.0 and got an even smaller number.

+----------------------+--------+--------+--------+
| Module               | Line   | Branch | Method |
+----------------------+--------+--------+--------+
| SixLabors.ImageSharp | 39.41% | 27.07% | 40.48% |
+----------------------+--------+--------+--------+

+---------+--------+--------+--------+
|         | Line   | Branch | Method |
+---------+--------+--------+--------+
| Total   | 39.41% | 27.07% | 40.48% |
+---------+--------+--------+--------+
| Average | 39.4%  | 27.07% | 40.47% |
+---------+--------+--------+--------+

I cannot run the coverage tests in netcorapp2.1 in Release mode with either version as that introduces JIT errors related to Systems.Numerics.Vectors causing our unit tests to fail.

Running the tests in Debug with 2.7.0 yields the following, again different result:

+----------------------+--------+--------+--------+
| Module               | Line   | Branch | Method |
+----------------------+--------+--------+--------+
| SixLabors.ImageSharp | 30.49% | 23.98% | 33.63% |
+----------------------+--------+--------+--------+

+---------+--------+--------+--------+
|         | Line   | Branch | Method |
+---------+--------+--------+--------+
| Total   | 30.49% | 23.98% | 33.63% |
+---------+--------+--------+--------+
| Average | 30.49% | 23.98% | 33.63% |
+---------+--------+--------+--------+

MarcoRossignoli · 2020-01-23T15:03:47Z

With 2.8.0 and 2.3.1 all is ok?To be sure that is related to xunit interaction.
Unrelated if you can you should move to collectors https://github.com/tonerdo/coverlet#vstest-integration-preferred-due-to-know-issue

JimBobSquarePants · 2020-01-24T03:02:07Z

Yeah OK with both of those versions.
Baseline coverage 83.79%
https://github.com/SixLabors/ImageSharp/runs/398717930#step:11:424

I'll have a look at migrating across. I saw you're planning to drop MSBuild support.

MarcoRossignoli · 2020-01-24T09:47:42Z

I saw you're planning to drop MSBuild support.

"Drop" is heavy we've a lot of users on it, but the idea is to move(at our pace) to maintain only integration with vstest collector to avoid know issue(behaviour of vstest engine) and have a less drift between drivers feature(we need to keep a set of feats, threshold check and merge on all), but we're in the discussion stage.

JimBobSquarePants · 2020-01-24T10:50:12Z

Thanks, I had a play around locally with the collector. It generated a report (how do you set the output file and location btw? The options are different) but not an output to the console.

Having a quick look at the relevant sections in the report though with my limited experience it looked like the results were the same as MSBuild and incorrect.

MarcoRossignoli · 2020-01-24T11:31:45Z

Having a quick look at the relevant sections in the report though with my limited experience it looked like the results were the same as MSBuild and incorrect.

Ok thank's for the test, I noticed that the two ci above uses different file format, the correct one xml(opencover)

Calculating coverage result...
  Generating report 'D:\a\ImageSharp\ImageSharp\tests\..\coverage.netcoreapp3.1.xml'

+----------------------+--------+--------+--------+
| Module               | Line   | Branch | Method |
+----------------------+--------+--------+--------+
| SixLabors.ImageSharp | 83.79% | 76.25% | 77.91% |
+----------------------+--------+--------+--------+

and the second one(where you see strange percentage) lcov,

Calculating coverage result...
  Generating report 'D:\a\ImageSharp\ImageSharp\tests\..\SixLabors.ImageSharp.Tests.netcoreapp3.1.netcoreapp3.1.lcov'

+----------------------+--------+--------+--------+
| Module               | Line   | Branch | Method |
+----------------------+--------+--------+--------+
| SixLabors.ImageSharp | 63.28% | 56.72% | 63.56% |
+----------------------+--------+--------+--------+

is this ok?If you run using opencover with new xunit version does the percentage drops again?(to exclude a report generator bug)

NB you see Generating report 'D:\a\ImageSharp\ImageSharp\tests\..\SixLabors.ImageSharp.Tests.netcoreapp3.1.netcoreapp3.1.lcov' because in ver 2.8.0 we fixed that bug and now msbuild ver(another thing that we don't have on vstest) automatically append tfm for multitargets so you can remove your tfm variable on filename composition #177, changelog https://github.com/tonerdo/coverlet/blob/master/Documentation/Changelog.md#release-date-2020-01-03

Thanks, I had a play around locally with the collector. It generated a report (how do you set the output file and location btw?

Collectors are pretty new and we're porting msbuild/.net tool feat to it(with some trouble due to vstest plat restriction/extension point) today you cannot specify file name but only result directory with default dotnet test -r|--results-directory <PATH> switch

Check our sample https://github.com/tonerdo/coverlet/tree/master/Documentation/Examples/VSTest/HelloWorld

but not an output to the console.

Yep this feat is not supported yet here the tracking issues https://github.com/tonerdo/coverlet/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+label%3Adriver-collectors+label%3Afeature-request

We're aware that collectors are today not so user friendly(like msbuild/dotnet tool), but unfortunately are the only usage that prevents strange issue so we're trying to move on it without losing any prev feat at best we can with what we have today, my "dream" is to help also vstest team and reach a point where we can use collectors and pass parameter without a custom xml files.

In short the idea is to try to move sto stable and well supported plat(vstest) without losing nothing, so we won't drop msbuild or .net tool but we'd like to invest in the future(also related to .NET 5 story).
The initial idea is to add missing "post-test" feature not supported on vstest with new verb added to our .net tool you can follow discussion #683

JimBobSquarePants · 2020-01-24T23:31:39Z

I just ran the test locally using the opencover output with the following results.

dotnet test -c Release -f netcoreapp3.1 /p:codecov=true

+----------------------+--------+--------+--------+
| Module               | Line   | Branch | Method |
+----------------------+--------+--------+--------+
| SixLabors.ImageSharp | 36.96% | 26.03% | 37.5%  |
+----------------------+--------+--------+--------+

+---------+--------+--------+--------+
|         | Line   | Branch | Method |
+---------+--------+--------+--------+
| Total   | 36.96% | 26.03% | 37.5%  |
+---------+--------+--------+--------+
| Average | 36.96% | 26.03% | 37.5%  |
+---------+--------+--------+--------+

So it looks like this is even worse.

I'm not precious about the output format at all as long as Coveralls supports it. The switch was made because it was a 10mb difference in output file size.

MarcoRossignoli · 2020-01-24T23:33:56Z

Ok tomorrow I'll go on with the investigation... thanks for the support!
First of all I need to repro, I got some unexpected test fail, BTW in case I'll attach log to this thread.

JimBobSquarePants · 2020-01-25T07:47:44Z

I don't know if this helps but I just ran coverage analysis via Visual Studio on the code which returned the result 83.79% which is what I would expect.

james_MJÖLNIR 2020-01-25 18_19_36.coverage.txt

Running Coverlet gives me

+----------------------+--------+--------+--------+
| Module               | Line   | Branch | Method |
+----------------------+--------+--------+--------+
| SixLabors.ImageSharp | 35.58% | 25.97% | 33.69% |
+----------------------+--------+--------+--------+

+---------+--------+--------+--------+
|         | Line   | Branch | Method |
+---------+--------+--------+--------+
| Total   | 35.58% | 25.97% | 33.69% |
+---------+--------+--------+--------+
| Average | 35.58% | 25.97% | 33.69% |
+---------+--------+--------+--------+

SixLabors.ImageSharp.Tests.netcoreapp3.1.netcoreapp3.1.lcov.txt

MarcoRossignoli · 2020-01-25T07:56:57Z

Thank's I don't know how at the moment how coverage in vs works(how it instruments). Btw this/next days I'll work on this and hope is also related to the other similar one 🤞

JimBobSquarePants · 2020-01-25T08:29:40Z

Much appreciated. Best of luck!

MarcoRossignoli · 2020-01-25T08:43:20Z

@JimBobSquarePants I get some exception if I move to 2.4.1 System.ArgumentException : GenericArguments[0], 'SixLabors.ImageSharp.Color', on 'Void Generic_To[TDestPixel](TDestPixel)' violates the constraint of type 'TDestPixel'.

I think is related to 

Can I use this branch to repro https://github.com/SixLabors/ImageSharp/compare/af/improve-tests?

JimBobSquarePants · 2020-01-25T08:52:22Z

You can use the master branch now. We've just merged the PR containing that branch.

MarcoRossignoli · 2020-01-25T09:12:00Z

Mmm I need also the previous version your new packages config does not allow come back to old xunit to compare because remote executor wants 2.4.1 asserts lib, BTW to be close to CI I've cloned two time with and without PR for now

JimBobSquarePants · 2020-01-25T09:24:58Z

No problem. Here's a branch to use.

https://github.com/SixLabors/ImageSharp/tree/js/coverlet-comparison-do-not-delete

MarcoRossignoli · 2020-01-25T09:28:36Z

Great thank's!

MarcoRossignoli · 2020-01-26T17:05:02Z

Fixed in SixLabors/ImageSharp#1094 wasn't xunit issue but https://github.com/tonerdo/coverlet/blob/master/Documentation/KnowIssues.md#1-vstest-stops-process-execution-earlydotnet-test

MarcoRossignoli added the needs more info More details are needed label Jan 22, 2020

JimBobSquarePants mentioned this issue Jan 23, 2020

Harden memory-intensive tests, repurpose Sandbox46 SixLabors/ImageSharp#1089

Merged

4 tasks

MarcoRossignoli mentioned this issue Jan 25, 2020

Collecting code coverage prevents all test discovery on net472 only #578

Closed

MarcoRossignoli mentioned this issue Jan 25, 2020

Fix coverage SixLabors/ImageSharp#1094

Merged

MarcoRossignoli closed this as completed Jan 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Coverage is inconsistent following Xunit upgrade. #700

Coverage is inconsistent following Xunit upgrade. #700

JimBobSquarePants commented Jan 22, 2020

MarcoRossignoli commented Jan 22, 2020 •

edited

Loading

JimBobSquarePants commented Jan 23, 2020 •

edited

Loading

MarcoRossignoli commented Jan 23, 2020

JimBobSquarePants commented Jan 24, 2020

MarcoRossignoli commented Jan 24, 2020

JimBobSquarePants commented Jan 24, 2020

MarcoRossignoli commented Jan 24, 2020 •

edited

Loading

JimBobSquarePants commented Jan 24, 2020

MarcoRossignoli commented Jan 24, 2020 •

edited

Loading

JimBobSquarePants commented Jan 25, 2020

MarcoRossignoli commented Jan 25, 2020 •

edited

Loading

JimBobSquarePants commented Jan 25, 2020

MarcoRossignoli commented Jan 25, 2020

JimBobSquarePants commented Jan 25, 2020

MarcoRossignoli commented Jan 25, 2020

JimBobSquarePants commented Jan 25, 2020

MarcoRossignoli commented Jan 25, 2020

MarcoRossignoli commented Jan 26, 2020

Coverage is inconsistent following Xunit upgrade. #700

Coverage is inconsistent following Xunit upgrade. #700

Comments

JimBobSquarePants commented Jan 22, 2020

Issue

Coverlet Versions

MarcoRossignoli commented Jan 22, 2020 • edited Loading

JimBobSquarePants commented Jan 23, 2020 • edited Loading

MarcoRossignoli commented Jan 23, 2020

JimBobSquarePants commented Jan 24, 2020

MarcoRossignoli commented Jan 24, 2020

JimBobSquarePants commented Jan 24, 2020

MarcoRossignoli commented Jan 24, 2020 • edited Loading

JimBobSquarePants commented Jan 24, 2020

MarcoRossignoli commented Jan 24, 2020 • edited Loading

JimBobSquarePants commented Jan 25, 2020

MarcoRossignoli commented Jan 25, 2020 • edited Loading

JimBobSquarePants commented Jan 25, 2020

MarcoRossignoli commented Jan 25, 2020

JimBobSquarePants commented Jan 25, 2020

MarcoRossignoli commented Jan 25, 2020

JimBobSquarePants commented Jan 25, 2020

MarcoRossignoli commented Jan 25, 2020

MarcoRossignoli commented Jan 26, 2020

MarcoRossignoli commented Jan 22, 2020 •

edited

Loading

JimBobSquarePants commented Jan 23, 2020 •

edited

Loading

MarcoRossignoli commented Jan 24, 2020 •

edited

Loading

MarcoRossignoli commented Jan 24, 2020 •

edited

Loading

MarcoRossignoli commented Jan 25, 2020 •

edited

Loading