[rfc] [icedtea-web] get rid of repated sout/serr in reproducers testcases

[Danesh Dadachanji]

On 28/05/12 05:07 AM, Jiri Vanek wrote:
> On 05/25/2012 09:08 PM, Danesh Dadachanji wrote:
>> On 25/05/12 08:55 AM, Jiri Vanek wrote:
>>> Hi!
>>>
>>> I would like to get rid of repeating std out/err reprint in reproducers by moving this into
>>> ServerAccess class. This patch is adding
>>> simple loging interface for stdout/err and (mainly) is hiding reprinting (logging since now:-) of
>>> processes out/err and "connectng: "
>>> message into executeProcess.
>>>
>>> My goal in longer term is to have html file (generated from logged xml file probably) which I will
>>> be able to link (anchor inlcuded)
>>> from current test results html files.And so I will be able to quickly find whats wrong. Now it is
>>> becoming little bit painfull. >(Both
>>> stdou/err of processes and searching in looong output file)
>>>
>>> Saad, Danesh - you have number of reproducers in development. Please count with this change.
>>>
>>>
>>
>> Thanks for letting us know. I have some comments below. Could you also add a ChangeLog entry?
>>
>
> All issues should be fixed. Except renaming - i Renamed it little bit differently: logErrorReprint and logOutputReprint. Thats because
> I would like chars before auto-completion finish it for me as short as possible:)
>

I'm all for making autocompletion easier. =)

>
> Sorry  of changelog inside patch, but I accidentally make some work in same branch before sending to you.
>

No worries. =) Just one minor comment below.

[snip]

>>> diff -r 2c84a314c411 tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java
>>> --- a/tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java Thu May 24 15:37:30 2012
>>> -0400
>>> +++ b/tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java Fri May 25 14:36:05 2012
>>> +0200
>>> @@ -116,6 +116,10 @@
>>> * all terminated processes are stored here. As wee need to 'wait' to termination to be finished.
>>> */
>>> private static Set<Thread> terminated = new HashSet<Thread>();
>>> + /**
>>> + * this flag is indicating whether ooutput of executeProcess should be logged. By default true.
>>> + */
>>
>> s/ooutput/output/
>> I think also for style, we use single lined comments for class vars - /** <comment-on-one-line */
>
> All docs in ServerAcces are multi-lined. I would like to keep them so.
>

Oh I didn't realize that. Thanks for clarifying!

[snip]

>
> diff -r 6df151bb5320 ChangeLog
> --- a/ChangeLog	Fri May 25 11:44:13 2012 -0400
> +++ b/ChangeLog	Mon May 28 10:58:03 2012 +0200
> @@ -1,3 +1,21 @@
> +2012-05-20  Jiri Vanek<jvanek at redhat.com>
> +
> +	Get rid of repeated sout/serr  in reproducers testcases/unit tests and
> +	introduce bottleneck for loging.
> +	* tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java:
> +	(PROCESS_LOG) static flag for enabling/disabling automatic logging of
> +	statically executed processes.
> +	(logErrorReprint) (logOutputReprint) (logNoReprint) new methods, to call
> +	logging bottleneck.
> +	(log) main logging bottleneck, reprint message to according stream with
> +	calling test-class and test-method as suffix.
> +	(getTestMethod) new method to analyze calling test-method.
> +	(executeProcess) enhanced for conditional automatic logging of start of
> +	process and outputs of process.
> +	*/tests/jnlp_tests/*/*/testcases/*.java : and
> +	*/tests/netx/unit/**.java: all System.out replaced by ServerAccess.logOutputReprint

Please list all the files, even if there are many:

http://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html#Style-of-Change-Logs

(Thanks to Omair for the link!)

> +	and System.err replaced by ServerAccess.logErrorReprint
> +
>   2012-05-25  Adam Domurad<adomurad at redhat.com>
>
>   	Changed for-loops over iterators and indices to for-each loops if they

Once, done feel free to commit to HEAD.

Cheers,
Danesh