[RFC][icedtea-web] Test for regression of ServiceManager not setup correctly

[Jiri Vanek]

On 02/21/2012 08:07 PM, Danesh Dadachanji wrote:
> On 17/02/12 06:08 PM, Omair Majid wrote:
>> On 02/17/2012 05:34 PM, Danesh Dadachanji wrote:
>>> Hi,
>>>
>>> Here's a test that checks whether or not ServiceManager is setup
>>> correctly by the time an applet constructor is called. The constructor
>>> uses BasicService and will throw a NPE if it is not setup.
>>>
>>
>> Would it be possible to extend this test to check that services are
>> available in init(), start(), stop() and destroy() as well?
>
> This is definitely a good idea, I've added checking in init/start but I don't see a way of reaching stop/destroy without the awt robot actually closing the applet window. I looked into sending a WINDOW_CLOSING event somehow but don't have access to the window object. When that event is heard, AppletInstance#destroy() is called (eventually) but the application instance is something we don't have access to from within the applet itself. I have left the methods and respective test cases commented out with FIXMEs, if you'd rather me remove them then let me know.
>
awt robot is getting longer and longer run :(
But in-browser tests are lttle bit closer. This will needs to be checked in browser later too.
> That's the main reason the killer thread is there though, if (hopefully once) we have a way of closing the applet, all the killer threads being used just to destroy the applet should be removed.
>
>  >
>> There also seem to be some spacing/indentation issues in the patch.
>
> Ah I thought I cleaned this up before sending, fixed in this patch.
>
>>> The test is for checking regression against this changeset:
>>> http://icedtea.classpath.org/hg/icedtea-web/rev/221174bcd4ec
>>>
>>> Thank you very much to Jiri for helping me get this through the door!
>>>
>>> +2012-02-17 Danesh Dadachanji<ddadacha at redhat.com>
>>> +
>>> + Adding test for regression of JNLP API accessibility in constructor
>>> + methods of applets.
>>> + * Makefile.am: Added classes.jar to classpath when compiling
>>> jnlp_tests.
>>> + * tests/jnlp_tests/simple/CheckServices/: Tests ServiceManager is
>>> setup
>>> + correctly when called from applet constructors.
>>
>> Please list all the files added or changed.
>>
>
> Done below, apologies for being lazy. =)
>
> +2012-02-21 Danesh Dadachanji <ddadacha at redhat.com>
> +
> + Adding test for regression of JNLP API accessibility in constructor
> + methods of applets.
> + * Makefile.am: Added classes.jar to classpath when compiling jnlp_tests.
> + * tests/jnlp_tests/simple/CheckServices/resources/CheckServices.jnlp:
> + New test file added. Tests ServiceManager is setup correctly when called
> + from applet constructors.
> + * tests/jnlp_tests/simple/CheckServices/srcs/CheckServices.java: Same.
> + * tests/jnlp_tests/simple/CheckServices/testcases/CheckServicesTests.java:
> + Same as above.
> +
>
>> In general, am not sure how good these applet tests might be. We know
>> that applets run under javaws use different code paths from applets run
>> under a browser. Still it's better to have a few tests than none.
>
As mentioned above in-html tests will be necessary to be added when browser-testing will be pushed.



There are spaces instead of tabs in changelog. After fixing it, looks OK. TYVM and feel free to add this to growing family of tests;)

J.
> Responding to this in the reply to Jiri's email.
>
> Thanks for the review!
>
> Cheers,
> Danesh