[fyi][icedtea-web] fixed Browsers.none behaviour

[Danesh Dadachanji]

On 28/06/12 09:42 AM, Jiri Vanek wrote:
> modified.browsers.run==all|one were ignoring Browser.none behaviout. This is fixing it.
> Also tehre was bug in ResourceTest  with missing slashes causing unrelated failures. Fixed.
>

What do you think about checking if whatever is passed into execute* starts with a "/", if not, adding one? Seems like this slips 
through the cracks every now and then.

Everything looks good, just 2 very minor nitpicks below, then feel free to push!

>
> Thanx for check,
>      J.
>
> 2012-06-27  Jiri Vanek <jvanek at redhat.com>
>
>      Fixed resource tests and Browsers.none
>      * tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ResourcesTest.java:
>      Added slash into all executeBrowsers urls.
>      Added midori and epiphany to simple proxies test.
>      * tests/netx/jnlp_testsengine/net/sourceforge/jnlp/browsertesting/BrowserTestRunner.java:
>      Corrected handling of Browsers.none together with -Dmodified.browsers.run swith
>
> fixedBrowsers.noneBehaviour.diff
>
>
> diff -r ee7c412bbd8f tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ResourcesTest.java
> --- a/tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ResourcesTest.java	Tue Jun 26 12:41:49 2012 +0200
> +++ b/tests/netx/jnlp_testsengine/net/sourceforge/jnlp/ResourcesTest.java	Thu Jun 28 15:35:44 2012 +0200
> @@ -48,7 +48,6 @@
>   import net.sourceforge.jnlp.browsertesting.BrowserTest;
>   import net.sourceforge.jnlp.browsertesting.Browsers;
>   import net.sourceforge.jnlp.annotations.NeedsDisplay;
> -import net.sourceforge.jnlp.annotations.TestInBrowsers;
>   import net.sourceforge.jnlp.browsertesting.browsers.LinuxBrowser;
>   import net.sourceforge.jnlp.annotations.TestInBrowsers;
>   import org.junit.Assert;
> @@ -62,7 +61,7 @@
>       @NeedsDisplay
>       public void testNonExisitngBrowserWillNotDeadlock() throws Exception {
>           server.setCurrentBrowser(Browsers.none);
> -        ServerAccess.ProcessResult pr = server.executeBrowser("simpletest1.jnlp");
> +        ServerAccess.ProcessResult pr = server.executeBrowser("/simpletest1.jnlp");
>           Assert.assertNull(pr.process);
>           Assert.assertEquals(pr.stderr, "");
>           Assert.assertEquals(pr.stdout, "");
> @@ -87,7 +86,7 @@
>       @Test
>       @TestInBrowsers(testIn=Browsers.none)
>       public void testNonExisitngBrowserWillNotCauseMess() throws Exception {
> -        ServerAccess.ProcessResult pr = server.executeBrowser("simpletest1.jnlp");
> +        ServerAccess.ProcessResult pr = server.executeBrowser("/simpletest1.jnlp");
>           Assert.assertNull(pr.process);
>           Assert.assertEquals(pr.stderr, "");
>           Assert.assertEquals(pr.stdout, "");
> @@ -175,15 +174,15 @@
>           expected = 2;
>           Assert.assertTrue("Created from  " + s + "there must be " + expected + " browsers in factory. Is" + bf.getAllBrowsers().size(), bf.getAllBrowsers().size() == expected);
>
> -        s = Browsers.firefox.toExec() + ":" + Browsers.chromiumBrowser + ":" + Browsers.googleChrome.toExec() + ":" + Browsers.opera;
> +        s = Browsers.firefox.toExec() + ":" + Browsers.chromiumBrowser + ":" + Browsers.googleChrome.toExec() + ":" + Browsers.opera+":" + Browsers.epiphany+":" + Browsers.midori;

Could you fix the spacing between the +":" bits?

>           bf = new BrowserFactory(s);
> -        expected = 4;
> +        expected = 6;
>           Assert.assertTrue("Created from  " + s + "there must be " + expected + " browsers in factory. Is" + bf.getAllBrowsers().size(), bf.getAllBrowsers().size() == expected);
>           testFullFactory(bf);
>
>           s = "fgfd/" + Browsers.firefox.toExec() + ":" + "/fgfd/" + Browsers.chromiumBrowser + ":" + "fgfd/dfsdf/" + Browsers.googleChrome.toExec() + ":" +"/g/fgfd/" + Browsers.opera;
>           bf = new BrowserFactory(s);
> -        expected = 4;
> +        expected = 6;
>           Assert.assertTrue("Created from  " + s + "there must be " + expected + " browsers in factory. Is" + bf.getAllBrowsers().size(), bf.getAllBrowsers().size() == expected);
>           testFullFactory(bf);
>
> @@ -289,7 +288,7 @@
>                   e2.append(s).append("\n");
>               }
>           };
> -       ServerAccess.ProcessResult pr=server.executeBrowser("simpletest1.jnlp",le,lo);
> +       ServerAccess.ProcessResult pr=server.executeBrowser("/simpletest1.jnlp",le,lo);

