[rfc][icedtea-web] reproducer for PR955

[Jiri Vanek]

On 08/10/2012 06:19 PM, Danesh Dadachanji wrote:
>
>
> On 09/08/12 08:54 AM, Jiri Vanek wrote:
>> Sorry minor (but important) typo in variable w1 false->true.
>>
>>
>> -------- Original Message --------
>> Subject: [rfc][icedtea-web] reproducer for PR955
>> Date: Thu, 09 Aug 2012 14:50:39 +0200
>> From: Jiri Vanek <jvanek at redhat.com>
>> To: IcedTea Distro List <distro-pkg-dev at openjdk.java.net>
>> CC: Danesh Dadachanji <ddadacha at redhat.com>
>>
>> A agreed with Danesh, here is set of reproducers to reprodcue PR955.
>> Except the hack which is forwarding modified locales to subproces the work is quite simple.
>>
>
> Thank you very much for taking care of this! Some comments below.
>
>>
>> tests/reproducers/simple/LocalisedInformationElement/resources/LocalisedInformationElement1.jnlp:
>>
>> tests/reproducers/simple/LocalisedInformationElement/resources/LocalisedInformationElement2.jnlp:
>>
>> tests/reproducers/simple/LocalisedInformationElement/resources/LocalisedInformationElement3.jnlp:
>>
>> tests/reproducers/simple/LocalisedInformationElement/resources/LocalisedInformationElement4.jnlp:
>>      Test jnlp files with various combinations of locales, reproducers of PR955.
>>
>> tests/reproducers/simple/LocalisedInformationElement/resources/LocalisedInformationElement_noLoc.jnlp
>>      Jnlp file with which is not affected by PR955 and is helping to catch error in LOCALE
>> changing hack
>>      tests/reproducers/simple/LocalisedInformationElement/srcs/LocalisedInformationElement.java:
>>      Reproducer main calss, after loading prints out default locale.
>>
>> tests/reproducers/simple/LocalisedInformationElement/testcases/LocalisedInformationElementTest.java:
>>      Testacases launching above jnlps under various locales.
>
> Typo in test cases. =) I also assume your actual changelog has the right format (files starting with
> * and whatnot).
>
>>      tests/test-extensions/net/sourceforge/jnlp/ServerAccess.java:
>>      Added set of methods allowing passing of custom variables to ThreadedProcess.
>>      tests/test-extensions/net/sourceforge/jnlp/ThreadedProcess.java:
>>      Added processing of custom variables.
>>
>>
>> J.
>>
>>
>>
>>
>> localesReprodcuer_2.diff
>>
>>
>
> [snip]
>
>> diff -r 1d033579d40e
>> tests/reproducers/simple/LocalisedInformationElement/srcs/LocalisedInformationElement.java
>> --- /dev/null    Thu Jan 01 00:00:00 1970 +0000
>> +++
>> b/tests/reproducers/simple/LocalisedInformationElement/srcs/LocalisedInformationElement.java
>> Thu Aug 09 14:36:39 2012 +0200
>> @@ -0,0 +1,53 @@
>> +
>> +import java.util.Locale;
>> +
>> +/* SimpleTest1.java
>> +Copyright (C) 2011 Red Hat, Inc.
>
> This isn't SimpleTest1 and it's no longer 2011. ;)
>
> [snip]
>
>> diff -r 1d033579d40e
>> tests/reproducers/simple/LocalisedInformationElement/testcases/LocalisedInformationElementTest.java
>> --- /dev/null    Thu Jan 01 00:00:00 1970 +0000
>> +++
>> b/tests/reproducers/simple/LocalisedInformationElement/testcases/LocalisedInformationElementTest.java
>> Thu Aug 09 14:36:39 2012 +0200
>> @@ -0,0 +1,356 @@
>> +/* SimpleTest1Test.java
>> +Copyright (C) 2011 Red Hat, Inc.
>> +
>> +This file is part of IcedTea.
>
> Here too!
>
> [snip]
>
>> +
>> +    public static ProcessResult evaluateLocalisedInformationElementTest(String id, String[]
>> variables, boolean verbose) throws Exception {
>> +        ProcessResult pr = executeJavaws(verbose, variables, id);
>> +        String s = "LocalisedInformationElement launched";
>> +        Assert.assertTrue(id + " stdout should contains " + s + " bud didn't",
>> pr.stdout.contains(s));
>> +        //to strict?
>> +        //String ss = "xception";
>> +        //Assert.assertFalse(id + " stderr should not contains " + ss + " but did",
>> pr.stderr.contains(ss));
>> +        String locMatch = "(?s).*default locale: \\w{2}.*";
>> +        Assert.assertTrue(id + " stdout should match " + locMatch + " bud didn't",
>> pr.stdout.matches(locMatch));
>> +        return pr;
>> +    }
>> +
>> +    public static ProcessResult evaluateLocalisedInformationElementTestNotLaunched(String id,
>> String[] variables, boolean verbose) throws Exception {
>> +        ProcessResult pr = executeJavaws(verbose, variables, id);
>> +        String s = "LocalisedInformationElement launched";
>> +        Assert.assertFalse(id + " stdout should not contains " + s + " bud didn't",
>> pr.stdout.contains(s));
>> +        String ss = "xception";
>> +        Assert.assertTrue(id + " stderr should contains " + ss + " but did",
>> pr.stderr.contains(ss));
>> +        String locMatch = "(?s).*default locale: \\w{2}.*";
>> +        Assert.assertFalse(id + " stdout should not match " + locMatch + " bud didn't",
>> pr.stdout.matches(locMatch));
>> +        String sss = "MissingVendorException";
>> +        Assert.assertTrue(id + " stderr should contains " + sss + " but did",
>> pr.stderr.contains(sss));
>> +        return pr;
>> +    }
>> +
>
> For the above two, " stderr should contains " + ss + " but did" - this should be s/but did/but didn't/
>
>> +    private static ProcessResult executeJavaws(boolean verbose, String[] variables, String id)
>> throws Exception {
>> +        List<String> oa = new ArrayList<String>(1);
>> +        if (verbose) {
>> +            oa.add("-verbose");
>> +        }
>> +        final ProcessResult pr;
>> +        if (variables == null) {
>> +            pr = server.executeJavawsHeadless(oa, "/" + id + ".jnlp");
>> +        } else {
>> +            pr = server.executeJavawsHeadless(oa, "/" + id + ".jnlp", variables);
>> +        }
>> +        return pr;
>> +    }
>> +
>> +
>> +    //the description checkis disabled for all PR955, so it is representing just
>> +    //PR955 issue. Tests with enable description check are introduced later
>> +    private final boolean w1=false;
>> +
>> +    @Test
>> +    @Bug(id = "PR955")
>> +    public void testSimpletest1lunchWithLocalisedInformation1() throws Exception {
>> +        String[] l = getChangeLocalesForSubproces("en_US.UTF-8");
>> +        ProcessResult pr =
>> evaluateLocalisedInformationElementTest("LocalisedInformationElement1", l, true);
>> +        assertTiVeDe(pr, "localisedJnlp1.jnlp1", "IcedTea", "LocalisedInformationElement1.jnlp",
>> w1);
>> +    }
>> +
>
> Could you s/lunch/Launch/g for this file? A lot of the methods have this typo throughout this file.
> (note capital L for style)
>
> [snip]
>
>
>> +
>> +    @Test
>> +    @Bug(id = "PR955")
>> +    public void testSimpletest1lunchWithLocalisedInformation1_withPeaceMissing() throws Exception {
>> +        String[] l = getChangeLocalesForSubproces("en_US.UTF-8");
>> +        ProcessResult pr =
>> evaluateLocalisedInformationElementTestNotLaunched("LocalisedInformationElement2", l, w1);
>> +
>> +    }
>> +
>> +    @Test
>> +    @Bug(id = "PR955")
>> +    public void testSimpletest1lunchWithLocalisedInformation22_withPeaceMissing() throws Exception {
>> +        String[] l = getChangeLocalesForSubproces("cs_CZ");
>> +        ProcessResult pr =
>> evaluateLocalisedInformationElementTestNotLaunched("LocalisedInformationElement2", l, w1);
>> +    }
>> +
>
> These 2 tests fail for me. You're passing in w1 which is eventually given as the verbose flag. I
> believe a direct true should be passed instead.
>
> Also, could you run s/Peace/Piece/ too? If there was no peace in icedtea-web, we wouldn't get
> anything done. ;)
>
> [snip]
>
>> +    //following tests are testing localisation of homepage
>> +    //to lazy to do...
>
> Haha, I think you have more than enough tests to justify that!
>
> Everything else looks great, once the above have been fixed, please feel free to push!

Thanx, for review,  everything should be fixed and pushed. Extra-thank for catching w1 x verbose 
mistake. I have not checked again after this one-word quick fix.

J.

>
> Thanks again for doing all of this, these are amazing tests!
>
> Regards,
> Danesh