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

destfile agent parameter should be absolute also for default value #301

Merged
merged 1 commit into from Apr 29, 2015

Conversation

Projects
None yet
2 participants
@marchof
Member

marchof commented Apr 29, 2015

Hello,

At apache lucene we use carrot labs junit4 (http://labs.carrotsearch.com/randomizedtesting-junit4.html) which spawns multiple JVMs to make use of all of our cpus.

The jacoco:agent task made this very easy to integrate, but there was one trick, if you try to change the destination file, the setter always converts it to an absolute path (https://github.com/jacoco/jacoco/blob/master/org.jacoco.ant/src/org/jacoco/ant/AbstractCoverageTask.java#L74). Is this intentional?

It seems a little confusing since the default (jacoco.exec) is actually a relative path, but its impossible to set any relative path other than the default since the setter does this conversion.

For us its important that each jvm get a separate file (the easiest way to do that is a relative path), when we create our report we just provide all of them and everything works. The issue is easy to workaround, we just append the filename directly to the result of jacoco:agent ourselves, but it doesn't seem quite right.

In ant it is easy for someone to convert a relative path to an absolute one in their build script themselves, if they are doing something advanced and really need it.

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Apr 10, 2015

Member

This is actually intentional: The working directory of the Ant script is typically different from the JVM running the tests. If we would use a relative path the exec file would not be written at the location which is specified in the Ant file - which what you actually want but would probably break most user's builds.

How do you create your JVMs, also from Ant? In this case you could run jacoco:agent separately for every JVM.

Member

marchof commented Apr 10, 2015

This is actually intentional: The working directory of the Ant script is typically different from the JVM running the tests. If we would use a relative path the exec file would not be written at the location which is specified in the Ant file - which what you actually want but would probably break most user's builds.

How do you create your JVMs, also from Ant? In this case you could run jacoco:agent separately for every JVM.

@rmuir

This comment has been minimized.

Show comment
Hide comment
@rmuir

rmuir Apr 10, 2015

We don't create the JVMs from ant. the junit4 library forks them (we pass numcpus=auto by default as well).

There seems to be a bug either way. if the absolute path is intentional, then the default 'jacoco.exec' should be computed as an absolute path, too. But its not, as the setter isn't invoked in that case.

rmuir commented Apr 10, 2015

We don't create the JVMs from ant. the junit4 library forks them (we pass numcpus=auto by default as well).

There seems to be a bug either way. if the absolute path is intentional, then the default 'jacoco.exec' should be computed as an absolute path, too. But its not, as the setter isn't invoked in that case.

@marchof

This comment has been minimized.

Show comment
Hide comment
@marchof

marchof Apr 10, 2015

Member

You're right, this is a bug. Will fix this.

Member

marchof commented Apr 10, 2015

You're right, this is a bug. Will fix this.

@marchof marchof changed the title from jacoco ant tasks and getAbsolutePath to destfile agent parameter should be absolute also for default value Apr 10, 2015

GitHub #301: Absolute path for destfile option in Ant tasks
For the Ant tasks coverage and agent the destfile attribute is
now passed as an absolute path also in the default case.

marchof added a commit that referenced this pull request Apr 29, 2015

Merge pull request #301 from jacoco/issue-301
destfile agent parameter should be absolute also for default value

@marchof marchof merged commit bffe24a into master Apr 29, 2015

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@marchof marchof deleted the issue-301 branch Apr 29, 2015

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