Bug 145107 - Coverage for NB modules
Coverage for NB modules
Status: VERIFIED FIXED
Product: apisupport
Classification: Unclassified
Component: Harness
6.x
All All
: P2 (vote)
: 6.x
Assigned To: rmichalsky
issues@apisupport
:
Depends on: 154821
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-26 10:16 UTC by Jan Lahoda
Modified: 2009-07-08 05:34 UTC (History)
6 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
Proposed patch made against trunk using hg export -g (21.62 KB, text/plain)
2008-11-05 02:08 UTC, tomwheeler
Details
Patch for newer harness build.xml (1.22 KB, patch)
2008-11-25 13:36 UTC, rmichalsky
Details | Diff
Patch for NB.org modules (1.33 KB, patch)
2008-11-25 13:36 UTC, rmichalsky
Details | Diff
Chec for non-default platform (1.03 KB, patch)
2008-11-27 11:22 UTC, rmichalsky
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Lahoda 2008-08-26 10:16:35 UTC
[recent sources]

XTest was able to measure coverage of NB module tests. I was not able to find a way to run coverage for my tests (short
of rewriting nbbuild/templates/emma.xml myself).
Comment 1 Jaroslav Tulach 2008-08-28 08:10:23 UTC
Jan, did you rewrite emma.xml? Can you attach your changes? Also, I know Tom was interested in the test coverage and 
promised to look at ways for generating it.
Comment 2 tomwheeler 2008-08-28 15:18:46 UTC
Yes, I am very interested.  In fact, I have started to document how to use the new testing scheme here:

   http://wiki.netbeans.org/DevFaqUsingSimpletests

