Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parameter to include the current project in the aggregated report #1007

Merged
merged 5 commits into from Jun 27, 2022
Merged

Add parameter to include the current project in the aggregated report #1007

merged 5 commits into from Jun 27, 2022

Conversation

lion7
Copy link
Contributor

@lion7 lion7 commented Jan 30, 2020

This PR tries to solve the issue described in #812.
It is similar to #430, #680 and #970 but with the #680 (comment) of @marchof and #812 (comment) of @Godin taken into account.

@ramachandranms
Copy link

@marchof, @Godin

gentle bump, if this has fallen through cracks. would be great to have this feature as the workarounds are not easier to understand and get through. greatly appreciate a review to merge this. thanks in advance.

@pertsodian
Copy link

pertsodian commented Mar 20, 2020

Hi @marchof & @Godin, can we please review this PR? I think there are already quite a number of requests for this feature.

@kakawait
Copy link

kakawait commented Jun 1, 2020

Since the way that PR is dev, aka not breaking changes. So by reviewing it and merging it, you'll make many devs happy no and without breaking existing projects? :)

@lordjaxom
Copy link

+1
Would save us the hassle of introducing aggregator projects in 50+ repositories.

@marchof marchof added this to the 0.8.6 milestone Jun 9, 2020
@marchof marchof added this to Implementation in Current work items via automation Jun 9, 2020
@lordjaxom
Copy link

Just to state a report, we are currently using jacoco 0.8.5 + issue-812 (built locally) successfully within 50+ repositories to have full and accurate coverage reports with SonarQube without having to fuss with any projects' pom (both report-aggregate and SonarQube are configured in a global parent pom of our project tree).

@marchof
Copy link
Member

marchof commented Jun 11, 2020

@lordjaxom Can you describe the structure of your repos and shortly explain why this modification helps? I want to understand the use case. Thx!

@lordjaxom
Copy link

We mostly have multi-module-projects for EJB components which are separated into shared and server modules. The shared part contains interfaces, DTOs and utility classes, while the server part contains entities and implementations.

Since structurally sometimes the tests are located within the server part (even though the test covers DTO and utility classes) or integration tests cover multiple components at once, we would need aggregate reports over two or more modules. Ideally those would be generated within the server part, covering both server and shared.

Without this flag, we would either have to aggregate ourselves the standard report (covering only the server part) and the aggregate report (covering only the shared part since only dependencies are scanned) or introduce a third module as suggested in various documents to cover classes from both modules. Since "report" only actually reports coverage for classes present in the current module (not dependencies) and "report-aggregate" only reports coverage for classes from the dependencies, not the project itself.

@zuza-str
Copy link

@marchof are there any other concerns to merge this change? It would really make the life easier for many JaCoCo users :)

Copy link

@henrik242 henrik242 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@henrik242 henrik242 mentioned this pull request Sep 15, 2020
@Godin Godin removed this from the 0.8.6 milestone Sep 15, 2020
@twoodhouse
Copy link

My company is being forced to use aggregator modules in lots of projects to get around this. We are looking forward to this option being merged!

@cai3178940
Copy link

LGTM

@bogaertg
Copy link

up

@jycr
Copy link

jycr commented Jan 29, 2022

@kakawait , @henrik242, @twoodhouse: what is missing now to be able to merge this PR into master and add this feature in the next version?

@lyoumi
Copy link

lyoumi commented Jun 1, 2022

@lion7 @henrik242 @kakawait do we have any estimates for merging this PR to master?

@kakawait
Copy link

kakawait commented Jun 1, 2022

I'm not maintainer at all of Jacoco. I can't help you

@lion7
Copy link
Contributor Author

lion7 commented Jun 1, 2022

I'm willing to update this PR if one of the committers is willing to take a look at it.
Perhaps @Godin if he has time?

@marchof
Copy link
Member

marchof commented Jun 1, 2022

Hi @lion7, maintainer here. Thanks for your patience. Let's give it a try!

@lion7
Copy link
Contributor Author

lion7 commented Jun 1, 2022

