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 Artifact.SCOPE_PROVIDED artifacts to aggregate report #572

Merged
merged 1 commit into from
Aug 10, 2017
Merged

Add Artifact.SCOPE_PROVIDED artifacts to aggregate report #572

merged 1 commit into from
Aug 10, 2017

Conversation

chonton
Copy link
Contributor

@chonton chonton commented Aug 2, 2017

I have an aggregate maven project where one of the projects must be scoped as 'provided'. I would like it to be still considered for inclusion in the aggregate report.

@marchof
Copy link
Member

marchof commented Aug 2, 2017

@chonton Thanks for the contribution! May I ask you for your use case? Especially it looks like my current assumptions do not hold true:

  1. A separate project only for the purpose of creating the coverage report is required. In your case it looks like you can rely on a existing project.
  2. My understanding of usage of scope PROVIDED is a dependency on a common API (like Servlet) which is typically not part of the reactor itself. Our goal can only create reports for projects from the reactor.

@marchof marchof added the feedback pending Further information is requested label Aug 2, 2017
@chonton
Copy link
Contributor Author

chonton commented Aug 2, 2017

My typical reactor project has three modules:

  • api (interface jar)
  • service (implementation delivered as a docker image)
  • test (integration tests in a docker image)

Both service and test have a compile dependency on api. Additionally, test has a particular dependency on service; the service module must be built before test so that service docker image can be launched during pre-integration-test phase, and the classes from service should not be included in the compile or runtime classpath of test.

To prevent the service classes from being included in the test docker image, the dependency cannot be compile or runtime. The jacoco aggregate goal uses test to distinguish the projects that provide coverage without providing code.

This leaves provided, which seems correct, as the service is providing the implementation of api.

@marchof marchof removed the feedback pending Further information is requested label Aug 2, 2017
@marchof
Copy link
Member

marchof commented Aug 2, 2017

@chonton Indeed, this is nice use case where the aggregate-report works well. Thx!

@Godin Do you agree to merge this? If so I we need to add a change log.

@chonton
Copy link
Contributor Author

chonton commented Aug 7, 2017

@marchof and @Godin, anything else required to merge this PR?

@@ -46,7 +46,20 @@
* Project source and execution data is included in the report.</li>
* <li><code>test</code>: Only execution data is considered for the report.</li>
* </ul>
*
*
* A typical use case might be in a reactor project which has three modules:
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that we need this update of Javadoc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed in next commit.

@Godin Godin self-assigned this Aug 8, 2017
@Godin Godin added this to the 0.8.0 milestone Aug 8, 2017
@chonton
Copy link
Contributor Author

chonton commented Aug 10, 2017

anything else needed to merge this PR?

@Godin
Copy link
Member

Godin commented Aug 10, 2017

anything else needed to merge this PR?

@chonton not so much - I just need to come from vacation back to work 😉

@Godin Godin merged commit bb2ecb5 into jacoco:master Aug 10, 2017
@Godin
Copy link
Member

Godin commented Aug 10, 2017

@chonton merged. Thank you for your contribution! 👍

@jacoco jacoco locked as resolved and limited conversation to collaborators Jan 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants