harmony-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexey Petrenko (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HARMONY-2709) [classlib][swing] javax.swing.plaf.basic.BasicInternalFrameUI.setupMenuOpenKey() throws unspecified IllegalArgumentException
Date Fri, 16 Mar 2007 10:16:10 GMT

    [ https://issues.apache.org/jira/browse/HARMONY-2709?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12481549
] 

Alexey Petrenko commented on HARMONY-2709:
------------------------------------------

Sergey,
 
here is few additional things about your patches for this issue.
 
There are three additional unit test failures with your patch: BasicInternalFrameUITest.testGetMinimumSize,
BasicInternalFrameUITest.testBasicInternalFrameUI and BasicInternalFrameUITest.testGetPreferredSize.
Could you please check that all the unit tests are passed with your patch as it required by
"Good issue resolution guideline".
 
Here is a part of your patch:
=== cut === 
    protected void setupMenuOpenKey() {
         Object[] keys = (Object[])UIManager.get("InternalFrame.windowBindings");
         if (keys == null) {
             return;
         }
-
+        if (frame == null) {
+            throw new NullPointerException();
+        }
         InputMap map = LookAndFeel.makeComponentInputMap(frame, keys);
=== cut ===
The test shows that RI throws NPE even if UIManager returns null for this key. But Harmony
with your patch will return silently.
So your patch introduces two regressions to Harmony.
 
Here is your patch for the test:
=== cut ===  
+        try {
+            new BasicInternalFrameUI(null).setupMenuOpenKey();
+        } catch (NullPointerException npe) {
+            // PASSED
+        } catch (IllegalArgumentException e) {
+            fail("IllegalArgumentException thrown");
+        }
=== cut === 
 
I would change the test in the following way:
=== cut ===  
+        try {
+            new BasicInternalFrameUI(null).setupMenuOpenKey();
+            fail("NPE expected");
+        } catch (NullPointerException npe) {
+            // PASSED
+        }
=== cut === 
The test should fail in case of not exceptions. We do not need additional catch-fail clause
for IAE.
So I can not apply your test patch too.
 
Could you please be more careful with you patches in the future.
 
Thanks in advance.

P.S. I'll prepare new patches myself. 

> [classlib][swing] javax.swing.plaf.basic.BasicInternalFrameUI.setupMenuOpenKey() throws
unspecified IllegalArgumentException
> ----------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HARMONY-2709
>                 URL: https://issues.apache.org/jira/browse/HARMONY-2709
>             Project: Harmony
>          Issue Type: Bug
>          Components: Classlib
>            Reporter: Alexander Simbirtsev
>         Assigned To: Alexey Petrenko
>         Attachments: Harmony-2709-BasicInternalFrameUI.patch, Harmony-2709-BasicInternalFrameUITest.patch
>
>
> Compatibility
> There is no mention of any exception in the specification.
> Harmony throws unspecified IllegalArgumentException for setupMenuOpenKey() while RI throws
unspecified NPE.
> Use the following code to reproduce:
> import javax.swing.JInternalFrame;
> import javax.swing.plaf.basic.BasicInternalFrameUI;
> import junit.framework.TestCase;
> public class Test extends TestCase {
>     public void testcase1() {                                    
>         try {
>             testBasicInternalFrameUI fr = new testBasicInternalFrameUI(null);
>             fr.setupMenuOpenKey();
>         } catch (IllegalArgumentException e) {
>             fail("exception thrown");
>         } catch (NullPointerException e) {
>             // expected
>         }
>     }
> }
> class testBasicInternalFrameUI extends BasicInternalFrameUI {
>     testBasicInternalFrameUI(JInternalFrame s){
>         super(s);
>     }
>     
>     public void setupMenuOpenKey()  {
>         super.setupMenuOpenKey();
>     }
> }

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message