@marchof great! 😄. I redid my changes on the current master, it should be ok now🤞🏻

@lyoumi
Copy link

lyoumi commented Jun 7, 2022

@marchof @lion7 just to follow up this review. Do we have any more concerns about this PR?

@marchof
Copy link
Member

marchof commented Jun 8, 2022

@lyoumi My concern is that all builds are broken due to a source formatting issue. I can help to fix this when I‘m back from vacation next week.

@lion7
Copy link
Contributor Author

lion7 commented Jun 8, 2022

@lyoumi My concern is that all builds are broken due to a source formatting issue. I can help to fix this when I‘m back from vacation next week.

Oh, I totally missed that in my last commit. I will try to fix the build asap 🙂

@marchof marchof self-assigned this Jun 21, 2022
@marchof marchof requested a review from Godin June 21, 2022 09:12
@marchof
Copy link
Member

marchof commented Jun 21, 2022

@Godin May you also have a look at this?

@Godin Godin merged commit af33753 into jacoco:master Jun 27, 2022
Current work items automation moved this from Implementation to Done Jun 27, 2022
@Godin Godin added this to the 0.8.9 milestone Jun 27, 2022
@lion7 lion7 deleted the issue-812 branch June 27, 2022 16:55
@froque
Copy link

froque commented Oct 6, 2022

Any chance of releasing 0.8.9 with this change ?

@m-herold
Copy link

Can't wait to start using the new includeCurrentProject parameter. Are there already plans when the new version 0.8.9 will be released?