and hope to finish it up this weekend.  How to do code coverage for a platform application is specifically one of the 
TODO items listed at the bottom, so I would love to document it.
Comment 3 Jan Lahoda 2008-08-28 15:32:13 UTC
No, I did not rewrite it (emma.xml) - not enough time :-(.
Comment 4 tomwheeler 2008-08-28 18:55:30 UTC
Is there any requirement to use Emma?  

If I were to implement this myself, I would likely use Cobertura instead as all development on Emma seems to have 
ceased more than two years ago. The use of Cobertura for measuring coverage of a J2SE project is described here:

   http://wiki.netbeans.org/CoberturaAnt

and so if you're open to using Cobertura instead, I might be able to implement this on my own and contribute it back 
(although probably not this weekend).
Comment 5 tomwheeler 2008-10-13 07:05:11 UTC
I was doing well in my work on implementing coverage using Cobertura today and I came across a minor problem with the
generated report.  I did a Web search and this led me to an interesting page:

   http://weblogs.java.net/blog/fabriziogiudici/archive/2008/01/cobertura_and_h.html

It seems fellow Dream Team member Fabrizio Giudici has already implemented Cobertura code coverage for the platform.  I
have studied his implementation and think his approach is better and more complete than what I had and so I have just
e-mailed him to ask if we can use it instead.  

I did find that the FIXME comment he has in cobertura-init is no longer relevant.  Another minor problem was that
changing the value of the 'cobertura.datafile' property resulted in two files (one named with the new value and one
still named as 'cobertura.ser') being created; the data of each file is somewhat incomplete and the reports generated
incorrectly showed 100% coverage.  This seems to be a problem with Cobertura itself (or its Ant tasks) rather than how
it is being used because I had the same problem with my implementation.  I suspect a warning in the harness README
and/or a comment in the XML above where that value is defined will suffice in documenting this peril.

Since Fabrizio's page shows all the code and describes it in a fair amount of detail, would it be easier for me to send
patches for you to incorporate (I don't have commit access) or would it be easier for you to just make the necessary
changes based on his page?  I am happy to do it either way.
Comment 6 fabriziogiudici 2008-10-13 09:35:16 UTC
Sure you can! BTW, yes, I've seen that "double file" error too and I believe it is a Cobertura bug. I'm not using the
very last version, but I suppose you are and it's still here. And yes, my FIXME has gone since using NB 6.5.


Comment 7 fabriziogiudici 2008-10-13 09:49:00 UTC
BTW, the latest version of my script is here:

https://openbluesky.dev.java.net/svn/openbluesky/trunk/src/tools/ant/cobertura-rcp.xml
Comment 8 tomwheeler 2008-10-15 01:16:42 UTC
I figured out what was causing the "double file" error and have a fix: the cobertura-report Ant task had the datafile
attribute set, but it was missing from the cobertura-instrument task.  
Comment 9 Jesse Glick 2008-10-15 18:35:08 UTC
Tom - a contribution should be in the form of a complete source code patch against a recent revision of the main repo.
diff --git format is preferred. You probably want to read

http://wiki.netbeans.org/HgHowTos#section-HgHowTos-DevelopAPIReviewPatchesUsingMQ

BTW Fabrizio - your link cannot be accessed by the public.
Comment 10 fabriziogiudici 2008-10-15 21:54:26 UTC
Jesse,

you can use 'guest' and an empty password - it's a direct link to the Subversion repository.
Comment 11 tomwheeler 2008-10-16 14:37:43 UTC
I created a platform last night based on the latest code in the main repo and added all the necessary code to support
coverage at both the module and suite level (based on Fabrizio's code).  It is working fine and I have posted it here
(along with a suite which has some classes and various unit test to go with them for measuring coverage):

   http://www.tomwheeler.com/netbeans/coverage20081015.zip

I still need to turn this into a patch, though.  I plan to work on that part tonight once I figure out what specific
things I need to do in order to handle JAR files (binaries) in Mercurial.
Comment 12 rmichalsky 2008-10-16 15:20:05 UTC
Tom, Fabrizio, thanks a lot for this contribution! Looks good to me, I'd test it a bit and integrate next week. Not sure
about formal stuff, Contributor Agreement, etc., anyone happens to know the details?
Comment 13 tomwheeler 2008-10-16 15:33:56 UTC
I have signed the SCA long ago, but I don't know about Fabrizio.  If this page is up-to-date, it looks like he is not 
on the list:

   http://www.netbeans.org/about/legal/approved-contributors.html

If I were using CVS, I'd have the patch done by now :-)  I've made some simple contributions since the move to Hg, but 
this is my first significant patch since then and it's more complicated because there are JAR files involved. 

Richard, would you mind if I asked you a few questions offline about handling the binaries?  What I read about in the 
FAQ did not work, and I am not sure whether I am doing it wrong or my account is just not set up correctly.
Comment 14 Lukas Hasik 2008-10-16 15:34:29 UTC
afaik, Fabricio and Tom have signed the SCA already - https://sca.dev.java.net/CA_signatories.htm
Fabrizio Guidici - NB - fabrizioguidici, OpenJDK java.net - fabrizioguidici
Thomas W. Wheeler - NB - tomwheeler
Comment 15 tomwheeler 2008-10-16 15:38:40 UTC
Ah, indeed Fabrizio is there.  I was doing a Ctrl+F to find him on the page, using the correct spelling of his name 
(Giudici) :-)
Comment 16 fabriziogiudici 2008-10-16 21:32:52 UTC
Yes, I'm there. I see that 90% of the time those two vowels get swapped, I presume they are pretty unusual outside
italian :-) Please let me know when the patch is committed (but I presume I will be notified by this thread) as I'm
excited to become an official committer, even though it's really a tiny thing.
Comment 17 tomwheeler 2008-10-17 06:32:18 UTC
I have not been able to make a patch yet (uploading binaries fails to authenticate, perhaps because my Hg password is
not the same as what my CVS password was).  However, I did make some more improvements tonight; specifically, I updated
it to no longer assume modules are located below the suite and also to have the suite targets not make assumptions about
the name/path of the .ser file:

   http://www.tomwheeler.com/netbeans/coverage20081015.zip

I will turn all this into a patch when I get Hg straightened out.
Comment 18 tomwheeler 2008-11-05 02:07:10 UTC
Sorry for the delay; I had some end-of-month deadlines after I got my Hg account info.

I will attach the patch (made against a recent trunk checkout) shortly. I believe all typical cases are handled and it
has some improvements over Fabrizio's version as previously mentioned.  

Please try it out and make any suggestions. I will also update the harness/README document to account for these changes
once I have incorporated all feedback.  I should have at least an hour to devote to this each of the following two nights.
Comment 19 tomwheeler 2008-11-05 02:08:52 UTC
Created attachment 73260 [details]
Proposed patch made against trunk using hg export -g
Comment 20 Jesse Glick 2008-11-07 03:44:35 UTC
[JG01] You don't need all the <copy> tasks in build.xml. Just use release.*=* properties in project.properties.
Comment 21 rmichalsky 2008-11-07 09:17:40 UTC
Thanks for the patch, I'll look at it next week.
Comment 22 tomwheeler 2008-11-25 03:09:49 UTC
I haven't seen any feedback on my patch from Richard yet.  Can I assume that everything is OK?  

I will probably have a couple of hours to work on it this weekend and could incorporate Jesse's suggestion and update
the harness README if everything else is in good order.
Comment 23 rmichalsky 2008-11-25 13:35:32 UTC
Sorry for the delay. Patch seems good to me and works for suites, I'm attaching additional patch to make it work for
NB.org modules. Also current patch cannot be applied to apisupport.harness/build.xml as it has changed considerably,
attaching newer one as well.

So should I integrate current version or should I wait for your changes per Jesse's suggestions? Documentation to
harness README can be IMHO added separately, we need not to wait for it with integration.

Formally we'll have to wait for legal review that used libraries are available under sufficiently free license, but that
should pose no problem.
Comment 24 rmichalsky 2008-11-25 13:36:22 UTC
Created attachment 74132 [details]
Patch for newer harness build.xml
Comment 25 rmichalsky 2008-11-25 13:36:49 UTC
Created attachment 74133 [details]
Patch for NB.org modules
Comment 26 tomwheeler 2008-11-25 15:03:14 UTC
> So should I integrate current version or should I wait for your changes per 
> Jesse's suggestions? Documentation to harness README can be IMHO added 
> separately, we need not to wait for it with integration.

Not sure if this was directed at me, but since there are multiple patches, I'd find it easier to integrate it as is and
then I can address Jesse's suggestion and update the README to describe coverage support this weekend in a separate patch.
Comment 27 rmichalsky 2008-11-26 10:22:19 UTC
> Not sure if this was directed at me...

Yes, sorry for unclear wording. I found one issue - when trying to run Cobertura targets with JDK 1.5, build ends with
exception:
C:\NetBeansProjects\core-main\nbbuild\netbeans\harness\cobertura.xml:57: Expected a Task from 'java' but got an instance
of org.apache.tools.ant.taskdefs.PreSetDef$PreSetDefinition instead

With JDK 1.6 everything works and Cobertura itself claims to be able to run on 1.3.1 and later. Tom, do you have time to
look at it? Otherwise I'll investigate. Once this is resolved and the legal process is finished, I'll integrate.
Comment 28 tomwheeler 2008-11-26 16:32:06 UTC
Sure, I will take a look and see if I can find the problem, although it will probably be Friday before I get a chance 
due to the American Thanksgiving holiday tomorrow.
Comment 29 Jesse Glick 2008-11-26 18:52:20 UTC
The cause of "Expected a Task from 'java'..." is known. It is a bug in Cobertura, triggered by NB's use of <presetdef>
in jdk.xml. I am looking into how this could be fixed (other than patching Cobertura) - a bigger change involving the
Ant module as well as JDK selection support in the harness.
Comment 30 rmichalsky 2008-11-27 11:19:35 UTC
The problem appears only when non-default java platform is set for the project, I've added check with warning until it
is fully resolved. Still usable in most cases.
Comment 31 rmichalsky 2008-11-27 11:22:00 UTC
Created attachment 74229 [details]
Chec for non-default platform
Comment 32 tomwheeler 2009-03-04 18:53:01 UTC
Richard, is there a chance this could get into 6.7 M3?  Having coverage support would be *very* helpful and I feel like
it's 99% there already.  

I apologize for dropping the ball on looking at the Cobertura/Ant bug, but I had understood from Jesse's later update
that he knew the problem and was looking at solutions himself.

If you need something from me to make it happen, please let me know and I will do my best to do it.
Comment 33 Jesse Glick 2009-03-04 19:03:00 UTC
Blocked (in the case of a nondefault JDK only) by issue #154821, which I have not had time to deal with yet. Not sure if
I can get to it this week or not.
Comment 34 rmichalsky 2009-03-05 09:38:55 UTC
I am definitely counting with this getting into 6.7, I have most of it prepared, just needed to wait for legal reviews
(and then I was busy with suite chaining support). I should have updated the issue now and then, sorry for that.

Re issue #154821: IIRC there was a workaround that analyzed project must use the same JDK version as Ant, which is not a
default setup for NB.org modules, but should pose no problem for RCP developers, so I guess it is worth putting into 6.7
even without #154821 resolved.
Comment 35 Jesse Glick 2009-03-05 18:07:32 UTC
I agree that most RCP devs are going to using the default JDK anyway, so this is probably not a blocker so long as there
is a nice error message with apology.
Comment 36 rmichalsky 2009-05-04 16:04:00 UTC
applied into core-main #47e212858037

Changed couple things because of legal issues, scripts & folder renamed to 'testcoverage' and harness can do without
test coverage scripts - ideally they should be in separate module, but it is too late, maybe in next release if needed.

There is no dedicated UI (yet), can be run via 'Run target' menu on build scripts.

Currently cannot be directly used for NB.org modules because of issue #154821. A workaround is to run test coverage from
command line with -Dnbjdk.home.defaulted=true and perhaps -Dpermit.jdk6.builds=true if needed.
Comment 37 Quality Engineering 2009-05-07 07:59:39 UTC
Integrated into 'main-golden', will be available in build *200905070201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/47e212858037
User: Richard Michalsky <rmichalsky@netbeans.org>
Log: #145107: test coverage for apisupport projects
Comment 38 tomwheeler 2009-05-13 19:10:51 UTC
I have verified this in nightly build 200905120201.  Thanks so much for getting it into 6.7!
Comment 39 Quality Engineering 2009-07-08 05:34:59 UTC
Integrated into 'main-golden', will be available in build *200907080200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/4832c162fe6b
User: Jesse Glick <jglick@netbeans.org>
Log: #139700: XTest cleanup. Emma-based code coverage would need to be rewritten. Anyway #145107 offers Cobertura in the harness.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2014, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo