Fwd: Re: ping: [FYI][icedtea-web] minor changes in reproducers engine

[Omair Majid]

Hi Jiri,

Sorry about the delay in replying. I was quite busy with other stuff.

On 07/21/2011 04:31 AM, Jiri Vanek wrote:
>
>
> *tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java:
> String containing "localhost"  have been declared as final constant.
>       (SERVER_NAME) have been  moved instant Server instance so each
> server can have it name without affecting others
>       (getUrl()) added -  can return URL of server singleton.
> Implementation of this method is inside server, so each server can
> return its own useful URL.
>       (saveFile()) is now public.
>       Added identification for ThreadedProcess based on commandlineArgs
> and its run is now slowed by Thread.sleep
>       (ServerLuncher) inner class is now public (it was bug to not be as
> we have getIndependentInstance of it method ) and renamed to ServerLauncher
>       Enchanted wrapping of executeProcess
>

The formatting looks wrong, but I am going to assume you will fix this 
when editing the ChangeLog file.

>
> diff -r 86abbf8be0b1 tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java
> --- a/tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java	Thu Jun 23 15:29:45 2011 +0200
> +++ b/tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java	Mon Jul 11 13:27:20 2011 +0200
> @@ -576,8 +579,25 @@
>           }
>
>           public ThreadedProcess(List<String>  args) {
> +            this.args = args;
> +        }
>
> -            this.args = args;
> +        public String getCommandLine() {
> +            String commandLine = "unknown command";
> +            try {
> +                if (args != null&&  args.size()>  0) {
> +                    commandLine = "";
> +                    for (Iterator<String>  it = args.iterator(); it.hasNext();) {
> +                        String string = it.next();
> +                        commandLine = commandLine + " " + string;
> +
> +                    }

This is more of a FYI, but i think it's clearer to do it this way:

for (String string: args) {
   commandLine = commandLine + " " + string;
}

But it's fine the way it is. You don't have to change it.

> @@ -608,14 +628,26 @@
>        * wrapper arround tiny http server to separate lunch configgurations and servers.
>        * to allow terminations and stuff arround.
>        */
> +    public static class ServerLauncher implements Runnable {
>
> -    static class ServerLuncher implements Runnable {
> -
> +        /**
> +         * default url name part.
> +         * This can be changed in runtime, but will affect all following tasks upon those server
> +         */
> +        private String serverName = DEFAULT_LOCALHOST_NAME;
>           private boolean running;
>           private final Integer port;
>           private final File dir;
>
> -        public ServerLuncher(Integer portt, File dirr) {
> +        public String getServerName() {
> +            return serverName;
> +        }
> +
> +        public void setServerName(String serverName) {
> +            this.serverName = serverName;
> +        }
> +
> +        public ServerLauncher(Integer portt, File dirr) {

Typo here. Should be 'port' and 'dir'

Other than those minor problems, it looks fine to me. Okay to commit 
after fixing those.

Cheers,
Omair