Replace reflection with delegation (#119)

Author: sbabcoc

pom.xml

@@ -32,7 +32,7 @@
     <settings.version>2.3.10</settings.version>
     <junit.version>4.13.2</junit.version>
     <testng.version>6.10</testng.version>
-    <bytebuddy.version>1.11.13</bytebuddy.version>
+    <bytebuddy.version>1.12.8</bytebuddy.version>
     <logback.version>1.2.5</logback.version>
     <junitparams.version>1.1.1</junitparams.version>
     <powermock.version>2.0.9</powermock.version>
@@ -224,7 +224,10 @@
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-surefire-plugin</artifactId>
         <configuration>
-          <argLine>-javaagent:src/test/resources/test-agent.jar</argLine>
+          <argLine>
+            -javaagent:src/test/resources/test-agent.jar
+            --add-opens java.base/java.lang=ALL-UNNAMED
+          </argLine>
         </configuration>
       </plugin>
       <plugin>
@@ -298,7 +301,7 @@
         <configuration>
           <archive>
             <manifestEntries>
-              <Premain-Class>com.nordstrom.automation.junit.LifecycleHooks</Premain-Class>
+              <Premain-Class>com.nordstrom.automation.junit.JUnitAgent</Premain-Class>
               <Can-Redefine-Classes>false</Can-Redefine-Classes>
               <Can-Retransform-Classes>true</Can-Retransform-Classes>
             </manifestEntries>

src/main/java/com/nordstrom/automation/junit/AnnotationsAccessor.java

@@ -0,0 +1,17 @@
+package com.nordstrom.automation.junit;
+
+import java.lang.annotation.Annotation;
+
+/**
+ * This interface declares the annotations accessor method for the {@code Description} class.
+ */
+public interface AnnotationsAccessor {
+    
+    /**
+     * Get the annotations of this description.
+     * 
+     * @return array of {@link Annotation} objects
+     */
+    Annotation[] annotations();
+
+}

src/main/java/com/nordstrom/automation/junit/AtomicTest.java

@@ -4,6 +4,7 @@
 
 import java.lang.annotation.Annotation;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 import java.util.Objects;
@@ -36,7 +37,8 @@ public class AtomicTest {
     private Throwable thrown;
 
     private static final Pattern PARAM = Pattern.compile("[(\\[]");
-
+    private static final List<Class<? extends Annotation>> TEST_TYPES = Arrays.asList(Test.class, Theory.class);
+    
     public AtomicTest(Description description) {
         this.runner = Run.getThreadRunner();
         this.description = description;
@@ -118,27 +120,22 @@ public boolean includes(FrameworkMethod method) {
     }
 
     /**
-     * Determine if this atomic test represents a "theory" method.
+     * Determine if this atomic test represents a "theory" method permutation.
      * 
-     * @return {@code true} if this atomic test represents a "theory" method; otherwise {@code false}
+     * @return {@code true} if this atomic test represents a permutation; otherwise {@code false}
      */
     public boolean isTheory() {
         return isTheory(description);
     }
 
     /**
-     * Determine if the specified description represents a "theory" method.
+     * Determine if the specified description represents a "theory" method permutation.
      * 
      * @param description JUnit method description
-     * @return {@code true} if the specified description represents a "theory" method; otherwise {@code false}
+     * @return {@code true} if the specified description represents a permutation; otherwise {@code false}
      */
     public static boolean isTheory(Description description) {
-        try {
-            String uniqueId = LifecycleHooks.getFieldValue(description, "fUniqueId");
-            return ((uniqueId != null) && (uniqueId.startsWith("theory-id: ")));
-        } catch (IllegalAccessException | NoSuchFieldException | SecurityException e) {
-            return false;
-        }
+        return DescribeChild.isPermutation(description);
     }
 
     /**
@@ -157,11 +154,21 @@ public boolean isTest() {
      * @return {@code true} if description represents a test method; otherwise {@code false} 
      */
     public static boolean isTest(Description description) {
+        return (getTestAnnotation(description) != null);
+    }
+    
+    /**
+     * Get the annotation that marks the specified description as a test method.
+     * 
+     * @param description JUnit description object
+     * @return if description represents a test method, the {@link Test} or {@link Theory} annotation;
+     * otherwise {@code null}
+     */
+    public static Annotation getTestAnnotation(Description description) {
         for (Annotation annotation : description.getAnnotations()) {
-            if (annotation instanceof Test) return true;
-            if (annotation instanceof Theory) return true;
+            if (TEST_TYPES.contains(annotation.annotationType())) return annotation;
         }
-        return false;
+        return null;
     }
 
     /**

src/main/java/com/nordstrom/automation/junit/DescribeChild.java

@@ -1,11 +1,11 @@
 package com.nordstrom.automation.junit;
 
-import java.lang.reflect.Field;
 import java.util.Arrays;
 import java.util.concurrent.Callable;
 
 import org.junit.experimental.theories.PotentialAssignment.CouldNotGenerateValueException;
 import org.junit.experimental.theories.Theories.TheoryAnchor;
+import org.junit.experimental.theories.Theory;
 import org.junit.experimental.theories.internal.Assignments;
 import org.junit.runner.Description;
 import org.junit.runners.model.FrameworkMethod;
@@ -20,18 +20,7 @@
  */
 public class DescribeChild {
 
-    private static final Field uniqueId;
-    
-    static {
-        Field field = null;
-        try {
-            field = Description.class.getDeclaredField("fUniqueId");
-            field.setAccessible(true);
-        } catch (NoSuchFieldException | SecurityException e) {
-            field = null;
-        }
-        uniqueId = field;
-    }
+    private static final String PERM_TAG = "theory-id: ";
 
     /**
      * Interceptor for the {@link org.junit.runners.ParentRunner#describeChild describeChild} method.
@@ -59,17 +48,22 @@ public static Description intercept(@This final Object runner,
                     method.getName(), method.getAnnotations());
         }
 
-        // if able to override [uniqueId] of test
-        if ((uniqueId != null) && AtomicTest.isTest(description)) {
+        // if describing a theory method, but not tagged as a permutation
+        if ((description.getAnnotation(Theory.class) != null) && !isPermutation(description)) {
             try {
                 // get parent of test runner
                 Object parent = LifecycleHooks.getFieldValue(runner, "this$0");
                 // if child of TheoryAnchor statement
                 if (parent instanceof TheoryAnchor) {
                     // get assignments for this theory permutation
                     Assignments assignments = LifecycleHooks.getFieldValue(runner, "val$complete");
-                    // inject permutation ID into description
-                    injectPermutationId(description, assignments);
+                    // compute permutation ID
+                    String permutationId = computePermutationId(description, assignments);
+                    // if permutation ID was computed
+                    if (permutationId != null) {
+                        // inject computed permutation ID
+                        ((UniqueIdMutator) description).setUniqueId(permutationId);
+                    }
                 }
             } catch (IllegalAccessException | NoSuchFieldException | SecurityException | IllegalArgumentException e) {
                 // nothing to do here
@@ -79,42 +73,33 @@ public static Description intercept(@This final Object runner,
     }
 
     /**
-     * Inject permutation ID into the specified description, overriding its default ID.
-     *  
+     * Determine if the specified description represents a "theory" permutation.
+     * 
+     * @param description JUnit {@link Description} object
+     * @return {@code true} if permutation is described; otherwise {@code false}
+     */
+    static boolean isPermutation(final Description description) {
+        return ((UniqueIdAccessor) description).getUniqueId().toString().startsWith(PERM_TAG);
+    }
+    
+    /**
+     * Compute permutation ID for the specified description and assignments.
+     * 
      * @param description description of "theory" method
      * @param assignments arguments for this permutation
+     * @return theory method permutation ID (may be {@code null})
      */
-    private static void injectPermutationId(final Description description, final Assignments assignments) {
+    private static String computePermutationId(final Description description, final Assignments assignments) {
         try {
             Object[] args = assignments.getMethodArguments();
             Object[] perm = new Object[args.length + 1];
             perm[0] = description.getDisplayName();
             System.arraycopy(args, 0, perm, 1, args.length);
             int permutationId = Arrays.hashCode(perm);
-            String theoryId = String.format("theory-id: %08X", permutationId);
-            uniqueId.set(description, theoryId);
-        } catch (CouldNotGenerateValueException | SecurityException |
-                IllegalArgumentException | IllegalAccessException eaten) {
-            // nothing to do here
-        }
-    }
-    
-    /**
-     * Make a childless copy of the specified description.
-     * 
-     * @param description JUnit description
-     * @return copy of the specified description, including unique ID
-     */
-    static Description makeChildlessCopyOf(final Description description) {
-        Description descripCopy = description.childlessCopy();
-        if (uniqueId != null) {
-            try {
-                uniqueId.set(descripCopy, uniqueId.get(description));
-            } catch (IllegalArgumentException | IllegalAccessException eaten) {
-                // nothing to do here
-            }
+            return String.format(PERM_TAG + "%08X", permutationId);
+        } catch (CouldNotGenerateValueException e) {
+            return null;
         }
-        return descripCopy;
     }
 
 }

src/main/java/com/nordstrom/automation/junit/EachTestNotifierInit.java

@@ -66,6 +66,7 @@ public static void interceptor(@Argument(0) final RunNotifier notifier,
                         if (description.equals(entry.getValue())) {
                             // subject method resolved
                             method = entry.getKey();
+                            break;
                         }
                     }
                 } catch (IllegalAccessException | NoSuchFieldException | SecurityException e) {
@@ -189,6 +190,7 @@ static void releaseMappingsFor(EachTestNotifier notifier) {
         RunReflectiveCall.releaseCallableOf(description);
         ArtifactCollector.releaseWatchersOf(description);
         CreateTest.releaseMappingsFor(atomicTest.getRunner(), atomicTest.getIdentity(), target);
+        GetAnnotations.releaseAnnotationsFor(atomicTest.getIdentity());
     }
 
     /**

src/main/java/com/nordstrom/automation/junit/GetAnnotation.java

@@ -0,0 +1,55 @@
+package com.nordstrom.automation.junit;
+
+import java.lang.annotation.Annotation;
+import java.util.Objects;
+
+import org.junit.runners.model.FrameworkMethod;
+
+import net.bytebuddy.implementation.bind.annotation.Argument;
+import net.bytebuddy.implementation.bind.annotation.This;
+
+/**
+ * This class declares the interceptor for the {@link org.junit.runners.model.FrameworkMethod#getAnnotation} method.
+ */
+public class GetAnnotation {
+    
+    /**
+     * Interceptor for the {@link org.junit.runners.model.FrameworkMethod#getAnnotation} method.
+     * <p>
+     * <b>NOTE</b>: This interceptor does <b>not</b> invoke the original implementation in {@link FrameworkMethod}. It
+     * relies instead on the cached annotations collected by the {@link GetAnnotations} class, which injects a proxied
+     * replacement for the <b>{@code @Test}</b> annotation that enables global test timeout management.
+     * 
+     * @param <T> desired annotation type
+     * @param method target {@link FrameworkMethod} object
+     * @param annotationType desired annotation type
+     * @return this element's annotation for the specified annotation type if present on this element, else null
+     * @throws NullPointerException if the given annotation class is {@code null}
+     */
+    public static <T extends Annotation> T intercept(@This final FrameworkMethod method, @Argument(0) final Class<T> annotationType) {
+        Objects.requireNonNull(annotationType);
+        for (Annotation annotation : GetAnnotations.getAnnotationsFor(method)) {
+            if (annotation.annotationType().equals(annotationType)) {
+                return annotationType.cast(annotation);
+            }
+        }
+        return null;
+    }
+    
+    /**
+     * Inject the specified proxy annotation into the indicated method.
+     * 
+     * @param method target {@link FrameworkMethod} object
+     * @param proxyAnnotation mutable proxy annotation ({@link MutableTest})
+     */
+    static void injectProxy(FrameworkMethod method, Annotation proxyAnnotation) {
+    	Annotation[] annotations = GetAnnotations.getAnnotationsFor(method);
+    	for (int i = 0; i < annotations.length; i++) {
+    		if (annotations[i].annotationType().equals(proxyAnnotation.annotationType())) {
+    			annotations[i] = proxyAnnotation;
+    			break;
+    		}
+    	}
+    }
+    
+}

src/main/java/com/nordstrom/automation/junit/GetAnnotations.java

@@ -0,0 +1,56 @@
+package com.nordstrom.automation.junit;
+
+import java.lang.annotation.Annotation;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.junit.runners.model.FrameworkMethod;
+
+import net.bytebuddy.implementation.bind.annotation.This;
+
+/**
+ * This class declares the interceptor for the {@link org.junit.runners.model.FrameworkMethod#getAnnotations} method.
+ */
+public class GetAnnotations {
+    
+    private static final Map<Integer, Annotation[]> ANNOTATIONS = new ConcurrentHashMap<>();
+
+    /**
+     * Interceptor for the {@link org.junit.runners.model.FrameworkMethod#getAnnotations} method.
+     * 
+     * @param method target {@link FrameworkMethod} object
+     * @return the annotations attached to the target method
+     * @throws Exception {@code anything} (exception thrown by the intercepted method)
+     */
+    public static Annotation[] intercept(@This final FrameworkMethod method) throws Exception {
+        return getAnnotationsFor(method);
+    }
+    
+    /**
+     * Returns the annotations on the specified method.
+     * <p>
+     * <b>NOTE</b>: This method caches the annotations attached to the Java {@link Method} wrapped by the specified
+     * JUnit framework method and returns this on subsequent calls.
+     * 
+     * @param method target {@link FrameworkMethod} object
+     * @return array of annotations for the specified method
+     */
+    static Annotation[] getAnnotationsFor(FrameworkMethod method) {
+        Annotation[] annotations = ANNOTATIONS.get(method.hashCode());
+        if (annotations == null) {
+            annotations = method.getMethod().getAnnotations();
+            ANNOTATIONS.put(method.hashCode(), annotations);
+        }
+        return annotations;
+    }
+    
+    /**
+     * Release the cached annotations for the specified JUnit framework method.
+     * 
+     * @param method target {@link FrameworkMethod} object
+     */
+    static void releaseAnnotationsFor(FrameworkMethod method) {
+        ANNOTATIONS.remove(method.hashCode());
+    }
+
+}