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

[Deepak Bhole]

* Omair Majid <omajid at redhat.com> [2011-03-23 16:58]:
> 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.
> 

Ah, fair enough.

As for testng, yes, I am fine with using it.

Cheers,
Deepak

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