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

JaCoCo Test Coverage #203

Closed
wants to merge 7 commits into from
Closed

JaCoCo Test Coverage #203

wants to merge 7 commits into from

Conversation

chrisruffalo
Copy link
Contributor

Added a test coverage profile that creates jacoco execution data for each module and then aggregates the report (with an ant task) in the coverage-report module.

I had to modify one test class a little bit so it would play nice with the instrumentation from the JaCoCo agent.

New modules will have to be added to the coverage report by hand when they are added.

@undertow-pull-request
Copy link

Build 1261 is now running using a merge of e82abc9

@undertow-pull-request
Copy link

Build 1261 outcome was SUCCESS using a merge of e82abc9
Summary: Tests passed: 1101, ignored: 64 Build time: 0:3:10

@ctomc
Copy link
Contributor

ctomc commented May 22, 2014

This looks as a good thing, but can you please make by using maven plugin not ant, at that goes against every build principal we are trying to follow.
One thing is to use ant where there is no other option but for jacoco there is perfectly fine maven plugin.

@chrisruffalo
Copy link
Contributor Author

There IS a perfectly good Maven plugin, you are correct.

However I could not get it to work with the combined coverage. As a matter
of fact, I think you can see it in the coverage-report module's pom.xml.
It would merge the jacoco.exe files and end up with a coverage report that
had 0 classes in it despite having the proper sessions. I think we could
get this to work by copying all of the class (and source?) files forward
into the appropriate coverage-report directories but that would be just as
much of a hack, don't you think?

The other option is to have testing reports that are constrained to their
individual modules which would be pretty doable.

Thoughts?

On Thu, May 22, 2014 at 4:35 PM, Tomaz Cerar notifications@github.comwrote:

This looks as a good thing, but can you please make by using maven plugin
not ant, at that goes against every build principal we are trying to follow.
One thing is to use ant where there is no other option but for jacoco
there is perfectly fine maven plugin.


Reply to this email directly or view it on GitHubhttps://github.com//pull/203#issuecomment-43940708
.

@undertow-pull-request
Copy link

Build 1262 is now running using a merge of e82abc9

@undertow-pull-request
Copy link

Build 1263 is now running using a merge of e82abc9

@undertow-pull-request
Copy link

Build 1263 outcome was FAILURE using a merge of e82abc9
Summary: Tests failed: 14 (11 new), passed: 1089, ignored: 61 Build time: 0:7:3

Failed tests

io.undertow.server.security.SpnegoDigestAuthenticationTestCase.testDigestSuccess: java.lang.AssertionError: expected:<200> but was:<401>
    at org.junit.Assert.fail(Assert.java:88)
    at org.junit.Assert.failNotEquals(Assert.java:743)

io.undertow.server.handlers.proxy.LoadBalancingProxyTestCase.testLoadSharedWithServerShutdown: java.lang.RuntimeException: java.net.BindException: Address already in use: bind
    at io.undertow.Undertow.start(Undertow.java:165)
    at io.undertow.server.handlers.proxy.AbstractLoadBalancingProxyTestCase.testLoadSharedWithServerShutdown(AbstractLoadBalancingProxyTestCase.java:95)

io.undertow.server.handlers.proxy.LoadBalancingProxyHttpsTestCase.testLoadSharedWithServerShutdown: java.lang.RuntimeException: java.net.BindException: Address already in use: bind
    at io.undertow.Undertow.start(Undertow.java:165)
    at io.undertow.server.handlers.proxy.AbstractLoadBalancingProxyTestCase.testLoadSharedWithServerShutdown(AbstractLoadBalancingProxyTestCase.java:95)

io.undertow.server.security.SpnegoDigestAuthenticationTestCase.testNonceCountReUse{proxy}: java.lang.AssertionError: expected:<200> but was:<401>
    at org.junit.Assert.fail(Assert.java:88)
    at org.junit.Assert.failNotEquals(Assert.java:743)

io.undertow.server.handlers.proxy.LoadBalancingProxyTestCase.testLoadSharedWithServerShutdown{proxy}: java.lang.RuntimeException: java.net.BindException: Address already in use: bind
    at io.undertow.Undertow.start(Undertow.java:165)
    at io.undertow.server.handlers.proxy.AbstractLoadBalancingProxyTestCase.testLoadSharedWithServerShutdown(AbstractLoadBalancingProxyTestCase.java:95)

