Coverlet: Instrumenting non used assemblies produces non existing branches (whith hits =0) in branch coverage. Lowering coverage when using /p:MergeWith

[pape77]

@tonerdo When calculating the coverage from an assembly that's referenced in another test project, I get some differences on branch coverage than when i run the tests for the actual test project that is ment to test that assembly

| Module               | Line   | Branch | Method |
+----------------------+--------+--------+--------+
| ProjectName.Api         | 100%   | 100%   | 100%   |
+----------------------+--------+--------+--------+
| ProjectName.Application | 100%   | 60%    | 100%   |
+----------------------+--------+--------+--------+
| ProjectName.Data        | 100%   | 62.5%  | 100%   |
+----------------------+--------+--------+--------+

But separately:

ProjectName.Data:

+---------------+--------+--------+--------+
| Module        | Line   | Branch | Method |
+---------------+--------+--------+--------+
| ProjectName.Data | 100%   | 100%   | 100%   |
+---------------+--------+--------+--------+

ProjectName.Application:

+----------------------+--------+--------+--------+
| Module               | Line   | Branch | Method |
+----------------------+--------+--------+--------+
| ProjectName.Application | 100%   | 75%    | 100%   |
+----------------------+--------+--------+--------+
|ProjectName.Data        | 0%     | 0%     | 0%     |
+----------------------+--------+--------+--------+

ProjectName.Api:

+----------------------+--------+--------+--------+
| Module               | Line   | Branch | Method |
+----------------------+--------+--------+--------+
| ProjectName.Api         | 100%   | 100%   | 100%   |
+----------------------+--------+--------+--------+
| ProjectNameApplication | 0%     | 0%     | 0%     |
+----------------------+--------+--------+--------+
| ProjectName.Data        | 0%     | 0%     | 0%     |
+----------------------+--------+--------+--------+

So my idea was to get:

| Module               | Line   | Branch | Method |
+----------------------+--------+--------+--------+
| ProjectName.Application | 100%   | 75%    | 100%   |
+---------------+--------+--------+--------+
| ProjectName.Data | 100%   | 100%   | 100%   |
+---------------+--------+--------+--------+
| ProjectName.Api         | 100%   | 100%   | 100%   |
+----------------------+--------+--------+--------+

Just noticed ProjectName.Application detects some branches of the ProjectName.Data that ProjectName.Data does not detect when running the coverage for itself alone.

Is this normal/possible?

