Multi-report merging (within single project) #18

Closed
marchof opened this Issue Aug 29, 2012 · 12 comments

Comments

Projects
None yet
7 participants
@marchof
Member

marchof commented Aug 29, 2012

gelephantastic@SF

There are a few threads about Maven plugin datafile merging. I'd like to write in about the 2 major Maven merge use cases so you're aware. One is trivial, one is not. The trivial case is for multiple test types: unit tests, functional tests, and perhaps even integration tests or acceptance tests (like Concordion). For this case, I would expect that you could list additional, optional extra report files to conditionally be merged in at the report generation phase. This is trivial because it's all within a single Maven module. The harder case is merging the data file in a multi-project module, where I may want the parent pom to identify every report file that was used in any reactor module, and merge that into a master report for the whole build. This would ideally include all the "extra" reports from the trivial case above. I might try to code up the first use case. I expect I will tackle the problem exactly as I've described; the main dataFile parameter (defaulted to target/jacoco.exec) will still be expected. I might add either a list of files to (optionally) merge, or a directory that contains all the additional data files to be merged. It might be good to switch from target/jacoco.exec to using a directory, like target/jacoco/execution.exec, and then have the default behavior to merge everything in there, perhaps even then outputting the merged report to target/jacoco.exec. It's getting fuzzy here, since I'm out on a limb, but just a thought.

marchof

I like the idea to create a report on multiple exec-Files (like the Ant task also supports).

But to be honest I don't like the idea to recursively search directories. This might cause searching thousands of directories. Also there might be other *.exec files in the directories that should not be considered.

What is best practice here? Can't we supply a file set?

gelephantastic@SF

I picked the recursion simply because it was the smallest change (and also backwards compatible), but I certainly agree with your concerns. I don't know what the best practice is.. probably some sort of includes/excludes and a directory, or just statically listing all the reports that are expected?

In the case of the latter, do you use an API-breaking dataFiles/dataFile arrangement in the XML config, or do you keep dataFile and add an extraDataFiles/dataFile configuration block?

In the case of the former, you would keep a single dataFile but also add, I suppose, a dataFileFilter/include=*.exec or something of that nature. That option still requires recursion, but at least it is selective about what it picks up instead of trying to merge everything.

marchof

We can add a new property ("dataFiles") and deprecate the old one.

gelephantastic@SF

I think that's a good approach. I'm trying to figure out the "best practice" way of writing a Maven report plugin, to see how hard it would be to enable merging for a multi-module project. If I can just aggregrate the dataFiles list from all executed sub-modules, it will be a snap.

I'll fix my patch and re-attach (sans the multi-module investigation).

gelephantastic@SF

One thing I didn't address in this patch is handling the absence of report files. An exception will likely be raised for any desired report file that is not, in fact, present at the time of report generation. This is probably the desired behavior, though.

