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

Wrong coverage for "await foreach" when method is generic #1210

Closed
msallin opened this issue Aug 6, 2021 · 8 comments · Fixed by #1312
Closed

Wrong coverage for "await foreach" when method is generic #1210

msallin opened this issue Aug 6, 2021 · 8 comments · Fixed by #1312
Assignees
Labels
tenet-coverage Issue related to possible incorrect coverage with repro Issue with repro

Comments

@msallin
Copy link

msallin commented Aug 6, 2021

I'm using coverlet.msbuild v3.1.0 (and tried also with the current nightly build) and there is an uncovered statement reported.
image

I tried to reproduce it but I didn't managed to get an reproduction.
Any ideas what the problem could be and how I can provide an reproduction?

public class AsyncForeachReproduction
{
    private static async IAsyncEnumerable<int> RangeAsync(int start, int count)
    {
        for (int i = 0; i < count; i++)
        {
            await Task.Delay(i);
            yield return start + i;
        }
    }

    public async Task Execute()
    {
        await foreach (var a in RangeAsync(1, 5))
        {
            Console.WriteLine(a);
            await Task.CompletedTask;
        }

        await Task.CompletedTask;
    }
}
@MarcoRossignoli MarcoRossignoli added bug Something isn't working tenet-coverage Issue related to possible incorrect coverage labels Aug 14, 2021
@MarcoRossignoli
Copy link
Collaborator

Thanks for reporting this.

@MarcoRossignoli MarcoRossignoli added untriaged To be investigated and removed bug Something isn't working labels Aug 17, 2021
@msallin
Copy link
Author

msallin commented Feb 5, 2022

@MarcoRossignoli this is still the case with 3.1.1
Let me know if you need more information.

@MarcoRossignoli
Copy link
Collaborator

We need a repro to find out why/when that branch is emitted by the compiler, to repro you should create the same method using mocked object, but the "shape" needs to be "identical" to mimic the same context for the compiler.

@msallin
Copy link
Author

msallin commented Feb 5, 2022

Any hints/idea what could cause this i.e. Why my repro doesnt work?

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Feb 5, 2022

The code in the repro and the code that is failing are a lot different from compiler perspective, state machine are built in a different way because different object structure are involved(different nesting, visibility, different relationship between objects...different optimization)...you should create the same classes, mock those and retry.

Try to compare those with ILSpy and you'll see that the emitted IL is different.

@msallin
Copy link
Author

msallin commented Feb 5, 2022

Seems like it doesn't work if the method takes a type-parameter.

image

image

public class AwaitForeachReproductionFixture2
{
    [Fact]
    public async Task Execute_ShouldWork()
    {
        // Arrange
        var sut = new AwaitForeachReproduction2();

        // Act
        await sut.Execute<object>(RangeAsync(1, 3));
    }

    static async IAsyncEnumerable<int> RangeAsync(int start, int count)
    {
        for (int i = 0; i < count; i++)
        {
            await Task.Delay(i);
            yield return start + i;
        }
    }
}
public class AwaitForeachReproduction2
{
    public virtual async Task Execute<T>(IAsyncEnumerable<int> messages)
    {
        await foreach (int obj in messages)
        {
            await Task.Delay(1);
        }
    }
}

@msallin msallin changed the title Potentially wrong coverage for "await foreach" Wrong coverage for "await foreach" when method is generic Feb 5, 2022
@msallin
Copy link
Author

msallin commented Feb 7, 2022

See https://github.com/meggima/coverlet-reproductions AwaitForeachReproduction2.cs & AwaitForeachReproductionFixture2.cs

@MarcoRossignoli MarcoRossignoli added tenet-coverage Issue related to possible incorrect coverage with repro Issue with repro and removed tenet-coverage Issue related to possible incorrect coverage untriaged To be investigated labels Feb 7, 2022
@daveMueller
Copy link
Collaborator

I started working on this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tenet-coverage Issue related to possible incorrect coverage with repro Issue with repro
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants