Bug 145107

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.

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.

No, I did not rewrite it (emma.xml) - not enough time :-(.

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

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.

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.

BTW, the latest version of my script is here:

https://openbluesky.dev.java.net/svn/openbluesky/trunk/src/tools/ant/cobertura-rcp.xml

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.  

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.

Jesse,

you can use 'guest' and an empty password - it's a direct link to the Subversion repository.

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.

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?

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.

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

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) :-)

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.

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.

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.

Created attachment 73260 [details]
Proposed patch made against trunk using hg export -g

[JG01] You don't need all the <copy> tasks in build.xml. Just use release.*=* properties in project.properties.

Thanks for the patch, I'll look at it next week.

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.

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.

Created attachment 74132 [details]
Patch for newer harness build.xml

Created attachment 74133 [details]
Patch for NB.org modules

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

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

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.

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.

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.

Created attachment 74229 [details]
Chec for non-default platform

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.

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.

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.

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.

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.

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

I have verified this in nightly build 200905120201.  Thanks so much for getting it into 6.7!

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.