I dredged this up (http://docs.codehaus.org/display/MAVEN/Atypical+Plugin+Use+Cases) while thinking about multi-module merging/aggregation. I reviewed the Javadoc plugin, JXR, etc. - they have all ceased to use @Aggregator and resort to rather interesting tactics to handle multi-module reports. One of the main problems is that there are essentially 3 environments:

  1. Run as a report plugin within the Maven 3 site plugin.
  2. Inherited as a build plugin from the parent pom.
  3. Run once from an aggregator POM (no inheritance).

From what I can tell, this mess hasn't really been cleaned up in Maven 3 at all. @Aggregator is still around, the summarize phase doesn't exist, and parent/child ordering (in terms of when you want your plugin to run) is still tightly coupled to parent/child linkage declarations in the POM. Yuck.

kbrockhoff@SF

I took a different approach to aggregation. I have attached a diff with the changes I made to get my approach to work.

I define two profiles in my pom defining the build. One runs jacoco:agent with aggregate=true and then executes the unit tests. The other profile runs jacoco:agent and then executes both unit and integration tests. Both profiles result in a single jacoco.exec which is in the ${project.build.directory} of the execution root. Due to the issues discussed above about @Aggregator, I then have to run jacoco:report standalone to actually generate the report.

So the commands from the top level project are:

mvn clean install -Ptest.coverage (or -Pit.coverage)
mvn jacoco:report -Daggregate=true

gelephantastic@SF

This is a tough issue not made easier by Maven - I really wish they had added that summarize phase. I like your solution, but in my case, I can only make a single invocation of Maven (CI, business processes, separate scratch areas for build plans, all that jazz). Locally I can do whatever, I'm referring to our build infrastructure. Although perhaps with machinations I can trigger a dependent plan with the same scratch area.

Unrelated, I recommend you use variables and profile activation rather than setting profiles directly. So, mvn clean install -Dit.coverage; then you can activate any number of granular build options - you can, for example, choose to include test coverage with IT coverage if you like. I think I would want something like your solution AND something like mine, so I can run multiple invocations (test + IT) in a single build and aggregate both intra-module and inter-module reports such as you do here.

veithen@SF

There is another issue with kbrockhoff's changes. It will only work with multi-module projects where the root POM is also the parent POM. This is not necessarily the case. For projects that don't have this property, the execution fails with a NullPointerException. This is caused by the following piece of code in the patch:

while (!prj.isExecutionRoot()) {
  prj = prj.getParent();
}

Not sure how this should be fixed.

gelephantastic@SF

Nice catch. I did a little digging. I wish Maven actually had JavaDocs?, but anyway... MavenSession? has a "getProjects" method that might be usable to find all the projects that are targeted for building in the current execution context. There's also a getTopLevelProject() utility method.

It's probably a gross violation of Maven, and it might not even be possible if the ProjectDependencyGraph? implementation returns unmodifiable collections, but I wonder if it would be possible to insert a fake MavenProject? at the end of the sorted project list as a faux "summary" phase - you just clone the top level project and move the reporting execution. Somehow.

@scottcarey

This comment has been minimized.

Show comment
Hide comment
@scottcarey

scottcarey Mar 26, 2013

Perhaps look at what the javadoc plugin does for its aggregation goal.

Perhaps look at what the javadoc plugin does for its aggregation goal.

@melloware

This comment has been minimized.

Show comment
Hide comment
@melloware

melloware May 10, 2013

This would be an awesome feature as not having aggregated reports makes it difficult to get a true code coverage output. Thanks for sticking with this and keep up the great work!

This would be an awesome feature as not having aggregated reports makes it difficult to get a true code coverage output. Thanks for sticking with this and keep up the great work!

@Uko

This comment has been minimized.

Show comment
Hide comment

Uko commented May 23, 2013

👍

@Godin Godin modified the milestones: 0.6.6, 0.6.5 Mar 3, 2014

@Godin Godin modified the milestones: 0.7.2, 0.7.1 May 8, 2014

@AlexanderWillner

This comment has been minimized.

Show comment
Hide comment
@AlexanderWillner

AlexanderWillner Aug 1, 2014

Is this issue really on the roadmap for 0.7.2? Any plans to support multi module projects any time soon?

Is this issue really on the roadmap for 0.7.2? Any plans to support multi module projects any time soon?

@Godin Godin modified the milestones: 0.7.2, 0.7.3 Sep 12, 2014

@Godin Godin modified the milestones: 0.7.4, 0.7.3, 0.7.5 Feb 20, 2015

@velo

This comment has been minimized.

Show comment
Hide comment
@velo

velo Mar 24, 2015

Any hope on this?

If I update this patch and make a PR is there something Jacoco team would evaluate?

velo commented Mar 24, 2015

Any hope on this?

If I update this patch and make a PR is there something Jacoco team would evaluate?

@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

Godin Mar 24, 2015

Member

@velo we already have multiple patches in a different manner regarding this same functionality - see #217 , #97 , #69, so don't think that new one will help to get this out quicker. For me seems that there is multiple approaches in usage and so implementation of a such feature, so we need to clearly specify requirements before jumping into implementation and before review of proposed patches. Taking into account massive interest in such feature, I'll do my best to allocate time on upcoming weekend to sort things out and start progress on this. Thanks for your understanding.

Member

Godin commented Mar 24, 2015

@velo we already have multiple patches in a different manner regarding this same functionality - see #217 , #97 , #69, so don't think that new one will help to get this out quicker. For me seems that there is multiple approaches in usage and so implementation of a such feature, so we need to clearly specify requirements before jumping into implementation and before review of proposed patches. Taking into account massive interest in such feature, I'll do my best to allocate time on upcoming weekend to sort things out and start progress on this. Thanks for your understanding.

@Godin Godin modified the milestones: Maven multi-module projects, 0.7.5 Mar 24, 2015

@Godin Godin self-assigned this Mar 24, 2015

@velo

This comment has been minimized.

Show comment
Hide comment
@velo

velo Mar 24, 2015

Ok @Godin if you need any help lemme know!

velo commented Mar 24, 2015

Ok @Godin if you need any help lemme know!

@scottcarey

This comment has been minimized.

Show comment
Hide comment
@scottcarey

scottcarey Mar 24, 2015

Maven 3.2.1 added the ability for a plugin to hook into when the entire reactor finishes, for global things like this that need to know when everything is done and do work for the whole build instead of each project. That would probably be great for this. http://maven.apache.org/docs/3.2.1/release-notes.html
https://jira.codehaus.org/browse/MNG-5389

But would require users to be on at least maven 3.2.1.

My use case is most like #97. I need transitive coverage reports on a build with 100+ total modules, all in one place, including transitive coverage. If I had this, I would not need per project reports at all, just the global aggregate. Essentially, I want the coverage of every item in the entire reactor (all projects) from all tests. It is a maven best practice in certain circumstances to move the test code out of the module being tested. Without this feature, the coverage is 0%!

Maven 3.2.1 added the ability for a plugin to hook into when the entire reactor finishes, for global things like this that need to know when everything is done and do work for the whole build instead of each project. That would probably be great for this. http://maven.apache.org/docs/3.2.1/release-notes.html
https://jira.codehaus.org/browse/MNG-5389

But would require users to be on at least maven 3.2.1.

My use case is most like #97. I need transitive coverage reports on a build with 100+ total modules, all in one place, including transitive coverage. If I had this, I would not need per project reports at all, just the global aggregate. Essentially, I want the coverage of every item in the entire reactor (all projects) from all tests. It is a maven best practice in certain circumstances to move the test code out of the module being tested. Without this feature, the coverage is 0%!

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Mar 25, 2015

Member

@Godin BTW, I started documenting use cases here: https://github.com/jacoco/jacoco/wiki/MavenMultiModule

Member

marchof commented Mar 25, 2015

@Godin BTW, I started documenting use cases here: https://github.com/jacoco/jacoco/wiki/MavenMultiModule

@velo

This comment has been minimized.

Show comment
Hide comment
@velo

velo Mar 25, 2015

FWIW, how I use jacoco + modules today:

I made maven resource plugin copy /target//*.class to my parent target!
Then I run jacoco:merge and jacoco:report

https://github.com/velo/maven-formatter-plugin/blob/master/pom.xml#L233
+
$ mvn resources:resources jacoco:merge jacoco:report

velo commented Mar 25, 2015

FWIW, how I use jacoco + modules today:

I made maven resource plugin copy /target//*.class to my parent target!
Then I run jacoco:merge and jacoco:report

https://github.com/velo/maven-formatter-plugin/blob/master/pom.xml#L233
+
$ mvn resources:resources jacoco:merge jacoco:report

tnguyen1 added a commit to bonitasoft/bonita-web that referenced this issue Nov 5, 2015

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Mar 22, 2016

Member

I propose a solution to this issue with PR #388.

Member

marchof commented Mar 22, 2016

I propose a solution to this issue with PR #388.

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Apr 19, 2016

Member

Fixed with #388.

Member

marchof commented Apr 19, 2016

Fixed with #388.

@marchof marchof closed this Apr 19, 2016

@Godin Godin modified the milestones: 0.7.7, Maven multi-module projects Jun 10, 2016

@pedrorijo91 pedrorijo91 referenced this issue in codacy/codacy-coverage-reporter Jun 24, 2016

Closed

Unknown argument #26

@jacoco jacoco locked and limited conversation to collaborators Jan 11, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.