[rfc][icedtea-web] closing listener idea

[Adam Domurad]

Apologies - I originally replied off list.


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

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