[rfc][icedtea-web] closing listener idea

[Jiri Vanek]

On 09/18/2012 06:21 PM, Adam Domurad wrote:
> On Tue, 2012-09-18 at 17:35 +0200, Jiri Vanek wrote:
>> On 09/17/2012 08:54 PM, Adam Domurad wrote:
>>> On Fri, 2012-08-17 at 12:19 +0200, Jiri Vanek wrote:
>>>> On 08/15/2012 06:57 PM, Jiri Vanek wrote:
>>>>> On 08/14/2012 05:50 PM, Adam Domurad wrote:
>>>>>> On Tue, 2012-08-14 at 17:06 +0200, Jiri Vanek wrote:
>>>>>>> This idea should speed up browser tests a lot. When all conditions for pass/failure are done,
>>>>>>> browser is terminated and is not waiting for time-out any more.
>>>>>>> It is bringing some more load to tester, but its usage is mandatory.
>>>>>> Thanks very much for the ideas!
>>>>>>
>>>>>> It would have been nice if it were like this from the beginning - but
>>>>>> nevertheless this would be a great improvement.
>>>>>>
>>>>>> I'm 100% for converting all tests to this form, and mandating it for
>>>>>> future applet tests. I'm sure as the most frequent runner of the test
>>>>>> suite you can agree, at least for mandating it for future tests :)
>>>>>
>>>>> Definitely! I will need some helpt to get it to tests in head ;)
>>>>>>
>>>>>>>
>>>>>>> Usage here is demonstrated on simple applet test.
>>>>>>> Any ideas (especially how to avoid doubled "contains" or "matches") welcomed.
>>>>>>>
>>>>>>> J.
>>>>>>>
>>>>>>> * tests/reproducers/simple/AppletTest/testcases/AppletTestTests.java: Integrated ClosingListener
>>>>>>> speed-up.
>>>>>>> * tests/test-extensions/net/sourceforge/jnlp/ClosingListener.java: New file, interface for all
>>>>>>> ClosingListeners.
>>>>>>> * tests/test-extensions/net/sourceforge/jnlp/CountingClosingListener.java: Implementation of
>>>>>>> ClsoingListeners which is storing copy of complete output of program.
>>>>>>> * tests/test-extensions/net/sourceforge/jnlp/ProcessAssasin.java: added (setTimeout) method as
>>>>>>> cleanest possibility to terminate process immediately
>>>>>>> * tests/test-extensions/net/sourceforge/jnlp/ServerAccess.java: new method (setUpClosingListener)
>>>>>>> for setting necessary variables in ClosingListener
>>>>>>
>>>>>> This method is nice enough, although is it possible to have a
>>>>>> standardized way to see if an exception has caused the applet to stop ?
>>>>>>
>>>>>> I'm very much in favour of adding a clear end marker to the end of tests
>>>>>> rather than duplicating contains/matches. If you don't like a hardcoded
>>>>>> ending message, then, what if we had a closing listener implementation
>>>>>> that takes a single string as an argument, and a string is printed at
>>>>>> the end of the applet. Custom closing listeners could always also be
>>>>>> created.
>>>>>>
>>>>>
>>>>> I'm little bit against using such a constants (which must be copy pasted into reproducer to complete
>>>>> the evil ) but I admit it canhel a lot. But I would recommend to be careful with such a simple
>>>>> "terminator" - as exception can be thrown "later".
>>>>>
>>>>> Anyway some idea how to integrate this is in this updated patch.
>>>>>
>>>>> I have posted also one more idea to patch, and although it eliminates duplicity, it have quite a lot
>>>>> of code :-/.
>>>>>
>>>>> Both new apporaches shown in AppletTestSigned
>>>>>
>>>>> thanx for discussion,
>>>>>
>>>>> J.
>>>>>
>>>>>
>>>>> Ps - I have not tested this new code due to lack of time today so be patient with it. I will do in
>>>>> friday. I'm posting it for you to get the feedback for more ideas!
>>>>
>>>> No ideas ? Never mind - debugged version of above attached There was serious error in listeners
>>> I had ideas, I had to think them through though :)
>>
>> Good :)
>>
>>> I don't know if I like the verbosity here. Creating a contains-rule for
>>
>> Yap. I was little bit unhappy with it too.
>>
>>> everything separately and then combining them into an array of rules
>>> seems like a lot to simply close the browser at the right time.
>>>
>>> Better would be something like:
>>> RuleSet passingRules = new RuleSet().contains(str1, str2).matches(str3,
>>> str4);
>>> RuleSet failingRules = new RuleSet().contains(str5, str6).matches(str7,
>>> str8);
>>
>> Sure. This make sense. Just except RulesSet i would rather keep the class already in this patch
>> (unless you stroongly disagree with name). So:
> Name was illustrative, as long as its clear I have no preference
> (RulesSet isn't completely clear by itself).
>>
>> rulesFolowingClosingListener.add(rule1).add(rule2)...
>>
>> This is no problem to add and I like this idea.
> I still feel like a simple .contains(String...) and .matches(String...)
> would make this more palatable. 99% of rules are going to be 'contains'

I agree.and I can include also not/match/not/contains simplifications.

My main reason for Rules was to gather

Assert.something("something should contains/match X", s.matches/contains(X))
someClsongListener{ canClose(){ return s.matches/contains(X)}}

this s.matches/contains(X) is thre three times. The rule should unify it.


> by the looks of it. If not, it would still be worthwhile to have a
> variable-number-of-arguments add function, so you can call eg
> rulesFolowingClosingListener.add(rule1, rule2). I really want think it
> would be most natural for the existing tests if the auto closing
> listener was, in most cases, just a bunch of strings to look for, which
> would typically be named constants anyway.
>
> It's still a little unwieldy, but I see your point about why you prefer
> it, more notes below.
>>
>>> But I'm still not sure. It would be much more ideal if the applet simply
>>> signal when it is completed somehow, independent of the validation
>>> rules. I think simply checking for an ***APPLET FINISHED*** message that
>>
>> I agree. Jana have also used just this simple approach.My only concern  here is the magical string.
>> But I think we agre at this aprt, can I post final version of the idea with implemented and used
> Whether the version is final is up to reviewer ;)
>>
>> AutoOkClosingListener and AutoErrorClosingListener (although I believe only to AutoOkClosingListener
>> to be usefull at all)
>
>>
>>
>>
>>> was standard for all tests is a good-enough solution. It will cause all
>>> well-behaving applets to speed up, which should be at least the majority
>>> of them. It could also be nice to have some components common to all
>>> applets, (eg common patterns such as Killer thread), that way we could
>>> have something like TestAppletsUtil.printFinishedMessage();
>>
>> I would like to satisfy your requirement, but I'm afraid of loosing of independence and of
>> simplicity ("one jar")  of reprodcuers. But I'm also starting  to feel that this becomes necessary.
>>
>> To achieve this, "troubles" will arise and we will have to  deal with them:
>>    - tools jars must be compiled and deployed to jnlp_test_server (no issue here except setting IDEs)
>>    - all reproducers (or some mark that "this particular one" ) have to be compiled with such a tool
>> jar/classes on classapth
>> - the applets/apps which are really using such a code must have this tools jar on classapth by html/jnlp
>>
>> both above are breaking pureness of reproducer. I will be more happy if `some mark that "this
>> particular one"` will be invented but here I'm hesitating too [rfc] :)
> You're right ... I totally overlooked that the applets can't just
> magically access this jar :). That does make it a lot more messy than I
> thought.
>
>>
>>>
>>>> (also was caused your listeners in previous reproducer not working). It is included here but I'm
>>>> going to fix this separately.
>>
>> After posting Auto*ClosingListeners to review for you, I will attach patch or "patch"  with rest of
>> listeners and with some "tools jar" to this thread to keep solving this.
>>
>> Please consider the advantages and disadvantages of "tools jar" and of other ClosingListeners
>>
>> I still think that the generalized  Rules listeners can bring more advantages then the tools jars.
>> Damn I hate the tools jar. It can bring so much profit [magical string constants, killer and such a
>> stuff..] but I'm really afraid it can bring more unforeseen bad :( [failures due to one more jar on
>> classapth both compile and runtime time]
> I can understand your disdain for the idea... it is somewhat of an
> artificial addition to what are supposed to be simple isolated tests.
>
> There is the dark route of adding more testing based flags, so that eg a
> checkable message is printed whenever an applet destroy() or maybe
> stop() method is called. This has the similar brittleness of a hardcoded
> message however, and escapes the isolation of the test system....
> Probably bad news.
>
> Your disdain for the tools jar idea is very understandable, and I'm
> inclined to agree. Overall, I'm starting to think that a standard,
> magic, "**APPLET FINISHED**" string copypasted into every applet is the
> best route. There would then be a convenient default closing listener
> provided that checks for "**APPLET FINISHED**". For most uses, this
> would be good enough. Where there are multiple exit scenarios and a
> single magic string isn't good, the other closing listeners could be
> used.
>

agreed.

> Thanks for looking into this,
> - Adam
>>
>>>>
>>>>
>>>> J.
>>>>
>>>
>>>
>>
>
>