[rfc][icedtea-web] signed applets test

[Danesh Dadachanji]

On 12/04/12 04:02 AM, Jiri Vanek wrote:
> 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...
>

I missed a few last time. Sorry :S

+2012-04-11  Jiri Vanek  <jvanek at redhat.com>
+
+	Allowed signed appletes in automatic reproducers tests

s/appletes/applets

+	* tests/jnlp_tests/signed/AppletTestSigned/resources/AppletTestSigned.html:
+	html file for launching signed applet. It still is different from the one for
+	calling unsigned applets - red.

Is the " - red" bit at the end still relevant? In fact, I think the entire sentence isn't quite necessary anymore. I think we can 
recognize that if it's for launching a signed applet, it'd be a different page than launching an unsigned one. =)

IMO, "html file for launching signed applet." is enough. It's up to you if you want to keep it though. =)

+	* tests/jnlp_tests/signed/AppletTestSigned/resources/AppletTestSigned.jnlp:
+	jnlp file for lunched signed applet

s/lunched/launched

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

I don't think you noticed this comment last time, any thoughts?

>> > +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 ;)

Okay. =)

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

Hmm okay, something to keep in mind if it does go in then.

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

Awesome, thanks! After the above typos are fixed, go ahead and push to HEAD.

Cheers,
Danesh