Spacing =)

>           server.setCurrentBrowser(BrowserFactory.getFactory().getFirst().getID());
>           Assert.assertNotNull(server.getCurrentBrowsers());
>           Assert.assertNotNull(server.getCurrentBrowser());
> @@ -310,6 +309,8 @@
>           Assert.assertEquals(bf.getBrowser(Browsers.googleChrome).getID(), Browsers.googleChrome);
>           Assert.assertEquals(bf.getBrowser(Browsers.firefox).getID(), Browsers.firefox);
>           Assert.assertEquals(bf.getBrowser(Browsers.opera).getID(), Browsers.opera);
> +        Assert.assertEquals(bf.getBrowser(Browsers.epiphany).getID(), Browsers.epiphany);
> +        Assert.assertEquals(bf.getBrowser(Browsers.midori).getID(), Browsers.midori);
>       }
>
>       private void testBrowser(Browser browser) throws IOException {
> diff -r ee7c412bbd8f tests/netx/jnlp_testsengine/net/sourceforge/jnlp/browsertesting/BrowserTestRunner.java
> --- a/tests/netx/jnlp_testsengine/net/sourceforge/jnlp/browsertesting/BrowserTestRunner.java	Tue Jun 26 12:41:49 2012 +0200
> +++ b/tests/netx/jnlp_testsengine/net/sourceforge/jnlp/browsertesting/BrowserTestRunner.java	Thu Jun 28 15:35:44 2012 +0200
> @@ -36,12 +36,16 @@
>                   String mbr = System.getProperty("modified.browsers.run");
>                   if (mbr != null) {
>                       if (mbr.equalsIgnoreCase("all")) {
> -                        testableBrowsers = BrowserFactory.getFactory().getBrowsers(new Browsers[]{Browsers.all});
> +                        if (!isBrowsersNoneSet(tib)) {
> +                            testableBrowsers = BrowserFactory.getFactory().getBrowsers(new Browsers[]{Browsers.all});
> +                        }
>                       } else if (mbr.equalsIgnoreCase("one")) {
>                           //this complication here is for case like
>                           // namely enumerated concrete browsers, so we want to pick up
>                           // random one from those already enumerated
> -                        testableBrowsers = Arrays.asList(new Browsers[]{testableBrowsers.get(new Random().nextInt(testableBrowsers.size()))});
> +                        if (isBrowsersNoneSet(tib)) {
> +                            testableBrowsers = Arrays.asList(new Browsers[]{testableBrowsers.get(new Random().nextInt(testableBrowsers.size()))});
> +                        }
>                       } else if (mbr.equalsIgnoreCase("ignore")) {
>                           testableBrowsers = BrowserFactory.getFactory().getBrowsers(new Browsers[]{Browsers.none});
>                           browserIgnoration = true;
> @@ -66,6 +70,13 @@
>           }
>       }
>
> +    private boolean isBrowsersNoneSet(TestInBrowsers tib) {
> +        if (tib.testIn().length == 1 && tib.testIn()[0] == Browsers.none) {
> +            return true;
> +        }
> +        return false;
> +    }
> +
>       private void injectBrowserCatched(FrameworkMethod method, Browsers browser) {
>           try {
>               injcetBrowser(method, browser);
>

Please go ahead and push!

Cheers,
Danesh