[rfc][icedtea-web] closing listener idea

[Jiri Vanek]

On 09/18/2012 06:36 PM, Jiri Vanek wrote:
> 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)

Ok. This basic part posted to review: [icedtea-web][rfc] Basic closing listeners implementation and 
example
>>
>>>
>>>
>>>
>>>> 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.


ok. Here is attached second set of listeners - Ruels one. I still consider them as much more 
bullet-proof, safer, and in finish also much less verbose.

I have added your chaining-add methods, although add/Not/Matches/Contains/  (all except addRule) are 
in conflict with the double code I wanted to prevent by adding them.

If you mind, can you please considered as review of this second part?


For now I'm considering the "tools-jar" as abandoned ok?


Thanx for ideas,
  J.

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

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