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

[Danesh Dadachanji]

On 12/03/12 12:36 PM, Jiri Vanek wrote:
> 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.
>

Will do.

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

Thanks for the comments! Pushed to HEAD[1]. The regression could occur 
in 1.2 as well so can I push there too?

Cheers,
Danesh

[1] http://icedtea.classpath.org/hg/icedtea-web/rev/0de1a0ec2ddc