ndwnu pushed a commit to ndwnu/nls-routing-map-matcher that referenced this pull request Apr 10, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [org.apache.maven.plugins:maven-compiler-plugin](https://maven.apache.org/plugins/) | build | minor | `3.10.1` -> `3.11.0` |
| [org.jacoco:jacoco-maven-plugin](https://www.jacoco.org/jacoco/trunk/doc/maven.html) ([source](https://github.com/jacoco/jacoco)) | build | patch | `0.8.8` -> `0.8.10` |
| [com.graphhopper:graphhopper-map-matching](https://www.graphhopper.com) ([source](https://github.com/graphhopper/graphhopper)) | compile | patch | `7.0` -> `7.0-testgithub6` |
| [com.graphhopper:graphhopper-core](https://www.graphhopper.com) ([source](https://github.com/graphhopper/graphhopper)) | compile | patch | `7.0` -> `7.0-testgithub6` |
| [org.springframework.boot:spring-boot-starter-parent](https://spring.io/projects/spring-boot) ([source](https://github.com/spring-projects/spring-boot)) | parent | patch | `3.0.5` -> `3.0.6` |

---

### Release Notes

<details>
<summary>jacoco/jacoco</summary>

### [`v0.8.10`](https://github.com/jacoco/jacoco/releases/tag/v0.8.10): 0.8.10

[Compare Source](jacoco/jacoco@v0.8.9...v0.8.10)

#### Fixed bugs

-   Agent should not require configuration of permissions for `SecurityManager` outside of its `codeBase` (GitHub [#&#8203;1425](jacoco/jacoco#1425)).

### [`v0.8.9`](https://github.com/jacoco/jacoco/releases/tag/v0.8.9): 0.8.9

[Compare Source](jacoco/jacoco@v0.8.8...v0.8.9)

#### New Features

-   JaCoCo now officially supports Java 19 and 20 (GitHub [#&#8203;1371](jacoco/jacoco#1371), [#&#8203;1386](jacoco/jacoco#1386)).
-   Experimental support for Java 21 class files (GitHub [#&#8203;1386](jacoco/jacoco#1386)).
-   Add parameter to include the current project in the `report-aggregate` Maven goal (GitHub [#&#8203;1007](jacoco/jacoco#1007)).
-   Component accessors generated by the Java compilers for records are filtered out during generation of report. Contributed by Tesla Zhang (GitHub [#&#8203;1393](jacoco/jacoco#1393)).

#### Fixed bugs

-   Agent should not open `java.lang` package to unnamed module of the application class loader (GitHub [#&#8203;1334](jacoco/jacoco#1334)).

#### Non-functional Changes

-   JaCoCo now depends on ASM 9.5 (GitHub [#&#8203;1299](jacoco/jacoco#1299), [#&#8203;1368](jacoco/jacoco#1368), [#&#8203;1416](jacoco/jacoco#1416)).
-   JaCoCo build now requires JDK 11 (GitHub [#&#8203;1413](jacoco/jacoco#1413)).

</details>

<details>
<summary>graphhopper/graphhopper</summary>

### [`v7.0-pre2`](graphhopper/graphhopper@7.0-pre1...7.0-pre2)

[Compare Source](graphhopper/graphhopper@7.0-pre1...7.0-pre2...
ndwlocatieservices added a commit to ndwnu/nls-routing-map-matcher that referenced this pull request Apr 16, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [org.apache.maven.plugins:maven-compiler-plugin](https://maven.apache.org/plugins/) | build | minor | `3.10.1` -> `3.11.0` |
| [org.jacoco:jacoco-maven-plugin](https://www.jacoco.org/jacoco/trunk/doc/maven.html) ([source](https://github.com/jacoco/jacoco)) | build | patch | `0.8.8` -> `0.8.10` |
| [com.graphhopper:graphhopper-map-matching](https://www.graphhopper.com) ([source](https://github.com/graphhopper/graphhopper)) | compile | patch | `7.0` -> `7.0-testgithub6` |
| [com.graphhopper:graphhopper-core](https://www.graphhopper.com) ([source](https://github.com/graphhopper/graphhopper)) | compile | patch | `7.0` -> `7.0-testgithub6` |
| [org.springframework.boot:spring-boot-starter-parent](https://spring.io/projects/spring-boot) ([source](https://github.com/spring-projects/spring-boot)) | parent | patch | `3.0.5` -> `3.0.6` |

---

### Release Notes

<details>
<summary>jacoco/jacoco</summary>

### [`v0.8.10`](https://github.com/jacoco/jacoco/releases/tag/v0.8.10): 0.8.10

[Compare Source](jacoco/jacoco@v0.8.9...v0.8.10)

#### Fixed bugs

-   Agent should not require configuration of permissions for `SecurityManager` outside of its `codeBase` (GitHub [#&#8203;1425](jacoco/jacoco#1425)).

### [`v0.8.9`](https://github.com/jacoco/jacoco/releases/tag/v0.8.9): 0.8.9

[Compare Source](jacoco/jacoco@v0.8.8...v0.8.9)

#### New Features

-   JaCoCo now officially supports Java 19 and 20 (GitHub [#&#8203;1371](jacoco/jacoco#1371), [#&#8203;1386](jacoco/jacoco#1386)).
-   Experimental support for Java 21 class files (GitHub [#&#8203;1386](jacoco/jacoco#1386)).
-   Add parameter to include the current project in the `report-aggregate` Maven goal (GitHub [#&#8203;1007](jacoco/jacoco#1007)).
-   Component accessors generated by the Java compilers for records are filtered out during generation of report. Contributed by Tesla Zhang (GitHub [#&#8203;1393](jacoco/jacoco#1393)).

#### Fixed bugs

-   Agent should not open `java.lang` package to unnamed module of the application class loader (GitHub [#&#8203;1334](jacoco/jacoco#1334)).

#### Non-functional Changes

-   JaCoCo now depends on ASM 9.5 (GitHub [#&#8203;1299](jacoco/jacoco#1299), [#&#8203;1368](jacoco/jacoco#1368), [#&#8203;1416](jacoco/jacoco#1416)).
-   JaCoCo build now requires JDK 11 (GitHub [#&#8203;1413](jacoco/jacoco#1413)).

</details>

<details>
<summary>graphhopper/graphhopper</summary>

### [`v7.0-pre2`](graphhopper/graphhopper@7.0-pre1...7.0-pre2)

[Compare Source](graphhopper/graphhopper@7.0-pre1...7.0-pre2...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet