You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by duncangrant <gi...@git.apache.org> on 2015/01/16 10:48:39 UTC
[GitHub] incubator-brooklyn pull request: Cobertura to use maven plugin rat...
GitHub user duncangrant opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/455
Cobertura to use maven plugin rather than ant-run
PLEASE DON'T MERGE YET
Seems to lose ability to output xml but does give us an aggregated report.
Also when run it seems to compile twice (presumably once without and then once with instrumentation) and you get warnings of the form:
WARNING: Duplicate name in Manifest: Manifest-Version. Ensure that the manifest does not have duplicate entries, and
that blank lines separate individual sections in both your
manifest and in the META-INF/MANIFEST.MF entry in the jar file.
You seem to also get these warnings if you try to do an incremental build so I don't know if they're important.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/duncangrant/incubator-brooklyn cobertura
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/455.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #455
----
commit a10159f1415a814d5e1c593d3ee91d7e57741a5e
Author: Duncan Grant <du...@cloudsoftcorp.com>
Date: 2015-01-12T15:43:57Z
Cobertura to use maven plugin rather than ant-run
Seems to lose ability to output xml
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Cobertura to use maven plugin rat...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/455#issuecomment-70824479
I am using Apple's Java 1.6.
Turns out that apple's build doesn't have tools.jar, it's merged with classes.jar. Cobertura depends on it so it fails.
Here's a related issue https://jira.codehaus.org/browse/MCOBERTURA-180.
Possible (temporary) solution: http://permalink.gmane.org/gmane.comp.java.maven-plugins.mojo.user/4320
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Cobertura to use maven plugin rat...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/455#issuecomment-72212674
It's fine by me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Cobertura to use maven plugin rat...
Posted by duncangrant <gi...@git.apache.org>.
Github user duncangrant commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/455#issuecomment-70823014
What java version are you using? I'm using oracle jdk 1.7 - Alasdair thinks it might be to do with this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Cobertura to use maven plugin rat...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/455#discussion_r23292264
--- Diff: pom.xml ---
@@ -824,11 +823,11 @@
<artifactId>build-helper-maven-plugin</artifactId>
<version>1.7</version>
</plugin>
- <plugin>
+ <!-- <plugin>
--- End diff --
Remove if not used.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Cobertura to use maven plugin rat...
Posted by duncangrant <gi...@git.apache.org>.
Github user duncangrant closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/455
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Cobertura to use maven plugin rat...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/455#discussion_r23292305
--- Diff: usage/rest-server/pom.xml ---
@@ -246,7 +246,12 @@
</resource>
</resources>
<plugins>
-
+ <plugin>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <compilerId>javac</compilerId>
--- End diff --
What's the reason for overriding the compiler here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Cobertura to use maven plugin rat...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/455#issuecomment-76696284
if tests running twice is the normal thing then fine
would be nice to move to this "proper" build but no urgency, i support closing it, but no forgetting about it!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Cobertura to use maven plugin rat...
Posted by duncangrant <gi...@git.apache.org>.
Github user duncangrant commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/455#issuecomment-76410811
The current ant-run based integration does produce output.xml. This is useful for integration with jenkins and if we want to produce reports from the output. For this reason I am going to close rather than merge until I can investigate the issue.
I hadn't notice it running the tests twice before. According to @grkvlt this is because cobertura runs the tests once to confirm they pass and then instruments them and runs them again.
I haven't ever managed to get the integration tests to run on my machine so I still don't know what the coverage will look like for those. Presumably it is possible to get both unit tests and integration.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Cobertura to use maven plugin rat...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/455#issuecomment-76402038
meant also to say: it works well for me, on os x. no `output.xml` as duncan g says. i don't know if that matters (or indeed if the current antrun-based integration generates any such file).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Cobertura to use maven plugin rat...
Posted by duncangrant <gi...@git.apache.org>.
Github user duncangrant commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/455#issuecomment-72212305
The suggested fix is quite messy. Perhaps it would be better to just accept that we can't run coverage on a mac running 1.6?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Cobertura to use maven plugin rat...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/455#issuecomment-70821148
I tried running the tests with `mvn clean install -PCoverage` and get the following error:
```
[ERROR] Failed to execute goal org.codehaus.mojo:cobertura-maven-plugin:2.6:instrument (default) on project brooklyn-parent: Execution default of goal org.codehaus.mojo:cobertura-maven-plugin:2.6:instrument failed: Plugin org.codehaus.mojo:cobertura-maven-plugin:2.6 or one of its dependencies could not be resolved: Could not find artifact com.sun:tools:jar:0 at specified path /System/Library/Java/JavaVirtualMachines/1.6.0.jdk/Contents/Home/../lib/tools.jar -> [Help 1]
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Cobertura to use maven plugin rat...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/455#issuecomment-76401478
dropping java1.6 support coverage sounds fine
this runs fine even without #534 (fixed problem with antrun cobertura); it must be using a different directory structure.
however not only does this compile twice (which is understandable at least in `/src/main` as we need the instrumented and non-instrumented versions of classes), this **runs** all the tests twice which seems bad; acceptable but a major slow-down.
do we have a way to run the coverage check with both unit and integration tests activated? the latter is important especially for packages such as `ssh`.
@duncangrant do you suggest we merge this now (with the faults which have been noted here) or close this and you reopen it when ready?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---