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

Use system properties in Jacoco agent configuration #262

Closed
wants to merge 9 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@debugger-zz

debugger-zz commented Nov 21, 2014

New Feature enables Jacoco agent to output "exec" results to a platform dependent directory. Examples java.io.tempdir or user.home.

Example setting the agent's destfile in jacoco-agent.properties:

replaceproperties=true
destfile=${user.home}/coverage-data/coverage.exec

The replacement feature is disabled by default. It can be enabled by
adding replaceproperties=true in agent's
jacoco-agent.properties. Alternatively the file's
configuration can be overridden by the system property
-Djacoco-agent.replaceproperties=true on java command
line.

The initial escaping mechanism was removed due to conflicts with Windows paths and anonymous classes (for more details see documentation). Documentation has been updated and explains the effects of enabling replaceproperties.

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Nov 21, 2014

Member

Why should JaCoCo implement its own variable substitution? Ant, Maven, Shell, CMD Scripts all support this already.

Member

marchof commented Nov 21, 2014

Why should JaCoCo implement its own variable substitution? Ant, Maven, Shell, CMD Scripts all support this already.

@debugger-zz

This comment has been minimized.

Show comment
Hide comment
@debugger-zz

debugger-zz Nov 21, 2014

For an offline instrumented build you cannot specify a dest file that is in a writable platform independent location. Adding the variable replacement allows to perform instrumented integration tests on different platforms and always outputting to the home directory.

debugger-zz commented Nov 21, 2014

For an offline instrumented build you cannot specify a dest file that is in a writable platform independent location. Adding the variable replacement allows to perform instrumented integration tests on different platforms and always outputting to the home directory.

@buildhive

This comment has been minimized.

Show comment
Hide comment
@buildhive

buildhive commented Nov 21, 2014

Java Code Coverage Tools » jacoco #321 SUCCESS
This pull request looks good
(what's this?)

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Nov 21, 2014

Member

Valid point. What configuration mechanism do you have in mind?

http://www.eclemma.org/jacoco/trunk/doc/offline.html

Member

marchof commented Nov 21, 2014

Valid point. What configuration mechanism do you have in mind?

http://www.eclemma.org/jacoco/trunk/doc/offline.html

@debugger-zz

This comment has been minimized.

Show comment
Hide comment
@debugger-zz

debugger-zz Nov 21, 2014

I want to deploy a jacoco-agent.properties file as described in my initial comment.
The integration builds are run outside of maven and possibly on different platforms so I need a way to guarantee the output goes to the systems temp directory or the users home directory.
For Java applet's running in the browser the default working directory ("user.dir") is "C:\Program Files\Firefox" so I need a way to change it to something user writable.

debugger-zz commented Nov 21, 2014

I want to deploy a jacoco-agent.properties file as described in my initial comment.
The integration builds are run outside of maven and possibly on different platforms so I need a way to guarantee the output goes to the systems temp directory or the users home directory.
For Java applet's running in the browser the default working directory ("user.dir") is "C:\Program Files\Firefox" so I need a way to change it to something user writable.

@debugger-zz

This comment has been minimized.

Show comment
Hide comment
@debugger-zz

debugger-zz Nov 21, 2014

I did not update the documentation in offline instrumentation as it should also work for online instrumentation. Except if I did overlook something.

debugger-zz commented Nov 21, 2014

I did not update the documentation in offline instrumentation as it should also work for online instrumentation. Except if I did overlook something.

@buildhive

This comment has been minimized.

Show comment
Hide comment
@buildhive

buildhive commented Nov 21, 2014

Java Code Coverage Tools » jacoco #322 SUCCESS
This pull request looks good
(what's this?)

@debugger-zz debugger-zz changed the title from Agent cfg with environment variables to Use system properties in Jacoco agent configuration Nov 21, 2014

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Nov 21, 2014

Member

You implementation only works for offline parameters (properties files and system properties) which I think is ok. So documentation only applies to offline.html. As this is limited to offline instrumentation I don't think you need replaceproperties. Replacement could be allways active.

Member

marchof commented Nov 21, 2014

You implementation only works for offline parameters (properties files and system properties) which I think is ok. So documentation only applies to offline.html. As this is limited to offline instrumentation I don't think you need replaceproperties. Replacement could be allways active.

@debugger-zz

This comment has been minimized.

Show comment
Hide comment
@debugger-zz

debugger-zz Nov 21, 2014

Oh, I did not realize this only works in Offline-instrumentation. I wasn't aware of the class AgentOptions in org.jacoco.core. I fear overtime a use case for Online-instrumentation with such a mechanism might come up. I don't mind removing "replaceproperties". Anything else you want me to change?

debugger-zz commented Nov 21, 2014

Oh, I did not realize this only works in Offline-instrumentation. I wasn't aware of the class AgentOptions in org.jacoco.core. I fear overtime a use case for Online-instrumentation with such a mechanism might come up. I don't mind removing "replaceproperties". Anything else you want me to change?

marchof added a commit that referenced this pull request Dec 26, 2014

GitHub #262: Variable replacement for offline agent configuration.
For offline agent configuration properties can now contain variables in
${name} format which will be replaced with system properties at runtime.
Based on original PR by user 'debugger'.
@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Dec 26, 2014

Member

Finally I found some time to bring this feature into master. I created a new single commit 0092055 with the following modifications:

  • simplified replacement implementation
  • documentation for offline configuration only
  • change log (including attribution)
Member

marchof commented Dec 26, 2014

Finally I found some time to bring this feature into master. I created a new single commit 0092055 with the following modifications:

  • simplified replacement implementation
  • documentation for offline configuration only
  • change log (including attribution)

@marchof marchof closed this Dec 26, 2014

@Godin Godin added this to the 0.7.3 milestone Dec 27, 2014

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