SteveGilham / altcover

Cross-platform coverage gathering and processing tool set for dotnet/.Net Framework and Mono
MIT License
498 stars 18 forks source link

Branch Coverage not reporting the same branch coverage on the same line in two different classes in the same assembly. #143

Closed simkin2004 closed 2 years ago

simkin2004 commented 2 years ago

Originally thought my issue was #90, but realized that none of the methods are async methods.

I am getting 2 of 4 branches covered when calling "using(var connection = this.ProviderFactory.CreateConnection())" in one class and full branch coverage when calling the same method the same way in another class in the same assembly.

In both classes, the DbProviderFactory is passed in on the constructor of the class and is guaranteed to be not-null by a check in the constructor.

There are two unit tests for each class that ensure that the branch coverage on both the null and non-null branches of DbProviderFactory.CreateConnection() are covered.

In the same unit test run with the same configuration on a single assembly, one class shows that the specified code line is Branch Fully Covered and the other class shows 2 of 4 branches covered.

I have not been able to reproduce this issue in a smaller test set.

I have a attached the csproj, offending .cs files, and offending MsTests, and a screenshot of the SonarCloud showing the discrepancy. Branch-Coverage-Failure.zip

If you need the links to the code, tests, or SonarCloud, I can provide them.

SteveGilham commented 2 years ago

but realized that none of the methods are async methods.

But each are of signature

protected override async Task InvokeCoreAsync(IPipelineContext context)

The SaveContextToDatabaseStep.cs in the .zip doesn't have a using, as per the SaveContextToDatabaseStep-Coverage.png file but instead has a try/finally doing the same job manually. SupplementContextWithDatabaseRecordStep.cs does look like the listing in SupplementContextWithDatabaseRecordStep-Coverage.png.

From an initial inspection, it looks like the difference comes from the fact that there are two nested using clauses in the SupplementContextWithDatabaseRecordStep , so the generated code is woven differently around them; with a 4-way state-maching switch at the outer using in SaveContextToDatabaseStep, but a series of if/else choices in the other case.

SteveGilham commented 2 years ago

On more detailed inspection, the difference turns out to be that the code generated for the state machine code in SupplementContextWithDatabaseRecordStep/<InvokeCoreAsync>d__4::MoveNext() has only one generated switch based on the state, and that in a context such that the generated nature of the branching is "obvious" and so is ignored in the coverage, whereas SaveContextToDatabaseStep/<InvokeCoreAsync>d__4::MoveNext() has two separate such switches, and the interaction between them means that the second one is not being detected as generated.

So, it's really a false positive here; and a deficiency in the heuristic to detect such.

SteveGilham commented 2 years ago

Should be resolved in release 8.2.836

simkin2004 commented 2 years ago

Thank you! Thank you! Thank you! Thank you! Thank you! Thank you! Thank you! Thank you! A million times THANK YOU!