Fwd: Ping: [RFC][icedtea-web] reviewer needed - added engine for lunching reproducers

[Omair Majid]

On 06/16/2011 02:56 PM, Jiri Vanek wrote:
> On 06/15/2011 03:08 PM, Jiri Vanek wrote:
>>
>> I have solved the ProcessAssasin issue:)
>> Trick was in fact, that not process itself should be killed, but
>> thread keeping the process. Strange, but working.
>> Any knowhow arround welcomed!
>> (new patch is comming soon)
>> J.
>>
>
>
> Hi!
>
> This is final version of reproducers engine - now the assassin is
> working properly (inspired by jtreg), and also streams are handled "better"
> Nearly no changes in logic since last patch - just threads are corectly
> synchronized.
>

The patch looks fine to me. There are some things that I can think of 
that we might want to change, but that can be done later, after the 
patch is committed.

I do have a few concerns that I have noted below. I am fine with the 
patch once you address them.

> Regards and looking forward to write reproducers J.
>

Thanks in advance!

> +public class DeadLockTestTest {
> +
> +    private static ServerAccess server = new ServerAccess();
> +
> +
> +
> +   @Test
> +    public void testDeadLockTestTerminated() throws Exception {
> +        System.out.println("connecting deadlocktest request");
> +        ServerAccess.ProcessResult pr=server.executeJavawsHeadless(null,"/deadlocktest.jnlp");
> +        System.out.println(pr.stdout);
> +        System.err.println(pr.stderr);
> +        Assert.assertFalse(pr.stdout.contains("This process is hanging more then 30s. Should be killed"));
> +//        Assert.assertTrue(pr.stderr.contains("xception"));, exception is thrown by engine,not by application
> +        Assert.assertTrue(pr.wasTerminated);
> +        Assert.assertEquals(null, pr.returnValue);//killed process have no value
> +    }
> +
> +
> +
> +
> +    }
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +

Please remove extra whitespace.

> +public class SimpleTest1Test {
> +
> +    private static ServerAccess server = new ServerAccess();
> +
> +
> +
> +    @Test
> +    public void testSimpletest1lunchOk() throws Exception {
> +        System.out.println("connecting simpletest1 request");
> +        ServerAccess.ProcessResult pr=server.executeJavawsHeadless(null,"/simpletest1.jnlp");
> +        System.out.println(pr.stdout);
> +        System.err.println(pr.stderr);
> +        Assert.assertTrue(pr.stdout.contains("Good simple javaws exapmle"));
> +        Assert.assertFalse(pr.stderr.contains("xception"));
> +        Assert.assertFalse(pr.wasTerminated);
> +        Assert.assertEquals((Integer)0, pr.returnValue);
> +    }
> +
> +
> +
> +
> +
> +    }
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +

Likewise.


> +public class SimpleTest2Test {
> +
> +    private static ServerAccess server = new ServerAccess();
> +
> +
> +    @Test
> +    public void testSimpletest2lunchException() throws Exception {
> +        System.out.println("connecting simpletest2 request");
> +        ServerAccess.ProcessResult pr=server.executeJavawsHeadless(null,"/simpletest2.jnlp");
> +        System.out.println(pr.stdout);
> +        System.err.println(pr.stderr);
> +        Assert.assertTrue(pr.stdout.trim().length()<  1);
> +        Assert.assertTrue(pr.stderr.contains("Correct exception"));
> +        Assert.assertTrue(pr.stderr.contains("Exception"));
> +        Assert.assertFalse(pr.wasTerminated);
> +        //Assert.assertFalse(0==pr.returnValue);exception and still returned 0?
> +    }
> +
> +
> +
> +    }
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +

Same here.

> +public class ResourcesTest {
> +
> +    private static ServerAccess server = new ServerAccess();
> +
> +    @Test
> +    public void testResourcesExists() throws Exception {
> +        File[] simpleContent = server.getDir().listFiles(new FileFilter() {
> +
> +            public boolean accept(File file) {
> +                if (!file.isDirectory()) {
> +                    return true;
> +                } else {
> +                    return false;
> +                }
> +            }
> +        });
> +        Assert.assertNotNull(simpleContent);
> +        Assert.assertTrue(simpleContent.length>  5);
> +
> +        for (int i = 0; i<  simpleContent.length; i++) {
> +            File file = simpleContent[i];
> +            System.err.println(file.getName());
> +            if (file.getName().toLowerCase().endsWith(".jnlp")) {
> +                String c = server.getResourceAsString("/" + file.getName());
> +                Assert.assertTrue(c.contains("<"));
> +                Assert.assertTrue(c.contains(">"));
> +                Assert.assertTrue(c.contains("jnlp"));
> +                Assert.assertTrue(c.contains("resources"));
> +                Assert.assertTrue(c.replaceAll("\\s*", "").contains("</jnlp>"));
> +
> +            } else {
> +                byte[] c = server.getResourceAsBytes("/" + file.getName()).toByteArray();
> +                Assert.assertEquals(c.length, file.length());
> +            }
> +
> +        }
> +
> +    }
> +}
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +
> +

And here.

> diff -r 7ae04e8d830a tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ServerAccess.java	Thu Jun 16 20:52:18 2011 +0200
> @@ -0,0 +1,607 @@
> +/*
> + * To change this template, choose Tools | Templates
> + * and open the template in the editor.
> + */

Please remove this comment and replace it with a proper copyright.

> +
> +/**
> + *
> + * @author jvanek
> + */
> +public class ServerAccess {

Please add some javadocs to this file. Also, you list yourself as author 
here, but not in the AUTHORS file. Perhaps you want to add yourself 
there too?

Cheers,
Omair