io.undertow.server.handlers.proxy.LoadBalancingProxyHttpsTestCase.testLoadSharedWithServerShutdown{proxy}: java.lang.RuntimeException: java.net.BindException: Address already in use: bind
    at io.undertow.Undertow.start(Undertow.java:165)
    at io.undertow.server.handlers.proxy.AbstractLoadBalancingProxyTestCase.testLoadSharedWithServerShutdown(AbstractLoadBalancingProxyTestCase.java:95)

io.undertow.server.security.SpnegoDigestAuthenticationTestCase.testDigestSuccess{proxy}{ajp}: java.lang.AssertionError: expected:<200> but was:<401>
    at org.junit.Assert.fail(Assert.java:88)
    at org.junit.Assert.failNotEquals(Assert.java:743)

io.undertow.server.handlers.proxy.LoadBalancingProxyTestCase.testLoadSharedWithServerShutdown{proxy}{ajp}: java.lang.RuntimeException: java.net.BindException: Address already in use: bind
    at io.undertow.Undertow.start(Undertow.java:165)
    at io.undertow.server.handlers.proxy.AbstractLoadBalancingProxyTestCase.testLoadSharedWithServerShutdown(AbstractLoadBalancingProxyTestCase.java:95)

io.undertow.server.handlers.proxy.LoadBalancingProxyHttpsTestCase.testLoadSharedWithServerShutdown{proxy}{ajp}: java.lang.RuntimeException: java.net.BindException: Address already in use: bind
    at io.undertow.Undertow.start(Undertow.java:165)
    at io.undertow.server.handlers.proxy.AbstractLoadBalancingProxyTestCase.testLoadSharedWithServerShutdown(AbstractLoadBalancingProxyTestCase.java:95)

io.undertow.servlet.test.request.ExecutorPerServletTestCase.testSingleThreadExecutor{proxy}: java.util.concurrent.ExecutionException: java.lang.AssertionError: expected:<200> but was:<500>
    at java.util.concurrent.FutureTask.report(FutureTask.java:122)
    at java.util.concurrent.FutureTask.get(FutureTask.java:188)

io.undertow.servlet.test.request.ExecutorPerServletTestCase.testSingleThreadExecutor{proxy}{ajp}: java.util.concurrent.ExecutionException: java.lang.AssertionError: expected:<200> but was:<500>
    at java.util.concurrent.FutureTask.report(FutureTask.java:122)
    at java.util.concurrent.FutureTask.get(FutureTask.java:188)


##### there are 4 more failed tests, see build details

@ctomc
Copy link
Contributor

ctomc commented May 22, 2014

I want to test this a bit more, I think that that ant part wont even be needed, as our CI (teamcity) can parse the reports without it quite well.
Will report back.

@chrisruffalo
Copy link
Contributor Author

Ah! Good. I just wanted to be able to read them offline. Either way.

On Thursday, May 22, 2014, Tomaz Cerar notifications@github.com wrote:

I want to test this a bit more, I think that that ant part wont even be
needed, as our CI (teamcity) can parse the reports without it quite well.
Will report back.


Reply to this email directly or view it on GitHubhttps://github.com//pull/203#issuecomment-43945060
.

@chrisruffalo
Copy link
Contributor Author

So what we can do is enable individual reports inside projects and then have a final project merge them (the coverage-report project) and possibly just let team city read that one.

Or we can just do the .exec files. Either way it would be easy to set up. Let me know your preference and I can push the change.

@chrisruffalo
Copy link
Contributor Author

I also want to point out that JaCoCo pull request jacoco/jacoco#97 is about this and I just added a different take on it too (jacoco/jacoco#217);

Conflicts:
	core/src/test/java/io/undertow/util/HeaderOrderTestCase.java
@undertow-pull-request
Copy link

Build 1268 is now running using a merge of b6220c3

@undertow-pull-request
Copy link

Build 1268 outcome was FAILURE using a merge of b6220c3
Summary: Exit code 1 (new) Build time: 0:0:22

@undertow-pull-request
Copy link

Build 1273 is now running using a merge of b6220c3

@undertow-pull-request
Copy link

Build 1273 outcome was FAILURE using a merge of b6220c3
Summary: Exit code 1 Build time: 0:0:34

@undertow-pull-request
Copy link

Build 1275 is now running using a merge of b6220c3

@undertow-pull-request
Copy link

Build 1275 outcome was FAILURE using a merge of b6220c3
Summary: Exit code 1 Build time: 0:0:33

@stuartwdouglas
Copy link
Contributor

I have merged as is for now, if it turns out the ant part is not needed we can remove it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants