altcover icon indicating copy to clipboard operation
altcover copied to clipboard

issue//p:AltCoverVisibleBranches=true flags awaited async calls.

Open StevePy opened this issue 11 months ago • 4 comments

Earlier I had added the VisibleBranches flag to correct an issue which I suspect was related to issue #72, it was some time ago so I am trying to track down what test was reporting odd branch coverage. However, since enabling that setting it seems that the coverage for awaited async calls is reporting uncovered branches.

A simple code under test of:

using (var context = new AppDbContext(options)) { int count = await context.Searches.CountAsync(); // <- * }

with /p:AltCoverVisibleBranches=true is glagging the CountAsync line as 1 of 2 branches covered. Removing the flag has it reporting no uncovered branch. AFAIK when awaited async calls are compiled there is conditional logic inserted in the IL that test coverage needs to ignore. It seems strange that a flag intended to ignore switch/match would result in async calls being treated more pessimistically.

Is this expected or a possible bug?

StevePy avatar Jan 12 '25 22:01 StevePy

A bug in my current definition of invisible branches seems most likely.

The current behaviour of the visible branches flag is to suppress jumps that are contained entirely within a sequence point, the sort of implementation detail compiler injections which appear all too often. My suspicion is that this case may involve a branch that crosses into the next sequence point - something that also doesn't really look like a branch at the user level, but a case which the code currently doesn't check.

While I'm setting up a test case, would it be possible to share the section of the XML report for just the method that contains the unexpected jump (suitably redacting names as required)?

SteveGilham avatar Jan 13 '25 17:01 SteveGilham

WithFlag.txt WithoutFlag.txt

Attached two sections from the coverage.xml with the flag and without. The key areas would be searching for calls to the RetrieveSearchCriteria. This method has a number of awaited async calls including a dummy extra CountAsync() call against a vanilla DbContext instance.

What is strange is that the suspicious branch inclusion/behaviour is with the flag set which should be excluding extra switch conditions, yet without the flags the extra branch considerations for the async calls don't happen. For now I have disabled the flag. I don't think I had many cases where I was getting extra branch points. (Not as often as I'm now testing around async calls)

Edit: The RetrieveSearchCriteria method itself:

    [HttpGet]
    public async Task<GetRowsResult<SearchViewModel<TSearchCriteria>>> RetrieveSearchCriteria()
    {
        var configuration = new ConfigurationBuilder()
       .AddJsonFile("appsettings.json")
       .Build();

        var options = SupportPortalDbContext.BuildOptions(configuration);
        
        using (var context = new SupportPortalDbContext(options))
        {
            var count = await context.Searches.CountAsync();
        }

        var config = SearchViewModel<TSearchCriteria>.BuildConfig();
        try
        {
            using var contextScope = ContextScopeFactory.CreateReadOnly();

            var searchCount = await SearchRepository.GetSearches(typeof(TSearchCriteria))
                .CountAsync();

            if (searchCount == 0)
                await insertDefaultSearchCriteria();

            var searches = await SearchRepository.GetSearches(typeof(TSearchCriteria))
                .OrderBy(s => s.Name)
                .ProjectTo<SearchViewModel<TSearchCriteria>>(config)
                .AsNoTracking()
                .ToListAsync();

            var result = GetRowsResult<SearchViewModel<TSearchCriteria>>.Success(searches,
                searches.FirstOrDefault(s => s.IsSelected),
                () => searches.Count < ApplicationOptions.MaxSavedSearches);
            return result;
        }
        catch (Exception ex)
        {
            Logger.Error(ex, "Exception");
            return GetRowsResult<SearchViewModel<TSearchCriteria>>.Failure(ReadFailureReason_Exception);
        }
    }

The first 10 lines are the stub test scenario I injected to determine if the extra branches were related to something I had implemented vs. a vanilla DbContext call. Esentially each of the async calls result in an extra branch. I did a text compare between the two XML extract and it seems to go from 3 branch points without the flag to 18 with the flag? Seems weird given what that flag seemed to be added for. :)

StevePy avatar Jan 14 '25 05:01 StevePy

Just diff'ing the two files, WithFlag,txt clearly has more branch points simply comparing the point index (uspid) numbers.

The first method, canInsertSearchCriteria, has the extra branch

                <BranchPoint vc="1" uspid="1334" ordinal="8" offset="226" sl="217" path="0" offsetend="235" fileid="305" />

which starts and ends (offsets 226, 235) which are entirely within sequence point number 2045

                <SequencePoint vc="1" uspid="2045" ordinal="14" offset="215" sl="217" sc="13" el="217" ec="47" bec="1" bev="1" fileid="305" />
                <SequencePoint vc="1" uspid="2044" ordinal="15" offset="236" sl="218" sc="9" el="218" ec="10" bec="0" bev="0" fileid="305" />

which spans 215 to 235 inclusive; and similarly for the next few cases I inspected.

It looks like I have the flag set up so that its effect when set is to make the internal branches visible, which is actually the sensible way around; but the documentation is wrong.

SteveGilham avatar Jan 14 '25 14:01 SteveGilham

Hi, I think I'm hitting the same issue with F# Task CE where all return!, match! and let! lines show uncovered branches. Only do! lines are mostly shown as fully covered (although not always). I'm not passing any additional flags like AltCoverVisibleBranches. I'm using 9.0.1 version

Image

Lanayx avatar Feb 10 '25 19:02 Lanayx