@marchof Hi Marc, do you want me to do a review? I saw couple of TODOs, which I probably would be able to solve. Also if we will provide an offline mode, I'd like to be sure that it can be used for development for Android.

@Godin Absolutely, a review would be great!

Concerning the Android scenario it would be helpful to have a working example, or maybe at least some description how to integrate it with JaCoCo. I don't think I have a chance to work on this. Maybe then community jumps in once JaCoCo supports offline mode?

@marchof Ok, I'll do a review and testing. And I plan to experiment with scenario for Android, so maybe will provide an example. Also IMO we should enhance Maven Plugin to support offline instrumentation.

The Maven plug-in will be some work. For this we should really know the scenario why we take the hassle.

I see the offline feature as the last resort for very specific scenarios. I don't feel we should push this feature too much.

Scenario for Maven plugin is the same - to better support development for Android, i.e. to integrate into projects based on https://github.com/jayway/maven-android-plugin

But I agree that offline instrumentation should be a last resort.

@Godin Many thanks for making the commit hash work!

WDYT, can we merge this? Any concerns?

@marchof Currently I'm trying to use this for Android. For sure modification of Maven Plugin will be required for better integration. But not yet sure, if this will require other changes. So it's up to you to decide whether to merge or not. In last case - we can commit required changes later.

Sure, if you're working on an Android example we can wait until we see whether the current implementation actually does the job.

@marchof FYI : I spent a lot of time (around 5 hours) to make a "hello world" project for Android, and to use Emma for it (to understand how it works). Then I was able to instrument classes in-place with help of JaCoCo Ant Task. And run instrumented classes with JaCoCo Runtime under Android emulator. But now I'm a bit stuck as I don't know how to pass System properties in order to change destination of dump file. Here is a log from Android Emulator:

W/System.err(  937): java.io.FileNotFoundException: /jacoco.exec: open failed: EROFS (Read-only file system)
W/System.err(  937):    at libcore.io.IoBridge.open(IoBridge.java:416)
W/System.err(  937):    at java.io.FileOutputStream.<init>(FileOutputStream.java:88)
W/System.err(  937):    at org.jacoco.agent.rt_160d19c.controller.LocalController.startup(LocalController.java:46)
W/System.err(  937):    at org.jacoco.agent.rt_160d19c.Agent.startup(Agent.java:100)
W/System.err(  937):    at org.jacoco.agent.rt_160d19c.Agent.getInstance(Agent.java:45)
W/System.err(  937):    at org.jacoco.agent.rt_160d19c.RT.<clinit>(RT.java:27)
W/System.err(  937):    at org.example.ExampleActivity.$jacocoInit(ExampleActivity.java)

As soon as this will be solved - we're done and can claim that JaCoCo can work for Android ;)

Another problem : seems that we can't rely on ShutdownHook.

And the last point is that original classes must be presented to generate report, which means that actually we can't instrument classes in-place.

Despite the 3 problems described above (actually using some hacks like hard-coded value for dump file) I was able to collect data and generate report.

And to be precise - here is info about how Emma integrated: android.test.InstrumentationTestRunner executes method com.vladium.emma.rt.RT.dumpCoverageData with path to the dump file. As a workaround I can imagine that we will provide this class, which will do a proper shutdown of agent. But WDYT?

Concerning configuration: EMMA has a alternative was for configuration, which is a optional file emma.properties on the class path. Do we need that?

Concerning ShutdownHook: A dumpCoverageData() would mean that we actually start exposing APIs be the agent which I wanted to avoid. Are there other alternatives like tcpclient output?

Concerning InstrumentationTestRunner: Or we simply document your prototype (blog?) with an example implementation and let the Android community pick-up an this? I don't think we can also maintain the Android integration.

@marchof

• I don't understand connection with emma.properties
• I agree that dumpCoverageData will expose a kind of API, but on other side this doesn't seem to be worse than MBean controller. I'm still continuing search of other ways to achieve this. However not sure that we can use tcpclient.
• To be clear and precise - InstrumentationTestRunner is not my prototype, but class provided by Android SDK. Indeed - we don't want to maintain a complex integration with Android. And that's why it should be as simple as possible and non-intrusive. And I believe that would be much better for evolution of project in general, if JaCoCo will work for Android out-of-the-box without hacks like hard-coded path to dump file.

Even if we can use solution like tcpclient, most important is how to configure agent, because I still didn't found a way to pass System properties and file mode is a default.

I managed to do following:

• static initializer within test sets system properties for JaCoCo (jacoco.output=tcpserver, jacoco.address=127.0.0.1), but from my point of view there is no guarantee that this code executed before initialization of agent
• method tearDown within test connects to agent and creates dump file, this adds dependency on jacoco-core and Android SDK complained about duplicate files, so I removed them from JAR (like about.html)

I wouldn't say that this is non-intrusive solution - requires free port, requires custom code with dependency on jacoco-core, stability not guaranteed (due to the way of setting properties), ...

@Godin

