[icedtea-web] RFC: add unit tests for the jnlp parser

Wed Mar 30 17:08:13 PDT 2011

On 03/30/2011 06:08 PM, Dr Andrew John Hughes wrote:
> On 14:20 Wed 30 Mar     , Omair Majid wrote:
>> >  Ah, ok. I thought we wanted to only include code which is GPL-friendly
>> >  in IcedTea. If we are ok with adding independent code under other
>> >  license, it certainly makes this a non-issue.
>> >
> It has to be FOSS.  I don't see a reason for GPL compatibility in
> completely independent chunks of code; that's quite a strong
> requirement OpenJDK itself is a hodge-podge of stuff, allowed by the
> ASSEMBLY_EXCEPTION.
>

Ok, that's great.

>>> >  >  What JUnit code is included?  Is it marked?
>>> >  >
>> >
>> >  Please see the attached patch. I havent marked it, but almost all the
>> >  code in CommandLine.java is based on JUnit code. The
>> >  LessVerboseTextListener.java implements JUnit interfaces, but it is
>> >  written from scratch.
>> >
> There does need to be something in there such as:
>
> // Based on code from X (C) Y
>

Sure, except I dont know the copyright owners. So I have left it as:
// Based on code from JUnit

> You're also lacking license headers.

That was a quick example I had put together. I have fixed it in the 
actual patch

> snip...
>
>> >
>>>>> >  >>  >    I would like to see less things in-tree (/me is looking at NanoXML), not
>>>>> >  >>  >    more.
>>>>> >  >>  >
>>> >  >  How is the NanoXML issue progressing?  It would be good to have a status update
>>> >  >  on things and the progress towards the various releases of IcedTea-Web now 1.0
>>> >  >  is done.
>>> >  >
>> >
>> >  Sorry, I must have forgotten to make a note of this. Generally I try and
>> >  keephttp://icedtea.classpath.org/wiki/IcedTea-Web#Release_Plans
>> >  updated.
> I don't check the wiki regularly anyway. I'd expect stuff to be discussed on the
> mailing list then documented on the wiki.
>
>> >  Anyway, it's mostly done. I wrote these parser unit tests to
>> >  test that my NanoXML changes did not break anything. I will clean up
>> >  that patch and post it after these unit tests are committed (so we can
>> >  see any regressions). Assuming Deepak is fine with it, it can go in
>> >  before 1.1 is released.
>> >
> Yeah I think we (mainly Deepak as maintainer) need to define clearly where we draw
> a line under 1.1.
>
> snip..
>> >
>> >  I should have elaborated more. JUnit/TestNG (and possibly others) are
>> >  just fine for developers. The problem here is that we want to generate
>> >  buildbot-readable test results. We can do that by writing a custom
>> >  junit-output formatter (like in the attached patch), by transforming
>> >  testng's xml output or by transforming junit's xml output. Unfortunately
>> >  the only way of getting junit to create xml output is through ant. Junit
>> >  itself has no option (at least nothing that I could see) that generates
>> >  output in xml format. If it is okay to use the custom junit output
>> >  formatter, than we dont need ant. Actually, if we dont care about the
>> >  buildbot, then we dont need the custom output formatter or any xml
>> >  processing at all.
>> >
> Ok, so whoever wrote the JUnit XML outputter introduced some odd Ant dependency
> that's unnecessary.  Got you.
>

Actually, not quite. JUnit has no xml outputter. Only ant (thought it's 
junit task) does.

