Addition of check-aggregate goal

[mike....@expanseinc.com]

Hello!

We're using report-aggregate extensively to manage our multi-module Maven projects, but also wanted to perform checks project wide. We ended up adding a custom check-aggregate goal to an internal fork, and have been using it for a while. We'd like to go back to the standard Jacoco, and would be happy to put up a PR to add a check-aggregate goal.

I wanted to check in before getting it into a state worthy of a PR and see if that's a feature you'd be willing to accept into the codebase. Does that seem like a reasonable addition?

[Evgeny Mandrikov]

Hi Mike,

First of all thank you for following correct process about addition of new feature by first starting a discussion with our little team in mailing list. And please excuse us for the delay with response.

Let me be completely transparent and honest with you by expressing my personal humble opinion about "check" goal in general:

If I'd be able to scroll time back, then personally would not introduce even "check" goal and would strongly vote against it. This opinion was formed over years since its introduction based on my own attempts to use it in various teams and projects and observations about its usage by others.

Checks "greater than" and "less then" are weak - frequent failures of these in continuous integration environment without proper mentoring/guidance/leadership/reviews leads to the fact that developers quickly learn how to add enough "easy" pretty useless coverage just to meet these criterias, while keep skipping "hard" cases. Even seen many cases where these criterias were leading to a changes in code under tests just to hide hard cases from these checks (such as usage of Streams/Optionals, change of design, etc where not needed) and eyes of reviewers.

And even with proper mentoring/guidance they are weak and important test cases are frequently missed, drawing these checks pretty much useless.

Strict check about exact total quickly becomes frequent point of merge conflicts: 100% coverage is usually not present and very hard to reach, branches introduce/change uncovered code for good reasons - have a look even at report for JaCoCo itself ( https://www.jacoco.org/jacoco/trunk/coverage/index.html ) where we strictly follow TDD, "accidental" coverage ( unrelated to modified code / tests ) is hard to find in big projects, and so correct value to resolve merge conflict is not easy to figure out, thus in eyes of developers this starts to look as "time waste", leading to frustrations and push-backs.

BTW aggregate-report is also quite frequently misused - just to bump numbers up by relying exactly on the "accidental" coverage.

What IMO works a way better than "check" that fails builds in CI - is protection of teams from managers who tend to look on raw coverage metric and proper leadership/work with a team to embrace principles of TDD, including code coverage measurements outside of CI in IDEs, while keeping their freedom to commit changes to meet business expectations and deadlines.

Coming back to initial question, based on the above personally I'm really unsure about introduction of "check-aggregate" and its following maintaince of a feature that does not really solve the problem (as it is already the case for "check") by our small team. I'd rather prefer to have/invest time into research of a better solutions.

Having said all this, I'm curious to learn your experience and what you think after that. And what thinks Marc ;)

Regards,

Evgeny

 

[Marc Hoffmann]

Hi Mike, hi Evgeny,

while I fully support Evgeny's view on the check goals in general, I'm afraid of the support problem:

Check goal as well as report-aggregate causes a lot of traffic on our bug tracker and mailing list. Mostly because people simply don't read documentation or come with questionable requirements (which they often do not understand themselfs -- maybe pushed by someone?).

I'm afraid that combining both (while I see valuable use cases) will simply kill our small team with support requests.

I would actually love to see all the advanced agregate/check etc. features in a separate project. So we had a chance to focus on the core code coverage engine.

Regards,
-marc

--
You received this message because you are subscribed to the Google Groups "JaCoCo and EclEmma Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jacoco+un...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jacoco/48112df7-f620-4403-a6d6-664e05198531%40googlegroups.com.

[mike....@expanseinc.com]

Hi Evgeny and Marc,

No worries about the delay, thanks for taking the time to write out such detailed responses!

We have some rules for minimum coverage that our projects with checks are typically well above. Past that, coverage levels are determined by reasonable developer practices. The checks act as more of a failsafe to enforce the bare minimum, rather than something that fails in the course of regular development, so it's been pretty painless for us in practice. I can understand that functionality not being a high value addition to Jacoco relative to the support costs. I do agree with many of your points about using checks at all; I've raised the question of whether we should at all with our team.

Our usage of Jacoco for checks is largely historical, now that I've thought about it more. We do, at this point, have another plugin that runs afterwards that parses the report and sends relevant portions to various data stores, and it seems quite reasonable to add checks into there, instead. Using Jacoco to generate the report and another tool to take action as a result of it seems like a natural separation of concerns.

The main other reason we forked in the first place was solved in 0.8.3 (thanks!), so moving the checks to our processing plugin seems like a very reasonable way to move back to standard Jacoco, if we want checks at all.

Thanks for taking the time to discuss the request, and for making such a useful tool!

> To unsubscribe from this group and stop receiving emails from it, send an email to jac...@googlegroups.com.