search

tomasbjerre / violations-lib

Java library for parsing report files from static code analysis.
Apache License 2.0
148 stars 39 forks source link

Support for jacoco code coverage #128

Closed ateglas closed 3 years ago

ateglas commented 3 years ago

One of the steps in our pipeline is enforcing code coverage metrics on the changed code. The way jacoco enforces those metrics is by having a task that passes/fails across the whole code. We were able to implement what we wanted with the help of the violations lib, with a twist:

  1. violation lib does not understand jacoco reports, so we wrote an extra step that converts the coverage xml to a code climate json, raising an issue for every method that is more than x lines long and has a coverage of less than y%
  2. we used violation lib plugins to read that report and post comments on the MR about methods with insufficient coverage (it is a bit hard to post at the individual line level, not from a technical perspective, but from a usability one)

I was thinking of contributing the jacoco report but I do not see a way of configuring parsers. I could contribute with my settings, but probably others would have different uses. What's your take on situations like this?

tomasbjerre commented 3 years ago

Configurable parsers has been requested before. I think it would make the tools/plugins much harder to understand and maintain. So I would like to avoid making parsers configurable.

But... perhaps adding a configurable Jacoco parser, with a default configuration, is a way forward. And perhaps allow the configuration to be changed in the violations-command-line tool. So that you can:

You have a Jacoco parser now if I understand correctly? Perhaps you open a PR with that even if it is not complete.

ateglas commented 3 years ago

It is a parser, in that it takes the xml and spits the code climate json, but not a perser in the violations lib implementation. Will check the idea about violations-command-line, sounds like a possible workaround for those who wish to configure the result. Whoever wants to use with the defaults could do so, and those that want to customize can use the command line together with whatever else plugin they are using (jenkins, gradle or maven) and achieve that goal.

Will raise a PR with the defaults first and we can take it from there about the customization.

On Tue, Jul 6, 2021 at 9:14 AM Tomas Bjerre @.***> wrote:

Configurable parsers has been requested before. I think it would make the tools/plugins much harder to understand and maintain. So I would like to avoid making parsers configurable.

But... perhaps adding a configurable Jacoco parser, with a default configuration, is a way forward. And perhaps allow the configuration to be changed in the violations-command-line tool. So that you can:

  • Transform the Jacoco to Codeclimate with violations-command-line
  • Report the Codeclimate report with any of the other tools/plugins.

You have a Jacoco parser now if I understand correctly? Perhaps you open a PR with that even if it is not complete.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tomasbjerre/violations-lib/issues/128#issuecomment-874490230, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGKS6SDBEKC5OLFPMFH76Y3TWKNN7ANCNFSM47324LMA .

tomasbjerre commented 3 years ago

Thanks for working on this! The feature is now released. I also added it to: https://github.com/tomasbjerre/violations-command-line

Hope it works. I did not test it.

ateglas commented 3 years ago

Awesome, thanks. Will do a test run today and will take a look at the command line as well.

On Mon, Jul 12, 2021 at 11:44 PM Tomas Bjerre @.***> wrote:

Thanks for working on this! The feature is now released. I also added it to: https://github.com/tomasbjerre/violations-command-line

Hope it works. I did not test it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tomasbjerre/violations-lib/issues/128#issuecomment-878582588, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGKS6SD4N7CCT7C2H35N3FTTXNH4JANCNFSM47324LMA .

ateglas commented 3 years ago

I played around with the new Jacoco parser these days, both with the library only (with defaults) and with the command line (with the jmc and jmlc parameters). Everything seems to work as expected, if there is a bug I could not find any.

On Tue, Jul 13, 2021 at 9:02 AM Aurelian Teglas @.***> wrote:

Awesome, thanks. Will do a test run today and will take a look at the command line as well.

On Mon, Jul 12, 2021 at 11:44 PM Tomas Bjerre @.***> wrote:

Thanks for working on this! The feature is now released. I also added it to: https://github.com/tomasbjerre/violations-command-line

Hope it works. I did not test it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tomasbjerre/violations-lib/issues/128#issuecomment-878582588, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGKS6SD4N7CCT7C2H35N3FTTXNH4JANCNFSM47324LMA .

tomasbjerre commented 3 years ago

Great!