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

[Omair Majid]

On 03/23/2011 04:26 PM, Deepak Bhole wrote:
> * Omair Majid<omajid at redhat.com>  [2011-03-22 15:09]:
>> Hi,
>>
>> The attached patch adds unit tests for the JNLP parser to
>> icedtea-web. A simple 'make check' should run these tests.
>>
>> The unit tests rely on the testng framework. The patch adds optional
>> dependencies on testng, qdox and bsh for running these unit tests. I
>> have also added a dependency on xlst for transforming the test
>> output into a more jtreg-like format (useful for the buildbots)
>>
>> I also looked into using junit for these tests, but junit's command
>> line output is rather limited. To generate xml output, ant is
>> required as well.
>>
>> Any thoughts or comments?
>>
>
> I'll let Andrew go over the makefile changes. As for the code, it looks
> good to me. I got 4 failures. 1 seems to be an error in the test, and
> the other 3 are either test errors (exception not caught) or parser
> errors (exception shouldn't be thrown in the first place). It seems more
> like the former though.. unless our parser is equipped to handle that
> kind of bad xml?
>

I was more curious about adding the various test dependencies to 
icedtea-web. Are you fine with using testng as the test framework?

As for the test failures, most (all?) of them are expected. Some of 
these tests are testing cases (like invalid xml) that break our parser.

> <snip>
>> +
>> +    @Test
>> +    public void testCommentInAttributes() throws ParseException {
>> +        String malformedJnlp = "<?xml?><jnlp spec='<!-- something -->'></jnlp>";
>> +        Node root = Parser.getRootNode(new ByteArrayInputStream(malformedJnlp.getBytes()));
>> +        Parser p = new Parser(null, null, root, false, false);
>> +        assertEquals("<!-- something -->", p.getSpecVersion().toString());
>> +    }
>> +
>
> The above test fails. However I think the test itself might be wrong. If
> there is no spec # and there is just a comment instead, shouldn't it
> default to 1.0+ (which is what it is doing)?
>

Hmm.. good point. Would you mind if I fix this after pushing this entire 
changeset?

> <snip>
>> +
>> +    @Test
>> +    public void testMalformedArguments() throws ParseException {
>> +        String malformedJnlp = originalJnlp.replace("arg2</argument", "arg2<argument");
>> +        Parser.getRootNode(new ByteArrayInputStream(malformedJnlp.getBytes()));
>> +    }
>> +
>
> The above test fails.
>

Expected, I suppose. The test creates invalid xml; our parser fails with 
that. There are JNLP files that omit such closing tags. Whether those 
applications are broken is another issue; I just wanted to capture that 
there JNLP files with invalid markup.

>> +    @Test
>> +    public void testTagNotClosed() throws ParseException {
>> +        String malformedJnlp = originalJnlp.replace("</jnlp>","<jnlp>");
>> +        Parser.getRootNode(new ByteArrayInputStream(malformedJnlp.getBytes()));
>> +    }
>> +
>
> The above test fails.
>

Just like the previous test, this tests creates invalid XML.

>> +    @Test
>> +    public void testUnquotedAttributes() throws ParseException {
>> +        String malformedJnlp = originalJnlp.replace("'jnlp.jnlp'", "jnlp.jnlp");
>> +        Parser.getRootNode(new ByteArrayInputStream(malformedJnlp.getBytes()));
>> +    }
>> +
>
> The above test fails.
>

Again, this is invalid XML.

Thanks for looking over the patch.

Thanks,
Omair