Hi Joel,

The new version is better, but for the testing in question I would prefer to see something even simpler like:

    public static void main(String[] args) throws Exception {
        int failed = 0;
        Class<?>[] testData = {/* list of class literals*/}
        for (Class<?> toTest: testData) {
            Object res = toTest.getAnnotatedSuperclass();

            if (res != null) {
                failed++;
System.out.println(toTest + ".getAnnotatedSuperclass() returns: "
                        + res + ", was non-null");
            }
        }

        if (failed != 0)
throw new RuntimeException("Test failed, check log for details");
    }


-Joe

On 8/21/2013 5:04 AM, Joel Borggrén-Franck wrote:
Hi Joe, Paul

I rewrote the test in Paul's style without using testNG.

http://cr.openjdk.java.net/~jfranck/8022343/webrev.01/

Please review.

cheers
/Joel

On 2013-08-19, Joe Darcy wrote:
Hi Joel,

I agree the code looks fine.

However, I concur with the general sentiment of Paul test advice
without advocating using testng for this task.

A loop over a Class<?>[] initialized with the kinds of values of
interest would seem to be better structured to me and allow for
better exception handing, etc.

-Joe

On 08/19/2013 01:34 AM, Paul Sandoz wrote:
Hi Joel,

The fix looks OK.

Not suggesting you do the following, unless you really want to, but the test is 
an example of where TestNG data providers are useful, since all cases will be 
tested and reported for pass or failure, rather than in this case the first 
failure will cause other checks (if any) not to be tested and in addition will 
not report which check failed (one needs to look at the stack trace).

Not tested:

   @DataProvider(name = "class")
   private static Object[][] getClasses() {
       // Using the stream API because i can :-)
       // Arguably simpler in this case to use new Object[][] { {} }
       return Stream.of(
                                 Object.class,
                                 If.class,
                                 Object[].class,
                                 void.class,
                                 int.class).
               map(e -> new Object[] { e }).
               toArray(Object[][]::new);
   }

   @Test(dataProvider = "class")
   public void testAnnotatedSuperClassIsNull(Class c) {
       assertNull(c.getAnnotatedSuperclass())
   }

Paul.

On Aug 16, 2013, at 2:17 PM, Joel Borggren-Franck <joel.fra...@oracle.com> 
wrote:

Hi

Please review this small fix for a type annotation reflection issue.

The javadoc spec for Class.getAnnotatedSuperclass says:

* If this Class represents either the Object class, an interface type, an
* array type, a primitive type, or void, the return value is null.

The patch fixes this.

Webrev at: http://cr.openjdk.java.net/~jfranck/8022343/webrew.00/

Patch also included it at the end of this mail.

cheers
/Joel



diff -r b07b19182e40 src/share/classes/java/lang/Class.java
--- a/src/share/classes/java/lang/Class.java    Thu Aug 15 15:04:59 2013 +0100
+++ b/src/share/classes/java/lang/Class.java    Fri Aug 16 13:20:31 2013 +0200
@@ -3338,8 +3338,16 @@
      * @since 1.8
      */
     public AnnotatedType getAnnotatedSuperclass() {
-         return 
TypeAnnotationParser.buildAnnotatedSuperclass(getRawTypeAnnotations(), 
getConstantPool(), this);
-}
+        if(this == Object.class ||
+                isInterface() ||
+                isArray() ||
+                isPrimitive() ||
+                this == Void.TYPE) {
+            return null;
+        }
+
+        return 
TypeAnnotationParser.buildAnnotatedSuperclass(getRawTypeAnnotations(), 
getConstantPool(), this);
+    }

     /**
      * Returns an array of AnnotatedType objects that represent the use of 
types to
diff -r b07b19182e40 test/java/lang/annotation/TypeAnnotationReflection.java
--- a/test/java/lang/annotation/TypeAnnotationReflection.java   Thu Aug 15 
15:04:59 2013 +0100
+++ b/test/java/lang/annotation/TypeAnnotationReflection.java   Fri Aug 16 
13:20:31 2013 +0200
@@ -23,7 +23,7 @@

/*
  * @test
- * @bug 8004698 8007073
+ * @bug 8004698 8007073 8022343
  * @summary Unit test for type annotations
  */

@@ -58,7 +58,7 @@
     }

     private static void testSuper() throws Exception {
-        check(Object.class.getAnnotatedSuperclass().getAnnotations().length == 
0);
+        check(Object.class.getAnnotatedSuperclass() == null);
         check(Class.class.getAnnotatedSuperclass().getAnnotations().length == 
0);

         AnnotatedType a;
diff -r b07b19182e40 
test/java/lang/annotation/typeAnnotations/GetAnnotatedSuperclass.java
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/test/java/lang/annotation/typeAnnotations/GetAnnotatedSuperclass.java     
Fri Aug 16 13:20:31 2013 +0200
@@ -0,0 +1,50 @@
+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8022343
+ * @summary make sure Class.getAnnotatedSuperclass() returns null when 
specified to do so
+ */
+
+import java.util.*;
+import java.lang.annotation.*;
+import java.lang.reflect.*;
+import java.io.Serializable;
+
+public class GetAnnotatedSuperclass {
+    public static void main(String[] args) throws Exception {
+        check(Object.class.getAnnotatedSuperclass() == null);
+        check(If.class.getAnnotatedSuperclass() == null);
+        check(Object[].class.getAnnotatedSuperclass() == null);
+        check(void.class.getAnnotatedSuperclass() == null);
+        check(int.class.getAnnotatedSuperclass() == null);
+    }
+
+    private static void check(boolean b) {
+        if (!b)
+            throw new RuntimeException();
+    }
+    interface If {}
+}
+

Reply via email to