beam-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stas Levin (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (BEAM-1399) Code coverage numbers are not accurate
Date Wed, 15 Feb 2017 19:06:41 GMT

    [ https://issues.apache.org/jira/browse/BEAM-1399?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15868375#comment-15868375
] 

Stas Levin edited comment on BEAM-1399 at 2/15/17 7:06 PM:
-----------------------------------------------------------

Having spent a while researching this, I can share the following findings.

I believe you are correct in that the coverage is inaccurate, and the {{report}} goal does
not capture coverage provided by external (to the tests) modules.
{{report-aggregate}} takes into account coverage provided by such modules, but it seems to
assume that the external module (e.g. {{DirectRunner}}) is all about integration tests, despite
it having unit tests of its own which are not reported, and so the coverage is yet again,
inaccurate.
In addition, as far as I could see {{report-aggregate}} does not provide a solution for the
rest of the modules, assuming we would like to obtain the coverage across the entire Beam
project and not just the SDK. This is also the assumption throughout the rest of this comment,
so if it's wrong and we are fine with covering the SDK only, my conclusions below may not
apply.

The general problem of obtaining code coverage for an arbitrary depth, multi-module maven
project is heavily discussed ^1-3^ and is believed to be NP complete.
Nonetheless some heuristics are out there:
# Introduce a new module, which should be *dependent on all the modules in the project*, this
will allow using {{Jacoco}}'s {{report-aggregate}} goal.
# Hack our way.
## Introduce a new reporter module
## Add {{maven-antrun-plugin}} to the new module and set up an {{Ant}} task to gain a more
fine-grained control ^4^ over the coverage report generation.

I do not believe (1) is appropriate for Beam since it requires tons of manual labor, and does
not scale well. This leaves us with (2).

To go with (2) we first need to address the following issues:
# We have test classes that essentially act as both unit tests, and integration tests (from
{{maven}}'s perspective). Such classes are characterised by having both regular test methods,
and test methods decorated with {{@NeedsRunner}} or {{@RunnableOnService}} which are run by
{{maven-surefire-plugin}} during the {{integration-test}} phase (as opposed to the {{test}}
phase for regular test methods). From my experiments this scenario is not well accommodated
^5^ in {{jacoco-maven-plugin}}.
# Reassigning {{@NeedsRunner}} and {{@RunnableOnService}}  based tests to be triggered during
the {{test}} phase does not do the trick, probably because they scan dependencies (for tests)
which fail to be ready when the {{test}} phase executes. I assume this was the reason they
were configured to run as part of the {{verify}} phase in the first place.
# I believe these issues can be alleviated by moving {{@NeedsRunner}} and {{@RunnableOnService}}
to separate *integration test* classes which will execute during the {{integration-test}}
phase and thus avoid interfering with the tests that run during the {{test}} phase.

My impression on this is that it is doable, but the benefit-cost ratio seems to be quite questionable.
Perhaps we could look at external tools such as [SonarQube|https://www.sonarqube.org] which
specialise in providing software metrics such as test coverage and the likes.
There is also a [Jenkins JaCoCo Plugin | https://wiki.jenkins-ci.org/display/JENKINS/JaCoCo+Plugin],
which I have not looked into.

1. https://groups.google.com/forum/#!searchin/jacoco/%22multi$20module%22$20coverage%7Csort:relevance
2. https://github.com/jacoco/jacoco/wiki/MavenMultiModule
3. https://www.google.co.il/search?q=jacoco+multi+module+coverage&oq=jacoco+multi+module+coverage&aqs=chrome.0.0j69i57j69i60l3.7899j0j4&sourceid=chrome&ie=UTF-8
4. http://www.eclemma.org/jacoco/trunk/doc/ant.html
5. http://www.eclemma.org/jacoco/trunk/doc/classids.html


was (Author: staslev):
Having spent a while researching this, I can share the following findings.

I believe you are correct in that the coverage is inaccurate, and the {{report}} goal does
not capture coverage provided by external (to the tests) modules.
{{report-aggregate}} takes into account coverage provided by such modules, but it seems to
assume that the external module (e.g. {{DirectRunner}}) is all about integration tests, despite
it having unit test of its own which are not reported, and so the coverage is yet again, inaccurate.
In addition, as far as I could see {{report-aggregate}} does not provide a solution for the
rest of the modules, assuming we would like to obtain the coverage across the entire Beam
project and not just the SDK. This is also the assumption throughout the rest of this comment,
so if it's wrong and we are fine with covering the SDK only, my conclusions below may not
apply.

The general problem of obtaining code coverage for an arbitrary depth, multi-module maven
project is heavily discussed ^1-3^ and is believed to be NP complete.
Nonetheless some heuristics are out there:
# Introduce a new module, which should be *dependent on all the modules in the project*, this
will allow using {{Jacoco}}'s {{report-aggregate}} goal.
# Hack our way.
## Introduce a new reporter module
## Add {{maven-antrun-plugin}} to the new module and set up an {{Ant}} task to gain a more
fine-grained control ^4^ over the coverage report generation.

I do not believe (1) is appropriate for Beam since it requires tons of manual labor, and does
not scale well. This leaves us with (2).

To go with (2) we first need to address the following issues:
# We have test classes that essentially act as both unit tests, and integration tests (from
{{maven}}'s perspective). Such classes are characterised by having both regular test methods,
and test methods decorated with {{@NeedsRunner}} or {{@RunnableOnService}} which are run by
{{maven-surefire-plugin}} during the {{integration-test}} phase (as opposed to the {{test}}
phase for regular test methods). From my experiments this scenario is not well accommodated
^5^ in {{jacoco-maven-plugin}}.
# Reassigning {{@NeedsRunner}} and {{@RunnableOnService}}  based tests to be triggered during
the {{test}} phase does not do the trick, probably because they scan dependencies (for tests)
which fail to be ready when the {{test}} phase executes. I assume this was the reason they
were configured to run as part of the {{verify}} phase in the first place.
# I believe these issues can be alleviated by moving {{@NeedsRunner}} and {{@RunnableOnService}}
to separate *integration test* classes which will execute during the {{integration-test}}
phase and thus avoid interfering with the tests that run during the {{test}} phase.

My impression on this is that it is doable, but the benefit-cost ratio seems to be quite questionable.
Perhaps we could look at external tools such as [SonarQube|https://www.sonarqube.org] which
specialise in providing software metrics such as test coverage and the likes.
There is also a [Jenkins JaCoCo Plugin | https://wiki.jenkins-ci.org/display/JENKINS/JaCoCo+Plugin],
which I have not looked into.

1. https://groups.google.com/forum/#!searchin/jacoco/%22multi$20module%22$20coverage%7Csort:relevance
2. https://github.com/jacoco/jacoco/wiki/MavenMultiModule
3. https://www.google.co.il/search?q=jacoco+multi+module+coverage&oq=jacoco+multi+module+coverage&aqs=chrome.0.0j69i57j69i60l3.7899j0j4&sourceid=chrome&ie=UTF-8
4. http://www.eclemma.org/jacoco/trunk/doc/ant.html
5. http://www.eclemma.org/jacoco/trunk/doc/classids.html

> Code coverage numbers are not accurate
> --------------------------------------
>
>                 Key: BEAM-1399
>                 URL: https://issues.apache.org/jira/browse/BEAM-1399
>             Project: Beam
>          Issue Type: Bug
>          Components: build-system, sdk-java-core, testing
>            Reporter: Daniel Halperin
>              Labels: newbie, starter
>
> We've started adding Java Code Coverage numbers to PRs using the jacoco plugin. However,
we are getting very low coverage reported for things like the Java SDK core.
> My belief is that this is happening because we test the bulk of the SDK not in the SDK
module , but in fact in the DirectRunner and other similar modules.
> JaCoCo has a {{report:aggregate}} target that might do the trick, but with a few minutes
of playing with it I wasn't able to make it work satisfactorily. Basic work in https://github.com/apache/beam/pull/1800
> This is a good "random improvement" issue for anyone to pick up.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Mime
View raw message