"ProjectName.Repositories.KeyRepository/<AddAsync>d__4": {
        "System.Void ProjectName.Repositories.KeyRepository/<AddAsync>d__4::MoveNext()": {
          "Lines": {
            "34": 4,
            "35": 4,
            "36": 1,
            "37": 1,
            "40": 3,
            "41": 3,
            "42": 3
          },
"Branches": [
            {
              "Line": 41,
              "Offset": 98,
              "EndOffset": 100,
              "Path": 0,
              "Ordinal": 2,
              "Hits": 0
            },
            {
              "Line": 41,
              "Offset": 98,
              "EndOffset": 165,
              "Path": 1,
              "Ordinal": 3,
              "Hits": 0
            }
          ]

"ProjectName.Data.Repositories.KeyRepository/<AddAsync>d__4": {
        "System.Void ProjectName.Data.Repositories.KeyRepository/<AddAsync>d__4::MoveNext()": {
          "Lines": {
            "34": 4,
            "35": 4,
            "36": 1,
            "37": 1,
            "40": 3,
            "41": 3,
            "42": 3
          },
          "Branches": [
            {
              "Line": 41,
              "Offset": 98,
              "EndOffset": 165,
              "Path": 1,
              "Ordinal": 3,
              "Hits": 1
            }
          ]

[tonerdo]

@pape77 I don't quite understand what the problem is. Naturally if you run the coverage under different contexts, the final output would be different. Can you perhaps share your project structure and what commands you run at each different stage

[pape77]

@tonerdo I'll try to explain better:

ProjectName.Api references ProjectName.Application and ProjectName.Data

ProjectName.Application references ProjectName.Data

ProjectName.Data doesn't reference any of the other two projects

I named my projects ProjectName.X.Tests where X is the corresponding project that I want to test.
In these test projects I only (and emphasis in only) test functionality that's in X module. The rest I mock it out.

So for example when I run ProjectName.Application.Tests, since ProjectName.Application references ProjectName.Data, coverlet instruments these two. And includes coverage for ProjectName.Data in the resulting coverage file too (with all coverage being 0, so hits = 0 everywhere).

This is as expected. The problem is that this coverage file includes some branchs with hits = 0 that don't exist at all when running a particular coverlet command over ProjectName.Data.Tests

As a result of the latest, when merging ProjectName.Application.Tests and ProjectName.Data.Test coverage files, it will indicate that ProjectName.Data has less coverage that it has on ProjectName.Data.Test on it's own, because of the mentioned non existing but generated branches with hits = 0 in ProjectName.Application.Test result

Codes to exec this are really simple:

dotnet test .\test\ProjectName.Application.Tests\  -l trx -r /results /p:CollectCoverage=true /p:Coverlet
OutputFormat='json' /p:CoverletOutput=/results/coverage

and then compare that output file with coverage file for ProjectName.Data module on it's own

dotnet test .\test\ProjectName.Data.Tests\  -l trx -r /results /p:CollectCoverage=true /p:Coverlet
OutputFormat='json' /p:CoverletOutput=/results/coverage

With the pr at #225 what I do is don't instrument the dlls that are referenced in test projects and not used/tested at all (as in my case, in which i mock everything, but still have the reference because i use the project's classes, and because it's referenced in the project I'm actually testing)

Here are the two files that I got generated with the commands above
Coverage.json for Application.Tests:
https://textuploader.com/dwtb4

Coverage.json for Data.Tests:
https://textuploader.com/dwtbp

Methods like FindAsync and AddAsync have branches with 0 hits in ProjectName.Application.Tests which don't exist in ProjectName.Data.Tests and the instrumented dll is the same.

I think this is not ok, I've tried writing all kind of tests for those silly 2 methods and I couldn't get the branch that is marked with 0 hits in Application to pop in the Data coverage file even with 0 hits whatsoever

Hopefully it's more clear now

Note: I only made focus on Application and Data projects. But same happens for Api Project in my first comment of the issue, since it references Application and Data projects

Note2: Note that the merging does work correctly but I just made a quick mention of it here since it's not part of the real issue i want to illustrate, if i run

dotnet test .\ProjectName.Data.Tests -l trx -r /results /p:CollectCoverage=true /p:CoverletOutputFormat='json%2
copencover' /p:CoverletOutput=/results/coverage

dotnet test .\test\ProjectName.Application.Tests\  -l trx -r /results /p:CollectCoverage=true /p:Coverlet
OutputFormat='json' /p:CoverletOutput=/results/coverage  /p:MergeWith='/results/coverage.json'

the result file will be the merged one between these 2 projects. Problem being, since the second run produces branches with 0 hits that don't exist in the output of the first command line, the merged coverage file will indeed increment hits only in the branches that are in the first run. Making the global coverage for ProjectName.Data to decrease (since branches with 0 hits exist in the final, merged, output)

[pape77]

In when trying to solve this (pr #225) i found a comment in the code that called my attention

In Coverage.cs line 169 :
// File not instrumented, or nothing in it called. Warn about this?
So when nothing in it is called, it's still instrumented and included in coverage.json, this is what i want to avoid, because if would be ok if it didn't pollute coverage, but since it does and we are not calling anything within this dll, then why should it pop in the final coverage file?
This is why, when finding something like this, i remove it from the _results variable

[pape77]

@tonerdo have you had the chance to take a second look at this? Hope it's clear now

[tonerdo]

@pape77 I think I get it now. I've noticed branch coverage reporting defaults to 0 when there are no branches, which shouldn't be the case. In terms of nothing being called I think I want to keep that default behaviour and I suggest you simply exclude the offending assembly

[pape77]

@tonerdo I cannot exclude the assembly since it's being used by the classes at my other project . If you ment by command line param, again i can't since i run this for the whole solution in one line.

But most importantly, the problem is not branch coverage defaulting to 0 if not covered. The problem is that the non used assembly is generating NON EXISTING branches, with coverage 0 which will never be covered by the actual tests in other projects and therefore not overrided when merging. This will make code coverage % to go down, not reflecting reality. That's why i want to exclude the assembly if nothing there is used, which... Make sense right? What's your reason for keeping it?

[pape77]

@tonerdo you mean the /p:Exclude param then. I have to specify what to exclude and i cannot make a generic test run for all my projects if I do, which is my final goal (probably not the only one with it).
Moreover, I would have to make one command line per test proj to exclude what i need, instead of one for the solution doing all the job.
Why should we keep this if it's polluting the coverage? :/ I doubt if anybody has noticed this behaviour, or they just see the file being generated and don't pay attention to what's happening.

[pape77]

@tonerdo One option could be adding a command line option param to exclude non used assemblies, if you really want to keep current behaviour as default. Wouldn't mind modifying my pr to do so.
But I really think the default bahaviour should be outputing the correct coverage when merging 😅

[pape77]

@tonerdo i updated my pr to add this as an option so default behaviour remains the same and fixes the issues for those who need to exclude non used files from coverage (#225)

I got an error when doing some http post call for a vs 2015 test on app veyor. Probably if you re run it, it will work.
Would you consider this for merging with this as an option? :)

[vlef]

Any news on this issue? I can't really tell my coworkers to use Coverlet in our builds if the coverage decreases in complex solutions where many project references are used. Our coverage will be lower numbers that don't make much sense, if I understand the issue correctly.

Like pape77, we really insist on running our tests per solution and having good results without any manual setup in our builds.

Thanks!

[pape77]

@vlef i already gave up and forked away. If you use current coverlet, even more things that I don't care about (and maybe you don't either) gets into the report. Like certain external dlls i use.
So decided to implement this in my own fork, sadly

[vlef]

Thanks for the heads up! That is quite unfortunate, especially since even the official Microsoft docs link to Coverlet.

[pape77]

Yep. I first wanted to exclude them by default. Then came with this option variable to keep the original behavior for whoever uses it. But no luck it seems 😢

[pape77]

@vlef if you plan to fork, you can copy the code from my original pr (so not the last commit but the first one). That will instrument only the dlls that you are touching with your tests. Although, i believe (as i mentioned) that now more things get instrumented even, not only the dlls with pdb files, but all. So you may have to remove a couple more things to make it look like the version i changed in my pr by that time