Navigation Menu

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

Add Maven Mojo to perform offline instrumentation #64

Merged
merged 11 commits into from Jan 15, 2013
Merged

Add Maven Mojo to perform offline instrumentation #64

merged 11 commits into from Jan 15, 2013

Conversation

Godin
Copy link
Member

@Godin Godin commented Jan 4, 2013

No description provided.

@stephanenicolas
Copy link

Where can we learn how to use Jacoco with offline instrumentation ? I would like to try it on an android project ?

@Godin
Copy link
Member Author

Godin commented Jan 2, 2013

Version 0.6.2 which supports offline instrumentation is still in development, so things can dramatically change and that's why there is no public and official information on how to do this. But if you want to do some experiments with trunk, then discussion in #58 might be interesting for you.

@stephanenicolas
Copy link

I don't feel confortable enough to start from scratch. But if you need someone to test the documentation when it is ready, please notify me.

Thx for your work,
Stéphane

@Godin
Copy link
Member Author

Godin commented Jan 2, 2013

Ok, cool! I'll keep this in mind.

@Godin
Copy link
Member Author

Godin commented Jan 11, 2013

@marchof Could you please review those changes?

@marchof
Copy link
Member

marchof commented Jan 12, 2013

@Godin I will have a closer look at it tomorrow!

<version>4.10</version>
<scope>test</scope>
</dependency>
<dependency>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a important step to provide the agent runtime here. Should we add a comment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 309ab4c

@marchof
Copy link
Member

marchof commented Jan 13, 2013

@Godin I feel like we should add a warning like this to the offline goals (also for the Ant task). The problem is that people migrating from other tools will expect an instrumentation step and might actually use this without any reason.

Warning: The preferred way for code coverage analysis with JaCoCo is on-the-fly instrumentation. Offline instrumentation has several drawbacks and should only be used if a specific scenario explicitly requires this mode. Please consult documentation about offline instrumentation before using this mode.

@marchof
Copy link
Member

marchof commented Jan 13, 2013

@Godin Minor comment about wording: I used "offline instrumentation" in documentation in lower case, you use upper case spelling. As "on-the-fly" is also lowercase I would prefer the lower case spelling.

@marchof
Copy link
Member

marchof commented Jan 13, 2013

@Godin Is it possible and reasonable to have a single goal 'instrument' which does also the cleanup job? I.e. have this goal bound to two phases by default?

try {
final FileFilter fileFilter = new FileFilter(this.getIncludes(),
this.getExcludes());
fileNames = FileUtils.getFileNames(classesDir,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Godin Can we somehow avoid or suppress the Eclipse compiler warning here? I like warning free workspaces ;-)

Type safety: The expression of type List needs unchecked conversion to conform to List

@Godin
Copy link
Member Author

Godin commented Jan 14, 2013

@marchof Not possible to bind the same goal to two phases by default, moreover this separation is required for support of Android.

Warning about offline instrumentation looks like a good idea for me - should I add it?

The rest has been fixed.

@marchof
Copy link
Member

marchof commented Jan 14, 2013

@Godin Ok, please add the warning to the Maven goals, I'll add it to the Ant task on master.

@Godin
Copy link
Member Author

Godin commented Jan 14, 2013

@marchof Warning has been added, so can I merge?

@marchof
Copy link
Member

marchof commented Jan 14, 2013

@Godin Go for it ;-)

* Performs offline instrumentation. Note that after execution of test you must
* restore original classes with help of "restore-instrumented-classes" goal.
* <p>
* <b>Warning:</b> The preferred way for code coverage analysis with JaCoCo is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Godin Either we use the "strong" tag here or we need to extend the stylesheet. Currently the "b" markup gets lost.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marchof Indeed - I'll change on "strong".

Godin added a commit that referenced this pull request Jan 15, 2013
Add Maven Mojos to perform offline instrumentation
@Godin Godin merged commit 3c1cb3d into master Jan 15, 2013
@Godin Godin deleted the issue-64 branch January 15, 2013 10:00
@stephanenicolas
Copy link

HI @Godin ,

this thread is a bit old now but your experiment with android helped me a lot to set up a unified code coverage for both robolectric and standard junit tests and get it displayed in a sonar dash board.

(see the Jacoco profile here : https://github.com/stephanenicolas/Quality-Tools-for-Android/blob/)

  • Robolectric are considered standard unit tests.
  • standard Android Junit tests are considered as standard integration tests.
    This makes sense as Robolectric tests mock an android platform and can be considered more "unit" tests thant standard android tests because the latter needs a real android platform and relies on networking, disk, locale, etc.

I have been dreaming about that for almost a year now. Thanks a lot for offline instrumentation via jacoco :)

Do you think it would be possible to give better names to the test suites inside the widget, and even to add more test suites ? For instance I would like to add UI testing (black box testing) or monkey testing.

@Godin
Copy link
Member Author

Godin commented Mar 24, 2013

Hi @stephanenicolas ,

No need to write the same in several topics.

We are glad to hear that our work is useful for you.

What do you mean by "widget" ? If this is about Sonar, then this is not an appropriate place to discuss this.

@stephanenicolas
Copy link

Hi Evgeny,

you're right, I have been over enthusiastic about this :)
Sorry for having disturbed this forum. I just believed that people reaching
both threads might have been interested in this work.

By widget, I meant the Sonar widget, so you're right again, that's not the
place to talk about it.

Stéphane

2013/3/24 Evgeny Mandrikov notifications@github.com

Hi @stephanenicolas https://github.com/stephanenicolas ,

No need to write the same in several topics.

We are glad to hear that our work is useful for you.

What do you mean by "widget" ? If this is about Sonar, then this is not an
appropriate place to discuss this.


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

Stéphane NICOLAS,
OCTO Technology
Développeur & Consultant Android / Java
..........................................................
50, Avenue des Champs-Elysées
75008 Paris
+33 (0)6.26.32.34.09
www.octo.com - blog.octo.com
www.usievents.com
...........................................................

@stephanenicolas
Copy link

Is offline instrumentation supposed to work for android with latest release
artifact ?

It looks like :
https://github.com/Godin/jacoco-experiments/blob/android/android/app.tests/pom.xml

is not working anymore with latest jacoco release. They only worked with
the snapshot 0.6.2-SNAPSHOT, but not afterwards.

Stéphane

@Godin
Copy link
Member Author

Godin commented Oct 29, 2013

@stephanenicolas It supposed to work, because there is no major changes on Maven Plugin side regarding this. In any case I suggest you to switch to Users Mailing List with better explanation of your problem.

@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants