search

GaelGirodon / ci-badges-action

Badges for test results and code coverage
GNU General Public License v3.0
13 stars 2 forks source link

JaCoCo reported code coverage is wrong #5

Open kevin-belellou opened 3 days ago

kevin-belellou commented 3 days ago

Hello @GaelGirodon,

First of all, let me say that I'm quite happy to have found you GitHub Actions, as it is exactly what I wanted! But I have an issue about JaCoCo code coverage reporting.

Contrary as said in the documentation for JaCoCo

The coverage will be extracted from the last tag with type LINE, from the first matching and valid report file.

It seems that the coverage value is read from the first <counter> tag instead of the last.

GaelGirodon commented 2 days ago

Hello, thanks for testing this GitHub Action and opening this issue (and a PR).

The JaCoCo report loader was added to support more common formats and make the action more versatile, but I've never used the action with this report format on a real project, you may be one of the first! I should have written better tests to identify this obvious bug sooner 😅

Looking at the current implementation again, I think I could improve it beyond fixing this bug (using regexes to extract values from XML is simple but is not a very good solution, and using 2 global regexes to find the same line is even worse...).

Would you mind if I close your pull request in favor of a broader improvement that I'll try to release this week?

kevin-belellou commented 2 days ago

No problem, my PR was really a dirty fix 🗑️

GaelGirodon commented 2 days ago

It is more or less what was missing from the initial implementation: the global flag on regular expressions and better test data, so your PR seems fine to fix this precise bug, but I think that a global improvement of the implementation is preferable!