Reviewer needed - fix for regression test java/awt/Insets/WindowWithWarningTest/WindowWithWarning

Mon Nov 29 07:46:52 PST 2010

On 14:49 Mon 29 Nov     , Pavel Tisnovsky wrote:
> Dr Andrew John Hughes wrote:
> > On 19:41 Fri 26 Nov     , Pavel Tisnovsky wrote:
> >> Hi all,
> >>
> >> can anyone please review fix for regression test
> >> java/awt/Insets/WindowWithWarningTest/WindowWithWarning? Mercurial
> >> export for IcedTea6 HEAD (1.10pre) is included in this mail as attachment.
> >>
> >> Description of the fix:
> >>
> >> This test must be run in an environment similar to sandbox used for
> >> running regular applets, which among other things means that each window
> >> created by the test itself has to be labelled by warning message "Java
> >> Applet Window".
> >>
> >> But for the test to (probably successfuly) proceed it is also required
> >> that AWT Robot is enabled (and it is disabled by default for applets,
> >> which reasonable, of course).
> >>
> >> To guarantee both conditions to run this test I copied system security
> >> policy file, added new line to enable AWT robot and then configured the
> >> test itself to use this policy file, because JTreg harness usually
> >> starts such tests (applets) without security manager installed.
> >>
> >> PS: I've made this fix with great help of Andrew, Deepak and Omair -
> >> thank you very much colleagues!
> >>
> >> Cheers
> >> Pavel
> > 
> > This looks pretty standard, compared with other JDK tests.
> > 
> > Could we not just allow the needed robot permission?  Are all the ones
> > for the policy file really needed?
> 
> Hi Andrew,
> 
> you are right, I've tried to run this test with simplified policy file.
> Second version of test fix is included in attachment - can you please
> review it?
> 

Changes look good.  You need to add a comment for Makefile.am in
the Changelog.  Otherwise, ok for commit.

> Btw, is it possible to use webrev instead of hg export to send changes
> for review? It might be easier to read (but I'm not sure if it's
> possible to use webrev tool from Sun/Oracle to another project than
> OpenJDK).
> 

It's possible, but I don't see why anyone would want to.  It's much
easier to review patches attached in e-mails IMHO; you don't have to
find somewhere to upload to and the reviewer doesn't have to mess
about with a web browser to view it. What's your issue with using hg
diff/export?  webrev does the same thing anyway.

> Pavel

> # HG changeset patch
> # User ptisnovs
> # Date 1291038232 -3600
> # Node ID 5bea2af2803f3bc5c5253e5e6255acea77834f2d
> # Parent  c3150835fe12e852c987f26440f86e65da40391d
> Regression test fix - setup of SecurityManager to run this test as
> regular applet but with AWT robot enabled.
> 
> diff -r c3150835fe12 -r 5bea2af2803f ChangeLog
> --- a/ChangeLog	Mon Nov 29 08:50:36 2010 +0100
> +++ b/ChangeLog	Mon Nov 29 14:43:52 2010 +0100
> @@ -1,3 +1,10 @@
> +2010-11-29  Pavel Tisnovsky  <ptisnovs at redhat.com>
> +
> +	* Makefile.am:
> +	* patches/jtreg-WindowWithWarningTest.patch:
> +	Regression test fix - setup of SecurityManager to
> +	run this test as regular applet but with AWT robot enabled.
> +
>  2010-11-29  Matthias Klose  <doko at ubuntu.com>
>  
>  	* patches/hotspot/hs19/params-cast-size_t.patch: Update for hs19.
> diff -r c3150835fe12 -r 5bea2af2803f Makefile.am
> --- a/Makefile.am	Mon Nov 29 08:50:36 2010 +0100
> +++ b/Makefile.am	Mon Nov 29 14:43:52 2010 +0100
> @@ -300,7 +300,8 @@
>  	patches/openjdk/6850606-bigdecimal_regression.patch \
>  	patches/openjdk/6876282-bigdecimal_divide.patch \
>  	patches/f14-fonts.patch \
> -	patches/jtreg-DeleteFont.patch
> +	patches/jtreg-DeleteFont.patch \
> +	patches/jtreg-WindowWithWarningTest.patch
>  
>  if WITH_ALT_HSBUILD
>  ICEDTEA_PATCHES += \
> diff -r c3150835fe12 -r 5bea2af2803f patches/jtreg-WindowWithWarningTest.patch
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/patches/jtreg-WindowWithWarningTest.patch	Mon Nov 29 14:43:52 2010 +0100
> @@ -0,0 +1,50 @@
> +--- /dev/null	1970-01-01 01:00:00.000000000 +0100
> ++++ openjdk/jdk/test/java/awt/Insets/WindowWithWarningTest/applet.policy	2010-11-26 18:01:32.000000000 +0100
> +@@ -0,0 +1,13 @@
> ++
> ++// Standard extensions get all permissions by default
> ++
> ++grant codeBase "file:${{java.ext.dirs}}/*" {
> ++	permission java.security.AllPermission;
> ++};
> ++
> ++// default permissions granted to all domains
> ++
> ++grant { 
> ++	permission java.awt.AWTPermission "createRobot", "enabled";
> ++};
> ++
> +--- openjdk-old/jdk/test/java/awt/Insets/WindowWithWarningTest/WindowWithWarningTest.html	2010-06-21 23:15:49.000000000 +0200
> ++++ openjdk/jdk/test/java/awt/Insets/WindowWithWarningTest/WindowWithWarningTest.html	2010-11-26 18:29:25.622555000 +0100
> +@@ -4,7 +4,7 @@
> +         @bug 6391770
> +         @summary Content of the Window should be laid out in the area left after WarningWindow was added.
> +         @author Yuri Nesterenko
> +-        @run applet WindowWithWarningTest.html
> ++        @run applet/othervm/policy=applet.policy WindowWithWarningTest.html
> + -->
> + 
> +   <head>
> +--- openjdk-old/jdk/test/java/awt/Insets/WindowWithWarningTest/WindowWithWarningTest.java	2010-06-21 23:15:49.000000000 +0200
> ++++ openjdk/jdk/test/java/awt/Insets/WindowWithWarningTest/WindowWithWarningTest.java	2010-11-26 18:10:15.000000000 +0100
> +@@ -53,6 +53,7 @@
> + import java.awt.*;
> + import java.awt.event.*;
> + import javax.swing.*;
> ++import java.security.*;
> + 
> + //Automated tests should run as applet tests if possible because they
> + // get their environments cleaned up, including AWT threads, any
> +@@ -91,12 +92,6 @@
> +     public void start ()
> +     {
> +         //Get things going.  Request focus, set size, et cetera
> +-        System.setSecurityManager( new SecurityManager() {
> +-        // deny AWTPermission("showWindowWithoutWarningBanner")
> +-            public boolean checkTopLevelWindow(Object window) {
> +-                return false;
> +-            }
> +-         });
> +         JFrame frame = new JFrame("Window Test");
> +         frame.setBounds(50, 50, 200, 200);
> +         frame.show();

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net
PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint = F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8