False negative with "/p:MergeWith" async/await branches

[MarcoRossignoli]

I moved discussion from old PR to new issue.

---

@pape77 issue #158 (comment)

I have a similar issue with this "ghost branches" which are never covered.
My code looks like this

public class KeyProvider : IKeyProvider
{
    private readonly IKeyRepository _repository;
    private readonly IMemoryCache _cache;

    public KeyProvider(IKeyRepository repository, IMemoryCache cache)
    {
        _repository = repository;
        _cache = cache;
    }

    public async Task<Key> GetByUuidAndTypeAsync(Guid uuid, KeyType type, CancellationToken cancellationToken)
    {
        return await _cache.GetOrCreateAsync($"uuid-{uuid.ToString()}-key-{type}",
            async entry => await _repository.FindOrCreateAsync(uuid, type, cancellationToken));
    }

public class Key
{
public int Id { get; set; }

    public Guid Uuid { get; set; }

    [Required]
    public Guid Kid { get; set; }

    [Required]
    public KeyType KeyType { get; set; }
}

public enum KeyType
{
Cenc,
Fairplay,
Aes
}
}
This code generates 4 branches, 2 of which I'm never able to cover

my tests:

public class KeyProviderTests
{
private readonly Mock _keyRepositoryMock;
private readonly MemoryCache _memoryCache;
private readonly Guid _testGuid = Guid.NewGuid();
private readonly KeyProvider _keyProvider;

    public KeyProviderTests()
    {
        _memoryCache = new MemoryCache(new MemoryCacheOptions());

        _keyRepositoryMock = new Mock<IKeyRepository>();

        _keyProvider = new KeyProvider(_keyRepositoryMock.Object, _memoryCache);
    }

    [Theory(DisplayName = "Should call repository method with correct values")]
    [InlineData(KeyType.Cenc)]
    [InlineData(KeyType.Aes)]
    [InlineData(KeyType.Fairplay)]
    public async Task GetByUuidAndTypeAsync01(KeyType keyType)
    {
        _keyRepositoryMock.Setup(kr =>
                kr.FindOrCreateAsync(_testGuid, keyType, It.IsAny<CancellationToken>()))
            .ReturnsAsync(new Key());

        await _keyProvider.GetByUuidAndTypeAsync(_testGuid, keyType, new CancellationToken());

        _keyRepositoryMock.Verify(
            kr => kr.FindOrCreateAsync(_testGuid, keyType, It.IsAny<CancellationToken>()), Times.Once);
    }

    [Theory(DisplayName = "Should get key from cache")]
    [InlineData(KeyType.Cenc)]
    [InlineData(KeyType.Aes)]
    [InlineData(KeyType.Fairplay)]
    public async Task GetByUuidAndTypeAsync02(KeyType keyType)
    {
        var cacheKey = $"uuid-{_testGuid.ToString()}-key-{keyType}";
        _memoryCache.Set(cacheKey, new Key());
       
        await _keyProvider.GetByUuidAndTypeAsync(_testGuid, keyType, new CancellationToken());

        _keyRepositoryMock.Verify(
            kr => kr.FindOrCreateAsync(_testGuid, keyType, It.IsAny<CancellationToken>()), Times.Never);
    }

}
The generated branches in my coverage.json are (notice the MoveNext() thing):

 "Keys.Application.Services.KeyProvider/<>c__DisplayClass3_0/<<GetByUuidAndTypeAsync>b__0>d": {
    "System.Void Keys.Application.Services.KeyProvider/<>c__DisplayClass3_0/<<GetByUuidAndTypeAsync>b__0>d::MoveNext()": {
      "Lines": {
        "27": 9
      },
      "Branches": [
        {
          "Line": 27,
          "Offset": 8,
          "EndOffset": 14,
          "Path": 0,
          "Ordinal": 0,
          "Hits": 3
        },
        {
          "Line": 27,
          "Offset": 81,
          "EndOffset": 83,
          "Path": 0,
          "Ordinal": 2,
          "Hits": 0
        },
        {
          "Line": 27,
          "Offset": 8,
          "EndOffset": 119,
          "Path": 1,
          "Ordinal": 1,
          "Hits": 0
        },
        {
          "Line": 27,
          "Offset": 81,
          "EndOffset": 147,
          "Path": 1,
          "Ordinal": 3,
          "Hits": 3
        }
      ]
    }
  },

