8000 Enable running prepare-agent goal within a pom packaged project by eswdd · Pull Request #119 · jacoco/jacoco · GitHub
[go: up one dir, main page]

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

Enable running prepare-agent goal within a pom packaged project #119

Closed
wants to merge 3 commits into from

Conversation

eswdd
Copy link
@eswdd eswdd commented Jul 24, 2013

Provides a mechanism to allow subclasses of AbstractJacocoMojo to allow pom packaged projects, and use that within the AgentMojo. See also #118

@ghost ghost assigned brockj and Godin Jul 24, 2013
@marchof
Copy link
Member
marchof commented Jul 24, 2013

@Godin Can you please review this?

@Godin
Copy link
Member
Godin commented Jul 25, 2013

@marchof Added into todo-list.

@mdekstrand
Copy link

I'd also like to see the merge and report goals allowed in a POM project. I'm wanting to use it to collect sub-project results to report overall code coverage in the aggregator project.

@marchof
Copy link
Member
marchof commented Nov 16, 2013

@mfriedenhagen What do you think about this request?

@mfriedenhagen
Copy link
Member

I can see a value in having pure pom projects with the ability for merge and report (e.g. merging data from multi-module builds). However I would like to see an integration test which "proves" that this is working as intended by @eswdd.

@eswdd
Copy link
Author
eswdd commented Nov 18, 2013

@mfriedenhagen I've written the integration test for what I wanted to achieve with this pull request. I can also see the value in having pure pom projects for merge and report, but I don't have a need for that, so haven't coded for it.

The buildhive issue looks like it's attempting to auto merge and failing, which is odd since I've rebaselined this branch this evening.

@ghost ghost assigned mfriedenhagen Dec 7, 2013
@marchof
Copy link
Member
marchof commented Dec 7, 2013

@mfriedenhagen Mirko, may I ask you to review and merge this PR?

@mfriedenhagen
Copy link
Member

I changed the milestone to 0.6.5 as @Godin is already running preparations for the 0.6.4 release.

@mfriedenhagen
Copy link
Member

@marchof, @Godin:

  • Skip jacoco instrumentation for mvn modules with package type ear #169 introduces a check for the existence of target/classes in the report-mojo.
  • I think we might drop the POM check completely, as setting the property in prepare-agent is probably not taking much more time then skipping completely and the report should be skipped because target/classes does not exist. What do you think?

@marchof
Copy link
Member
marchof commented Dec 16, 2013

@mfriedenhagen I fully agree with you here about the prepare-agent goal. Especially as there can be cases where a project execute tests but does not have its own target/classes folder.

The original request was for AbstractJacocoMojo. At least for the instrument goal it might make sense to skip it there is no target/classes.

@mfriedenhagen
Copy link
Member

@marchof: IMO we have to differentiate between mojos, some need target/classes, some do not. I would implement checks where needed instead of delegating to the AbstractJacocoMojo which has not enough information to decide.

@Godin
Copy link
Member
Godin commented Dec 16, 2013

@mfriedenhagen Indeed - this is reasonable.

@mfriedenhagen
Copy link
Member

#171 removes the check for pom projects and instead checks in the relevant mojos for the existence of target/classes where appropriate. This also means, prepare-agent always set the agent property.

@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.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
0