a7ex / xcresultparser

Parse the binary xcresult bundle from Xcode builds and testruns
MIT License
83 stars 24 forks source link

Cobertura Coverage Reports Incorrect Number of Hits #26

Closed MatthewTHFisher closed 1 month ago

MatthewTHFisher commented 1 month ago

Hey! We have found a bit of a bug/issue when using xcresultparser to convert a .xcresult file using the cobertura method. We use the number of hits in the outputted report to verify how many times something has been tested, however the created cobertura report has the incorrect amount of hits by a large amount.

After spending a while analysing this we figured out that the number of hits reported in the cobertura file matches the first digit of the number of hits found in Xcode. As an example I've attached a couple images below:

(Left Xcode with the number of hits in the right hand bar, Right Cobertura report for the associated file from Xcode)

Screenshot 2024-05-15 at 16 36 50

In the above image, line 70 in Xcode (case "body":), the tests report that the line gets 12k test hits but in cobertura it reports the hits as 1.

Line 71 in Xcode gets hit 5346 times but in cobertura it reports the hits as 5. This supports our theory that the converter is only getting the first digit from the .xcresultbundle file as the number of hits to include in the report.

This is consistent with our other files, I've added another below:

Screenshot 2024-05-15 at 16 57 10

xcresultparser version - 1.5.2 xcode version - 15.3

MatthewTHFisher commented 1 month ago

Think I figured the issue out! Will make a PR soon!

hisaac commented 1 month ago

There's actually an open PR from @maxwell-legrand that will fix that issue (among some other things) https://github.com/a7ex/xcresultparser/pull/25

hisaac commented 1 month ago

Just out of curiosity, how are you using the execution count? We were just discussing yesterday how someone might use that value.

a7ex commented 1 month ago

Hi! Great that you contribute, I am traveling until tomorrow and can only look into it tomorrow. I learned a couple of weeks ago that now there is a much faster way to get the coverage and was postponing to implement the change into xcresultparser... Procrastinating again :-) Now I will definitely take the time to collaborate again asap!

MatthewTHFisher commented 1 month ago

@hisaac Ahhhh ok I glanced over the MR but assumed it didn't solve the actual issue which is in the file CoverageConvertor.swift because there were no changes to it. I'll checkout the PR to see if it solves the problem!

As for how I use the execution count ("hits"); I do all my work with GitLab and they offer test coverage visualisation in their MRs. Using this allows me to quickly hover over covered lines to see how many times a piece of code has been 'hit', this has been handy to easily check that a piece of code has been tested a suitable amount of times.

For example, a function that verifies if an email is valid or not, testing it once would technically mean it's covered (and GitLab will highlight anything >=1 hits in green to show its been covered), but I would still flag it up that it has not been tested enough.

So the issue is a little annoying for code that has 100 hits because xcresultparser just says its only has 1 hit.

hisaac commented 1 month ago

Ah gotcha, yeah perhaps the solution you have in mind is more effective. Never hurts to open a PR with ideas. 😊

hisaac commented 1 month ago

And that's a cool feature of GitLab! I wasn't aware of that.

MatthewTHFisher commented 1 month ago

Yeah it is pretty nifty! Shame that GitHub doesn't have it!