[rfc][icedtea-web] reproducer for PR955

[Danesh Dadachanji]

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!

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

Regards,
Danesh