Closed afscrome closed 2 months ago
Can you share the coverage files you are using?
You can send them privately via email: reportgenerator@palmmedia.de or share them here.
Same for me. Regenerating reports with history on the same cobertura file generates reports with differences. @danielpalme Do you need any additional info to fix this ?
@AlexanderBartoshZ A sample cobertura file and history files would be great. And some instructions which commands you are executing.
@danielpalme Thank you for quick reaction!
You should have the coverage report in your inbox.
Repro:
Run this command 2 times:
reportgenerator -reporttypes:html -reports:output.cobertura.o.xml -targetdir:htmlh -historydir:h
Open the html report and check the difference - you expect none, but you get some:
Let me know if you need any other info
The coverage is collected using dotnet-coverage collect -f cobertura
from muplitple processes.
Some of the "changed" coverage lines are about the code that only runs within one process
Thank you! Will have a look as soon as possible!
@AlexanderBartoshZ I was able to fix the problem with the unexpected changes in history comparison.
One remaining problem is, that some classes appear twice in the report (e.g. Aspire.Dashboard.Otlp.Storage.CircularBuffer<T>
).
I guess you are using Microsoft CodeCoverage, therefore nested classes appear differently in the Cobertura file (compared to tools like coverlet). Details can be found here: #663.
Sorry for missing this @danielpalme . Not sure I'm allowed to share my files I'm afraid.
That said, I do believe I have a variant of the the same duplicate classes issue you highlighted above, although my issue is happening specifically withe Merge. I've tested locally and #697 fixes my inconsistency.
Thank you @danielpalme. As soon as there is a binary I can verify it with - Will gladly do that! Yes, as stated in the initial comment, dotnet-coverage (aka https://github.com/microsoft/codecoverage) is used
With regards to #663 :
What would be the right argument to change the .
to /
:
You cannot distinguish between nested classes and non-nested ones (?)
If there are enough arguments IMO the pressure for a change of https://github.com/microsoft/codecoverage can be built up. IF those do not work correctly with VS it could be a heavy argument
@AlexanderBartoshZ
If there are enough arguments IMO the pressure for a change of https://github.com/microsoft/codecoverage can be built up. IF those do not work correctly with VS it could be a heavy argument
Microsoft does not want to change form .
to '/'. See:
https://github.com/microsoft/codecoverage/issues/124#issuecomment-2175667139
Yes. Have read that ... But IMO, If there is a good argument they will have to
If I re-run
CoverageReportParser.ParseFiles
against the same input coverage files, I'm getting different total line counts.I believe the problem is coming from some kind of concurrency race condition - after removing the parallelism from the following line, I start getting consistent results. https://github.com/danielpalme/ReportGenerator/blob/4444f14f0b574d4deda7921e3ab6892c823474c7/src/ReportGenerator.Core/Parser/CoberturaParser.cs#L135
Digging further, I came across the following, and confirmed that my report files are ending up with multiple classes with the same name. When the earlier parallelism is enabled, these duplicates get added in different orders, meaning the
FirstOrDefault
can return different results if re-run against the same inputs. https://github.com/danielpalme/ReportGenerator/blob/4444f14f0b574d4deda7921e3ab6892c823474c7/src/ReportGenerator.Core/Parser/Analysis/Assembly.cs#L180At this point I'm getting a bit lost in the weeds, but I have two theories for the root cause.
The first is that the
Equals
method forClassNameParserResult
compares bothName
andDisplayName
, meaning we get all unique PAIRS of Name and DisplayName - giving us opportunities for later duplicates https://github.com/danielpalme/ReportGenerator/blob/4444f14f0b574d4deda7921e3ab6892c823474c7/src/ReportGenerator.Core/Parser/CoberturaParser.cs#L122-L128The other possible cause is a mismatch between the following bits of logic in filtering for elements for a given class: https://github.com/danielpalme/ReportGenerator/blob/4444f14f0b574d4deda7921e3ab6892c823474c7/src/ReportGenerator.Core/Parser/CoberturaParser.cs#L122-L128 https://github.com/danielpalme/ReportGenerator/blob/4444f14f0b574d4deda7921e3ab6892c823474c7/src/ReportGenerator.Core/Parser/CoberturaParser.cs#L149-L157 https://github.com/danielpalme/ReportGenerator/blob/4444f14f0b574d4deda7921e3ab6892c823474c7/src/ReportGenerator.Core/Parser/CoberturaParser.cs#L190-L198