Line 27 in my code is precisely the async await one:
async entry => await _repository.FindOrCreateAsync(uuid, type, cancellationToken));

hope it's useful

---

Repro keys.zip

/cc @kevindqc /@tonerdo

[MarcoRossignoli]

I was able to repro the issue with @pape77 code, I'll come back ASAP.

[MarcoRossignoli]

I was able to repro the issue with @pape77 code, I'll come back ASAP.

False positive for now...it was an issue with report generator.

@pape77 I need more info...can you provide "command line" to generate your coverage.json sample(with 4 branch) i.e. from root (Keys.sln place).
I cannot generate 4 branch for that call but only 2.

[pape77]

I removed the cache part to make it more explicit. That was adding two more branches because there was another await being used. There should be 2 branches now. One of which is never covered

[MarcoRossignoli]

My current test:

dotnet test "test\Keys.Application.Tests\Keys.Application.Tests.csproj" /p:CollectCoverage=true   /p:CoverletOutput=..\..\test-results\coverage\Keys.Application.Tests.json /p:Include="[Keys.Application]*"

result

{
  "Keys.Application.dll": {
    "...\\src\\coverlet\\Issue1\\src\\Keys.Application\\Services\\KeyProvider.cs": {
      "Keys.Application.Services.KeyProvider": {
        "System.Void Keys.Application.Services.KeyProvider::.ctor(Keys.Data.Repositories.Contracts.IKeyRepository)": {
          "Lines": {
            "16": 3,
            "17": 3,
            "18": 3,
            "19": 3
          },
          "Branches": []
        }
      },
      "Keys.Application.Services.KeyProvider/<GetByUuidAndTypeAsync>d__2": {
        "System.Void Keys.Application.Services.KeyProvider/<GetByUuidAndTypeAsync>d__2::MoveNext()": {
          "Lines": {
            "22": 3,
            "23": 3,
            "24": 3
          },
          "Branches": [
            {
              "Line": 23,
              "Offset": 62,
              "EndOffset": 128,
              "Path": 1,
              "Ordinal": 1,
              "Hits": 3
            }
          ]
        }
      }
    }
  }
}

[pape77]

I use the console version of coverlet. Will provide the script in a sec

[MarcoRossignoli]

I use the console version of coverlet. Will provide the script in a sec

Thank's here my playground if you find something strange https://github.com/MarcoRossignoli/marcorossignoli.github.io/tree/coverletbug/src/coverlet/Issue1

[pape77]

If I run your command I get the same as you. Correct branch. While while running with the merge with over all I get this incorrect branch thing

Just installed the latest coverlet.msbuild pkg on test projects. I still get the same result when running this on solution level:

dotnet test /p:CollectCoverage=true /p:CoverletOutput=..\..\test-results\coverage\ /p:MergeWith=..\..\test-results\coverage\coverage.json

[MarcoRossignoli]

If I run your command I get the same as you. Correct branch. While while running with the merge with over all I get this incorrect branch thing

Maybe merge issue...I go on...

[MarcoRossignoli]

@pape77 I found the issue...merge is the culprit, the issue is in this line https://github.com/tonerdo/coverlet/blob/master/src/coverlet.core/CoverageResult.cs#L94
To merge it uses all field, but different test has got different instrumentation values

"Branches": [
            {
              "Line": 23,
              "Offset": 62,
              "EndOffset": 128,
              "Path": 1,
              "Ordinal": 1,
              "Hits": 3
            },
            {
              "Line": 23,
              "Offset": 62,
              "EndOffset": 64,
              "Path": 0,
              "Ordinal": 0,
              "Hits": 0
            }
          ]

For instance if we compare only "Line" it works...but I think that it could lead to ambiguity, I'll try to understand if it's safe.

/cc @tonerdo

[pape77]

Nice! Hope it's fixed soon :)

[pape77]

I don't think only line is enough. I have things like this in my well generated coverage files:

"Branches": [
            {
              "Line": 44,
              "Offset": 18,
              "EndOffset": 20,
              "Path": 0,
              "Ordinal": 0,
              "Hits": 1
            },
            {
              "Line": 44,
              "Offset": 18,
              "EndOffset": 49,
              "Path": 1,
              "Ordinal": 1,
              "Hits": 1
            }
          ]

Maybe @tonerdo knows a bit more about this

[MarcoRossignoli]

I think it works because you've two hits if in different tests you cover that branch.

[MarcoRossignoli]

@kevindqc are you using merge?