[rfc][icedtea-web] signed applets test

[Jiri Vanek]

On 04/11/2012 08:55 PM, Danesh Dadachanji wrote:
...
>> There is question how to deal with -Xtrustall which is now useless when there is trusted
>> certificate (all signed reproducers are right
>> now signed with same cert, and when more to much reproducers will need to much different
>> certificates this will need re-factoring
>> probably too).
>>
>
> I don't mind -Xtrustall staying, I think it helps for testing stuff on the fly, outside the test
> suite. However, I'm not too strong about this so if you want to remove it, feel free.
>

Yap. I'm for to keep it inside too.

>> After this change is in, the PR905 reproducer have sense to go in too.
>>
>
> +1
>
>> changelog:
...
> There are a few typos, it should be:
>

fixed .. I hope...

>  > diff -r cde6d59a2901 Makefile.am
>  > --- a/Makefile.am Thu Apr 05 10:57:16 2012 -0400
>  > +++ b/Makefile.am Wed Apr 11 11:32:36 2012 +0200
>  > @@ -23,6 +23,12 @@
>  > JNLP_TESTS_SERVER_DEPLOYDIR=$(TESTS_DIR)/jnlp_test_server
>  > JNLP_TESTS_DIR=$(TESTS_DIR)/jnlp_tests
>  > KEYSTORE_NAME=teststore.ks
>  > +KEYSTORE_PASS=123456789
>
> I'm curious, why was this password chosen over the regular "changeit"? I know that you're getting it
> from below but I'm wondering if anyone knows why the original dev chose this over the normal one. I
> would rather it be "changeit" for consistency's sake though. What do you think?
>
>  > +EXPORTED_CERT=icedteatests.crt
>  > +TEST_CERT_ALIAS=icedteaweb
>  > +PUBLIC_KEYSTORE=~/.icedtea/security/trusted.certs
>  > +PUBLIC_PASS=changeit
>  > +
>
> [snip]
>
>  > diff -r cde6d59a2901 tests/jnlp_tests/signed/AppletTestSigned/resources/AppletTestSigned.html
>  > --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>  > +++ b/tests/jnlp_tests/signed/AppletTestSigned/resources/AppletTestSigned.html Wed Apr 11
> 11:32:36 2012 +0200
>
> Heh, nice failure message. ;) It's because the applet was eventually killed, right?
>
> I'm worried that such a harsh failure message will throw testers off, they may not realize it is
> meant to fail to run. Thoughts? It might be useful to add a "(This applet was killed, it should not
> be running)" message too?

nn - First reason was to distinguish normal applet's page/signed applet's page, then I got evil 
mind, some nice memories to blue death remembered 1.4, and result was this  distinguisher :)

I have removed it, and restoree original red. This was putting to big effort for test-writers. 
However, this can be isnpiration for feature ;)
>
> [snip]
>
> diff -r cde6d59a2901 tests/jnlp_tests/signed/AppletTestSigned/resources/AppletTestSigned.html
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/tests/jnlp_tests/signed/AppletTestSigned/resources/AppletTestSigned.html Wed Apr 11 11:32:36
> 2012 +0200
> [...]
>  > +<vendor>NetX</vendor>
>  > +<homepage href="http://jnlp.sourceforge.net/netx/"/>
>
> Please change these to use IcedTea and the test page[1], thanks.
>
grrr. Done. Thanx for warning.
> [snip]
>
>  > diff -r cde6d59a2901 tests/jnlp_tests/signed/AppletTestSigned/resources/bd.css
>  > --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>  > +++ b/tests/jnlp_tests/signed/AppletTestSigned/resources/bd.css Wed Apr 11 11:32:36 2012 +0200
>
> Shouldn't this have a copyright header too? Otherwise if it's easier, you can include it directly in
> the HTML file instead of a new file.

I n case blue death like  message will go in, then it definitely will be good idea to have separate 
css file and so not to write it again and again...
>
> [snip]
>
>  > diff -r cde6d59a2901 tests/jnlp_tests/signed/AppletTestSigned/srcs/AppletTestSigned.java
>  > --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>  > +++ b/tests/jnlp_tests/signed/AppletTestSigned/srcs/AppletTestSigned.java Wed Apr 11 11:32:36
> 2012 +0200
>  > @@ -0,0 +1,82 @@
>  > +
>  > +import java.applet.Applet;
>  > +
>  > +/* AppletTestSigned.java
>  > +Copyright (C) 2011 Red Hat, Inc.
>
> s/2011/2012/
>
>
> [snip]
>
>  > diff -r cde6d59a2901 tests/jnlp_tests/signed/AppletTestSigned/testcases/AppletTestSignedTests.java
>  > --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>  > +++ b/tests/jnlp_tests/signed/AppletTestSigned/testcases/AppletTestSignedTests.java Wed Apr 11
> 11:32:36 2012 +0200
>  > @@ -0,0 +1,100 @@
>  > +/* AppletTestTests.java
>  > +Copyright (C) 2011 Red Hat, Inc.
>
> s/2011/2012/
>
> [snip]
>
>  > + Assert.assertTrue("AppletTestSigned stdout should contains " + s3 + " bud didn't",
> pr.stdout.contains(s3));
>  > + String s0 = "AppletTestSigned was started";
>  > + Assert.assertTrue("AppletTestSigned stdout should contains " + s0 + " bud didn't",
> pr.stdout.contains(s0));
>  > + String s1 = "value1";
>  > + Assert.assertTrue("AppletTestSigned stdout should contains " + s1 + " bud didn't",
> pr.stdout.contains(s1));
>  > + String s2 = "value2";
>  > + Assert.assertTrue("AppletTestSigned stdout should contains " + s2 + " bud didn't",
> pr.stdout.contains(s2));
>  > + String s4 = "AppletTestSigned was stopped";
>  > + Assert.assertFalse("AppletTestSigned stdout shouldn't contains " + s4 + " bud did",
> pr.stdout.contains(s4));
>  > + String s5 = "AppletTestSigned will be destroyed";
>  > + Assert.assertFalse("AppletTestSigned stdout shouldn't contains " + s5 + " bud did",
> pr.stdout.contains(s5));
>  > + String ss = "xception";
>  > + Assert.assertFalse("AppletTestSigned stderr should not contains " + ss + " but did",
> pr.stderr.contains(ss));
>
> For "AppletTestSigned stdout should contains ", s/ contains / contain / (without the s). Also
> s/bud/but/ =)

All above should e fixed. Thanx for review!

>
> Everything else looks good! Thanks for getting this done.
>
> Cheers,
> Danesh
>
> [1] http://icedtea.classpath.org/wiki/IcedTea-Web#Testing_IcedTea-Web

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signedApplets2.diff
Type: text/x-patch
Size: 19122 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120412/f19dd85f/signedApplets2.diff