RFC: JMC-5657: Code coverage with Jacoco
Carmine Vincenzo Russo
carusso at redhat.com
Fri May 10 18:26:12 UTC 2019
thanks for your reply.
On Tue, May 7, 2019 at 6:38 PM Guru <guru.hb at oracle.com> wrote:
> Hi Carmine,
> Please check my comments inline.
> > On 07-May-2019, at 2:05 PM, Carmine Vincenzo Russo <carusso at redhat.com>
> > Hello everyone,
> > This patch adds Jacoco coverage over unit test in JMC.
> > The module covered in particular are: /core/tests and /application/tests.
> > The patch addresses the issue JMC-5657, but as noted by Klara Ward,
> > instead of adding Jcov, it's better to add Jacoco.
> > For the time being, each test group had a new module, report, where an
> > aggregate-report can be found.
> > You need to run 'mvn verify' in jmc root for /application/tests and in
> > /core for /core/tests to have the coverage report.
> Logically there are three build step
> 1. For releng
> 2. Core modules
> 3. Jmc product (i.e Application)
> Ideally `mvn verify` is run on ~/jmc/core (for step 2) and ~/jmc (for step
Obviously all the 3 steps are needed, I also checked, the 'mvn verify' for
~/jmc/core is automatically done while building the core modules on step 2.
For jmc product you still need to run 'mvn verify -P uitests' separately
after building the project since you need the test to be executed.
Unless there is restriction on defining Jacoco in root of
> ~/jmc/core/pom.xml and ~/jmc/pom.xml.
I explain this at the end.
> > There are also some aspects that concern me, so: Is this what we are
> > looking for?
> > The report for /core looks good and clean. The report for
> > /application/tests has good results, although there are some 0%,
> > that is ok, but also some N/A. Should we keep the modules in the report
> > with only N/A?
> > There are also some changes in /application/uitests when reading the
> > they are not yet applied but I would like to add them to the next patch.
> Please file a follow on new JBS and link with old one (relates to
With 'next patch' I just meant the updated patch after the RFC, basically
what I have done now, just wanted to have your opinions before going
forward with the ~/uitests coverage. If a follow up issue is needed, I'll
ask Alex to open it.
> > This brings me to my second concern: should the tests and uitests in
> > /application have a unique report or should I keep them separate?
> Good to have Unique rather than separate.
In the new patch attached ~/jmc/application/tests and
~/jmc/applciation/uitests have a unique coverage in
> > Lastly, do we want coverage with every verify build or do we want a maven
> > profile/flag to have the coverage only in some occasions?
> This could be based on how much time Jacoco Code coverage takes. If its
> negligible then good to have on every verify build or with a profile/flag.
In all my tests and attempts the module itself never took more than 10s,
while the build doesn't seem to be affected. So basically I don't think
it's a problem to have always the coverage in every build.
> > Overall, how does the patch look like? Do you think there are some more
> > modules that need to be covered? And do you have any suggestions on how
> > improve what I've done?
> If possible, please update the patch to ~/jmc/core/pom.xml and
As far as my knowledge goes, to have the report, we need a separate pom.xml
for the report, this because the report-aggregate relies on <dependency>
and not on <module>, even the JaCoCo examples for this have their own
report module. The best way, I think, is the current situation in which
we have ~/jmc/core/coverage/pom.xml and ~/jmc/application/coverage/pom.xml,
these generate and contains the coverage report, while in
~/jmc/core/pom.xml and ~/jmc/application/pom.xml we have the JaCoCo agent
that makes the coverage possible. If we try to move everything in
~/jmc/application/pom.xml and ~/jmc/core/pom.xml the reports will be
segregated in their own modules. Or if we try to move part of it in
~/jmc/pom.xml we stiil need to have both the agent configuration in
~/jmc/application/pom.xml and ~/jmc/core/pom.xml and the coverage as
All the changes and the new addition can be found here:
> > Carmine
> >  https://bugs.openjdk.java.net/browse/JMC-5657
> > 
> >  https://imgur.com/hPyyLsQ
> >  https://imgur.com/fqdzGvH
> >  https://imgur.com/ozqNcAB
> > --
> > Carmine Vincenzo Russo
> > <5657-0.patch>
Carmine Vincenzo Russo
More information about the jmc-dev