Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cobertura Coverage Reports Incorrect Number of Hits #26

Closed
MatthewTHFisher opened this issue May 15, 2024 · 8 comments · Fixed by #27
Closed

Cobertura Coverage Reports Incorrect Number of Hits #26

MatthewTHFisher opened this issue May 15, 2024 · 8 comments · Fixed by #27

Comments

@MatthewTHFisher
Copy link
Collaborator

MatthewTHFisher commented May 15, 2024

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
Copy link
Collaborator Author

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

@hisaac
Copy link
Collaborator

hisaac commented May 15, 2024

There's actually an open PR from @maxwell-legrand that will fix that issue (among some other things) #25

@hisaac
Copy link
Collaborator

hisaac commented May 15, 2024

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

@a7ex
Copy link
Owner

a7ex commented May 15, 2024

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
Copy link
Collaborator Author

MatthewTHFisher commented May 15, 2024

@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
Copy link
Collaborator

hisaac commented May 15, 2024

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

@hisaac
Copy link
Collaborator

hisaac commented May 15, 2024

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

@MatthewTHFisher
Copy link
Collaborator Author

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

@a7ex a7ex closed this as completed in #27 May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants