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

Regression in 0.7.9: "help" goal is not available in jacoco-maven-plugin #559

Merged
merged 1 commit into from Jul 4, 2017

Conversation

Projects
None yet
2 participants
@Godin
Member

Godin commented Jul 3, 2017

mvn org.jacoco:jacoco-maven-plugin:0.7.8:help

works, while

mvn org.jacoco:jacoco-maven-plugin:0.7.9:help

fails as well as

mvn org.jacoco:jacoco-maven-plugin:0.7.10-SNAPSHOT:help
@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Jul 3, 2017

Member

@Godin Looks like we should add an IT test for this 😉

Member

marchof commented Jul 3, 2017

@Godin Looks like we should add an IT test for this 😉

@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

Godin Jul 3, 2017

Member

I suppose that root cause was always or almost always here, but wasn't causing troubles because of release process, which was simplified in 0.7.9 by #211 revealing the issue.

Member

Godin commented Jul 3, 2017

I suppose that root cause was always or almost always here, but wasn't causing troubles because of release process, which was simplified in 0.7.9 by #211 revealing the issue.

@Godin Godin self-assigned this Jul 3, 2017

@Godin Godin added this to the 0.8.0 milestone Jul 3, 2017

@Godin Godin requested a review from marchof Jul 3, 2017

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Jul 4, 2017

Member

@Godin Anyways, wouldn't it make sense to add a simple IT test to ensure there is a help goal which produces some output? We have the same for CLI.

Member

marchof commented Jul 4, 2017

@Godin Anyways, wouldn't it make sense to add a simple IT test to ensure there is a help goal which produces some output? We have the same for CLI.

@marchof

marchof approved these changes Jul 4, 2017

@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

Godin Jul 4, 2017

Member

@marchof it just checks that there is help goal, but doesn't check content, what is IMO sufficient - we haven't seen problems with content so far, so why not trust generator

Member

Godin commented Jul 4, 2017

@marchof it just checks that there is help goal, but doesn't check content, what is IMO sufficient - we haven't seen problems with content so far, so why not trust generator

@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

Godin Jul 4, 2017

Member

@marchof on the other hand - I can add simple check of content, but anyway it won't catch complex regressions

Member

Godin commented Jul 4, 2017

@marchof on the other hand - I can add simple check of content, but anyway it won't catch complex regressions

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Jul 4, 2017

Member

@Godin Ok, thanks. I was confused as I haven't seen a separate test for this. The setup parent is used by all tests. This means the help goal is now executed within all tests?

Member

marchof commented Jul 4, 2017

@Godin Ok, thanks. I was confused as I haven't seen a separate test for this. The setup parent is used by all tests. This means the help goal is now executed within all tests?

@Godin

This comment has been minimized.

Show comment
Hide comment
@Godin

Godin Jul 4, 2017

Member

@marchof no - this means that when preparing (installing) setup-parent we'll test availability of help goal, i.e. only once. I did it this way to avoid addition of test, because each separate test is quite costly in terms of time.

Member

Godin commented Jul 4, 2017

@marchof no - this means that when preparing (installing) setup-parent we'll test availability of help goal, i.e. only once. I did it this way to avoid addition of test, because each separate test is quite costly in terms of time.

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Jul 4, 2017

Member

@Godin Ok, I see. Fine to me.

Member

marchof commented Jul 4, 2017

@Godin Ok, I see. Fine to me.

@Godin Godin merged commit 05a2aa7 into master Jul 4, 2017

4 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@Godin Godin deleted the issue-559 branch Jul 4, 2017

@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.