>>> >  >  Ant massively overcomplicates things, but I have no idea of the availability
>>> >  >  of the various dependencies for testng or their licenses.
>>> >  >
>>> >  >  Can you provide some of this info?
>>> >  >
>> >
>> >  Sure. TestNG is licensed under Apache 2.0. Its dependencies are QDox
>> >  (Apache 2.0) and Bean Shell (Sun Public License or LGPL).
>> >
>> >  TestNG (and its dependencies) are available in a number of Linux
>> >  distributions, including Debian (squeeze, wheezy and sid), Fedora (>=
>> >  13), Gentoo (java-experimental overlay), Mageia, Mandriva (, openSUSE
>> >  (>= 11.2), and Ubuntu (>= 9.10). It's not available in Arch Linux, and
>> >  Slackware (and possibly other distributions).
>> >
> It's in Gentoo's java-overlay tree (the same place I develop IcedTea ebuilds
> and have commit access to),

Ah, sorry. I dont have Gentoo. I did a quick search and saw that package 
in the java-experimental overlay.

> and this is the hideous dependency tree it has:

> [ebuild  N     ] dev-java/commons-logging-1.1.1  USE="-avalon-framework -avalon-logkit -doc -log4j -servletapi -source -test" 187 kB [0]
> [ebuild  N     ] dev-java/ant-apache-log4j-1.8.1  0 kB [0]
> [ebuild  N     ] dev-java/ant-swing-1.8.1  0 kB [0]
> [ebuild  N     ] dev-java/ant-apache-bcel-1.8.1  0 kB [0]
> [ebuild  N     ] dev-java/ant-apache-resolver-1.8.1  0 kB [0]
> [ebuild  N     ] dev-java/ant-apache-oro-1.8.1  0 kB [0]
> [ebuild  N     ] dev-java/jdepend-2.9-r4  USE="-doc -source" 296 kB [0]
> [ebuild  N     ] dev-java/jzlib-1.0.7-r1  USE="-doc -source" 50 kB [0]
> [ebuild  N     ] dev-java/commons-net-1.4.1-r1  USE="-doc -examples -source" 224 kB [0]
> [ebuild  N     ] dev-java/jakarta-regexp-1.4-r1  USE="-doc -source" 135 kB [0]
> [ebuild  N     ] dev-java/sun-jaf-1.1.1  USE="-doc -source" 123 kB [0]
> [ebuild  N     ] java-virtuals/jaf-1.1-r1  0 kB [0]
> [ebuild  N     ] dev-java/ant-commons-logging-1.8.1  0 kB [0]
> [ebuild  N     ] dev-java/ant-apache-regexp-1.8.1  0 kB [0]
> [ebuild  N     ] dev-java/ant-commons-net-1.8.1  0 kB [0]
> [ebuild  N     ] dev-java/ant-jdepend-1.8.1  0 kB [0]
> [ebuild  N     ] dev-java/jsch-0.1.44  USE="zlib -doc -examples -source" 278 kB [0]
> [ebuild  N     ] dev-java/bsf-2.4.0-r1  USE="javascript python -doc -examples -source -tcl" 293 kB [0]
> [ebuild  N     ] dev-java/sun-javamail-1.4.3  USE="-doc -source" 1,148 kB [0]
> [ebuild  N     ] dev-java/ant-apache-bsf-1.8.1  USE="javascript python -tcl" 0 kB [0]
> [ebuild  N     ] dev-java/ant-jsch-1.8.1  0 kB [0]
> [ebuild  N     ] java-virtuals/javamail-1.0-r1  0 kB [0]
> [ebuild  N     ] dev-java/ant-javamail-1.8.1  0 kB [0]
> [ebuild  N     ] dev-java/ant-1.8.1  USE="X antlr bcel bsf commonslogging commonsnet javamail jdepend jsch log4j oro regexp resolver -jai -jmf -testutil" 0 kB [0]
> [ebuild  N     ] dev-java/testng-5.2  USE="-doc -source" 2,605 kB [1]
>
> I think most of that is due to a bad dependency on the whole of Ant though.  Not sure how much
> of Ant it actually needs.
>

I think it's a build-time dependency, not a run-time dependency. But you 
are correct - the tree is quite big.

>> >  JUnit is licensed under Common Public License v.10.
>> >
>> >  JUnit is available in all the distributions listed above - including
>> >  Arch Linux and Slackware.
>> >
>>>>> >  >>  >    My custom runner:
>>>>> >  >>  >    Passed: net.sourceforge.jnlp.ParserBasic.testResourcesJar
>>>>> >  >>  >    Passed: net.sourceforge.jnlp.ParserBasic.testResourcesExtensions
>>>>> >  >>  >    Passed: net.sourceforge.jnlp.ParserBasic.testResourcesProperty
>>>>> >  >>  >    [snip]
>>>>> >  >>  >    FAILED: testUnquotedAttributes(net.sourceforge.jnlp.ParserMalformedXml)
>>>>> >  >>  >    Invalid XML document syntax.
>>>>> >  >>  >    Test results: passed: 23; failed: 4; ignored: 0
>>>>> >  >>  >
>>>>> >  >>  >    I suppose using plain JUnit is fine; it just wont have pretty output
>>>>> >  >>  >    suitable for buildbot.
>>>>> >  >>  >
>>> >  >  The custom runner looks fine.  What's the problem?
>>> >  >
>> >
>> >  Other than my licensing concerns (CPL in a GPL-friendly repository),
>> >  nothing.
>> >
> Great.
>
> I await the new patch.
>

Updated patch attached. Any thoughts or comments?

Cheers,
Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: add-junit-unit-tests-01.patch
Type: text/x-patch
Size: 30658 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110330/a9a0f5e4/add-junit-unit-tests-01.patch 