• What I wanted to propose is a similar configuration option as in EMMA: If the runtime finds a file jacoco.properties on the classpath, the configuration options from this file are used. Would that work? I can easily implement this.
• We could provide a new bundle org.jacoco.agent.api or something like that which provides an API to the agent, but still this requires custom code which depends on this.
• An alternative would be advances instrumentation options: You can define certain methods (e.g. from the JUnit framework) which are instrumented in a way that they trigger a dump. This does not require custom code, but detailed knowledge of the test framework in use.

@marchof

• At first look jacoco.properties seems good ( or even jacoco-agent.properties ), but do you think that this is a clean and good solution or we can imagine something better?
• If we will provide com.vladium.emma.rt.RT.dumpCoverageData no custom code will be required - existing code in Android SDK will do the job. This can a be a kind of compatibility layer in short term, while Android community not fully switched from Emma to JaCoCo. In long term I expect that they will use official API provided by JaCoCo - as you said org.jacoco.agent.api
• Advanced instrumentation doesn't seem good for me, since indeed will introduce back-dependency and will require knowledge of the test framework.

To sum up : agent API looks really sexy for me. And it can be minimalistic like existing MBean.

@Godin I'll add the ability load configuration from a jacoco-agent.properties file. Concerning the agent API I would like to work on this in a separate pull request.

@marchof Ok, sounds good, so we can merge this branch since offline instrumentation works fine, and adjust other parts separately..

@Godin I added an additional way to provide configuration options without system properties. So I propose to merge this one and create a separate pull request for the agent API.

@Godin Created issues #61, #62 and #63 to follow this up.

@marchof Cool! Do you want me to merge this one?

@marchof Created issue #64 to support offline instrumentation by Maven Plugin.

I am trying to pull these changes into my fork and have hit some test failures when trying to run InstrumentTaskTest from Eclipse:

testInstrumentAndRunWithSystemProperties [src\org\jacoco\ant\InstrumentTaskTest.xml]
org.jacoco.ant.test\src\org\jacoco\ant\InstrumentTaskTest.xml:77: Java returned: 1
[java] Exception in thread "main" java.lang.NoClassDefFoundError: $jacoco/runtime/package/name$/RT

testInstrumentAndRunWithConfigFile [src\org\jacoco\ant\InstrumentTaskTest.xml]
org.jacoco.ant.test\src\org\jacoco\ant\InstrumentTaskTest.xml:61: Java returned: 1
[java] Exception in thread "main" java.lang.NoClassDefFoundError: $jacoco/runtime/package/name$/RT

testInstrumentInvalidClassFile [src\org\jacoco\ant\InstrumentTaskTest.xml]
org.jacoco.ant.test\src\org\jacoco\ant\InstrumentTaskTest.xml:38: Problem: failed to create task or type truncate
Cause: The name is undefined.

I am using Eclipse 3.8.0. When I run the tests from Maven I hit different errors:

org.jacoco.ant.test\src\org\jacoco\ant\InstrumentTaskTest.xml:44: Didn't expect file 'C:\Users\mchr\AppData\Local\Temp\jacocoTest299116871/output/broken.class' to exist
org.jacoco.ant.test\src\org\jacoco\ant\InstrumentTaskTest.xml:67: Expected file 'C:\Users\mchr\AppData\Local\Temp\jacocoTest632977538/test.exec' to exist

Have you seen any of these errors before?

testInstrumentAndRunWithSystemProperties [src\org\jacoco\ant\InstrumentTaskTest.xml]
testInstrumentAndRunWithConfigFile [src\org\jacoco\ant\InstrumentTaskTest.xml]

This is a known issue. The problem is that the Ant tests are more or less integration tests which need properly processed resources.

testInstrumentInvalidClassFile [src\org\jacoco\ant\InstrumentTaskTest.xml]

Please check your dependencies, we need Ant 1.7.1 to run this tests (see master POM).

org.jacoco.ant.test\src\org\jacoco\ant\InstrumentTaskTest.xml Maven errors

Looks like a merge problem. Both tests have been adjusted this way for #66.

Thanks for the quick reply.

It's a pity that InstrumentTaskTest can't be run from Eclipse. It makes debugging failures more awkward.

My Eclipse includes Ant 1.8.3. Ant 1.7.1 is on my project classpath but Eclipse still seems to be using the built in Ant 1.8.3. Have you had any Ant version issues within Eclipse?

I am investigating the Maven failures. In testInstrumentInvalidClassFile it looks like the instrumentation correctly fails to process the truncated class file but it seems to be failing to delete the zero length output file.

testInstrumentInvalidClassFile fails on Windows because the task attempts to delete the invalid zero length output class file before the outputstream has been closed. I have fixed this: mchr3k@e716107

testInstrumentAndRunWithConfigFile is still failing for me. I'm investigating.

testInstrumentAndRunWithConfigFile fails on Windows due to the following line:

<echo file="${temp.dir}/jacoco-agent.properties">destfile=${temp.dir}/test.exec</echo>

This writes a path which includes \ path separators. These are not valid when loaded by the Properties class.

I'm not sure what the best way is to fix this as Ant doesn't seem to have built in support for replacing \ with / in a property.

@mchr3k Thanks for analyzing this! The property file should be created with the (optional) PropertyFile task instead. I need to check how to make the optional task work under Maven.

@mchr3k Should be fixed with a0e67db

Much better thanks!