Skip to content

Passing multiple --coverage files produces order-dependent and badly inflated results #6

Description

@dillon7f

Summary

--coverage accepts multiple files, which is the natural thing to do when a solution has several test projects. Supplying more than one produces wrong answers that depend on argument order.

Same source, same threshold, varying only the coverage files passed:

--coverage arguments crappy methods crap load
App only 60 525
App, then Functions 101 909
Functions, then App 164 1514

Adding a second coverage file made the report roughly three times worse, and reversing the order changed it again. Nothing about the code under analysis changed.

Root cause

AnalyzeCommand flattens every file into one list:

foreach (var covPath in resolvedCoveragePaths)
    coverageEntries.AddRange(CoberturaCoverageReader.Read(stream));

Duplicate entries for the same method are therefore normal. MethodCoverageMatcher then mishandles them in two separate ways:

// Pass 1 - first entry wins, so argument order decides the result
Coverage = exactMatches[0].Coverage

// Pass 2 - duplicates fail the guard, silently dropping the method to 0.0
if (nameKeyLookup.TryGetValue(nameKey, out var nameMatches) && nameMatches.Count == 1)

The second is why results get dramatically worse rather than slightly worse. A second test project that merely references the assembly under analysis reports its classes at ~0% (mine mentioned them 1,347 times), and every method relying on the name-only fallback is knocked to zero.

Suggested fix

Merge entries that are the same method arriving from different files, grouping on the raw Cobertura identity (class name, method name, signature, first line) and taking the maximum coverage. A method covered by one test project is covered, regardless of how many other projects loaded the assembly without exercising it.

Do not relax the guard to "all candidates resolve to the same canonical key". That looks correct and is not: fixing the compiler-rewritten matching (other issues) means dropping the signature from the key, so two async overloads resolve to the same canonical key and would be merged into one value. I tried exactly this and it silently gave one overload another's coverage, which reports an untested overload as covered. A false negative that hides untested code is worse than the false positives this whole area started with.

Overloads that collide on a signature-less key can still be separated, because the generated state machine keeps the source positions of the method it was rewritten from and Cobertura records them. Pair colliding source methods to coverage entries in line order, and only when the two sides line up exactly: equal counts, line numbers present on both sides, matching filenames. Anything less certain should fall through to the 0.0 default rather than guess.

This needs the reader to retain line numbers, which it currently discards.

Workaround

Pass exactly one coverage file, from the test project that actually covers the code being analyzed.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions