missing root project added

[momega]

No description provided.

[Godin]

Hi @momega ,

It is unclear, at least for me, both from message in mailing list and from commit message, what you are trying to achieve. To clarify - taking as an example project from our integration tests ( https://github.com/jacoco/jacoco/tree/master/jacoco-maven-plugin.test/it/it-report-aggregate ) this change will cause inclusion of source files of module "report" into the report produced in this module and has nothing to do with root. So, is it the goal or something else?

[momega]

Hi,

You are right, usage of words "root project" was not correct. Nevertheless
primary intention was to added the classes from module "report" from your
example . Also your test project "it-report-aggregate" generates site which
DOES NOT contain ReportTest class located report module.

During generating report/site original implementation loads all exec files
from all submodules and also from "report" module. But it omits "analyzing"
classes from report module. Here is the log from your project. I guest it
is wrong

[INFO] --- jacoco-maven-plugin:0.7.7.201606060606:report-aggregate
(report-aggregate) @ report ---
[INFO] Loading execution data file
/home/martin/git/jacoco/jacoco-maven-plugin.test/it/it-report-aggregate/report/target/jacoco.exec
[INFO] Loading execution data file
/home/martin/git/jacoco/jacoco-maven-plugin.test/it/it-report-aggregate/child1/target/jacoco.exec
[INFO] Loading execution data file
/home/martin/git/jacoco/jacoco-maven-plugin.test/it/it-report-aggregate/child1-test/target/jacoco.exec
[INFO] Loading execution data file
/home/martin/git/jacoco/jacoco-maven-plugin.test/it/it-report-aggregate/child2/target/jacoco.exec
[INFO] Analyzed bundle 'child1' with 2 classes
[INFO] Analyzed bundle 'child2' with 1 classes

So, yes, goal is add the source files from "report" module.

Best Regards,
MOmega

On Tue, Jun 28, 2016 at 9:01 AM, Evgeny Mandrikov notifications@github.com
wrote:

Hi @momega https://github.com/momega ,

It is unclear, at least for me, both from message in mailing list and from
commit message, what you are trying to achieve. To clarify - taking as an
example project from our integration tests (
https://github.com/jacoco/jacoco/tree/master/jacoco-maven-plugin.test/it/it-report-aggregate
) this change will cause inclusion of source files of module "report" into
the report produced in this module and has nothing to do with root. So, is
it the goal or something else?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#430 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAsk2_DZnwEKhy1vy8A5OnMJNb9Fsew3ks5qQMa0gaJpZM4I-yHW
.

[Godin]

Also your test project "it-report-aggregate" generates site which DOES NOT contain ReportTest class located report module.
...
But it omits "analyzing" classes from report module. Here is the log from your project. I guest it is wrong

Just for clarity: report won't contain file ReportTest.java even if module report will be included in analysis, because reports include main source files and not test files. And module report is not included into analysis not by mistake, it was done intentionally - see Javadoc in ReportAggregateMojo.java. So this is rather request for enhancement than bug fix.

This enhancement looks good to me and seems that such behaviour would match expectations of many users. @marchof do you have any objections, maybe something was overlooked by me what prevents us to do so?

@momega in case of absence of objections from @marchof this change will require some polishing - update of integration-test, javadoc and changelog, but before doing all this I suggest to wait a bit his response.

[marchof]

We haven't seen such a use case before. Typically you don't create coverage reports for tests itself.

@momega Can you please describe your use case a bit to get a better understanding why such a module structure might make sense?

The current implementation would always create an empty group for the report module itself. Which is not the desired bahaviour for the regular case.

[Godin]

@marchof if my memory is correct, then here we are talking about use case which showed up several times in mailing list ( including one of @momega ) - is when project contains main code, tests and also integration-tests

[marchof]

@Godin Ok, I see. In this case we still need a filter for the other cases to avoid empty groups in the report.

[adrianchrzastowski-kreditech]

Any plans to merge this pull request? This functionality still doesn’t work, version: 0.8.0.

//edit:
A few word about my needs.
I have project structure sth like it is shown below:

• loan-application
  • loan-aplication-common
  • loan-application-pl
  • loan-applicarion-de

these two last project are country specific, and both contain some code/tests.

Current setup of course is temporary but still I would like to:
build project only for Poland and have code coverage report agregated from modules:

• loan-aplication-common
• loan-application-pl

in one place.

I’ve tried to use report goal in loan-application-pl module but as it was described above, report doesn’t contain classes from: loan-application-pl module, as a work around I can configure loan-application-pl as a dependency in loan-application-pl module :/ .

[Godin]

@adrianchrzastowski-kreditech this was not accepted because #430 (comment) from @marchof was ignored by @momega I'm closing this PR as duplicate of #812 , which hopefully contains clean description of why this as well as almost identical #680 were not accepted.