refacto: proposal implementation for testng-team#3111

Author: juherr

testng-core-api/src/main/java/org/testng/IFactoryMethod.java

@@ -1,12 +1,8 @@
 package org.testng;
 
-import java.util.Optional;
-
 /** Represents a factory method */
 public interface IFactoryMethod {
 
-  /**
-   * @return - Returns parameters associated with a factory method wrapped within a {@link Optional}
-   */
-  Optional<Object[]> getParameters();
+  /** @return - Returns parameters associated with a factory method */
+  Object[] getParameters();
 }

testng-core-api/src/main/java/org/testng/IInstanceInfo.java

@@ -1,14 +1,15 @@
 package org.testng;
 
+import java.util.UUID;
+
 /**
  * This class defines a pair of instance/class. A method with @Factory can return an array of these
  * objects instead of Object[] so that instances can be dynamic proxies or mock objects and still
  * provide enough information to TestNG to figure out what classes the annotations should be looked
  * up in.
- *
- * @author <a href="mailto:[email protected]">Cedric Beust</a>
  */
 public interface IInstanceInfo<T> {
+  UUID getUid();
 
   /** @return The instance on which the tests will be invoked. */
   T getInstance();

testng-core-api/src/main/java/org/testng/IMethodInstance.java

@@ -5,5 +5,6 @@ public interface IMethodInstance {
 
   ITestNGMethod getMethod();
 
+  // TODO deprecate return IInstanceInfo<?>
   Object getInstance();
 }

testng-core-api/src/main/java/org/testng/ITestClassInstance.java

@@ -1,10 +1,7 @@
 package org.testng;
 
 /** Represents the ability to retrieve the parameters associated with a factory method. */
-public interface ITestClassInstance<T> {
-
-  /** @return - The actual instance associated with a factory method */
-  T getInstance();
+public interface ITestClassInstance<T> extends IInstanceInfo<T> {
 
   /**
    * @return - The actual index of instance associated with a factory method. This index has a 1:1
@@ -24,6 +21,8 @@ public interface ITestClassInstance<T> {
    */
   int getInvocationIndex();
 
+  IFactoryMethod getFactoryMethod();
+
   static Object embeddedInstance(Object original) {
     if (original instanceof ITestClassInstance) {
       return ((ITestClassInstance) original).getInstance();

testng-core-api/src/main/java/org/testng/ITestNGMethod.java

@@ -2,7 +2,6 @@
 
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import org.testng.annotations.CustomAttribute;
@@ -39,6 +38,7 @@ public interface ITestNGMethod extends Cloneable {
    */
   String getMethodName();
 
+  // TODO deprecate return IInstanceInfo<?>
   Object getInstance();
 
   /**
@@ -266,18 +266,11 @@ default boolean isDataDriven() {
    * @return - A {@link IParameterInfo} object that represents details about the parameters
    *     associated with the factory method.
    */
+  @Deprecated
   default IParameterInfo getFactoryMethodParamsInfo() {
     return null;
   }
 
-  /**
-   * @return - A {@link IFactoryMethod} implementation that contains attributes associated with a
-   *     factory method, wrapped within an {@link Optional}.
-   */
-  default Optional<IFactoryMethod> getFactoryMethod() {
-    return Optional.empty();
-  }
-
   /**
    * @return - An array of {@link CustomAttribute} that represents the custom attributes associated
    *     with a test.

testng-core-api/src/main/java/org/testng/internal/IParameterInfo.java

@@ -1,19 +1,18 @@
 package org.testng.internal;
 
 import org.testng.ITestClassInstance;
-import org.testng.ITestNGMethod;
 
 /**
  * Represents the ability to retrieve the parameters associated with a factory method.
  *
  * @deprecated - This interface stands deprecated as of TestNG <code>7.11.0</code>.
  */
 @Deprecated
-public interface IParameterInfo extends ITestClassInstance {
+public interface IParameterInfo<T> extends ITestClassInstance<T> {
   /**
    * @return - The parameters associated with the factory method as an array.
    * @deprecated - This method stands deprecated as of TestNG <code>7.11.0</code> Please use {@link
-   *     ITestNGMethod#getFactoryMethod()} to retrieve the parameters.
+   *     ITestClassInstance#getFactoryMethod()} to retrieve the parameters.
    */
   @Deprecated
   Object[] getParameters();

testng-core/src/main/java/org/testng/ClassMethodMap.java

@@ -16,11 +16,11 @@
  * @author <a href='mailto:the[dot]mindstorm[at]gmail[dot]com'>Alex Popescu</a>
  */
 public class ClassMethodMap {
-  private final Map<Object, Collection<ITestNGMethod>> classMap = Maps.newConcurrentMap();
+  private final Map<IInstanceInfo<?>, Collection<ITestNGMethod>> classMap = Maps.newConcurrentMap();
   // These two variables are used throughout the workers to keep track
   // of what beforeClass/afterClass methods have been invoked
-  private final Map<ITestClass, Set<Object>> beforeClassMethods = Maps.newConcurrentMap();
-  private final Map<ITestClass, Set<Object>> afterClassMethods = Maps.newConcurrentMap();
+  private final Map<ITestClass, Set<IInstanceInfo<?>>> beforeClassMethods = Maps.newConcurrentMap();
+  private final Map<ITestClass, Set<IInstanceInfo<?>>> afterClassMethods = Maps.newConcurrentMap();
 
   public ClassMethodMap(List<ITestNGMethod> methods, XmlMethodSelector xmlMethodSelector) {
     for (ITestNGMethod m : methods) {
@@ -31,7 +31,7 @@ public ClassMethodMap(List<ITestNGMethod> methods, XmlMethodSelector xmlMethodSe
         continue;
       }
 
-      Object instance = m.getInstance();
+      IInstanceInfo<?> instance = (IInstanceInfo<?>) m.getInstance();
       classMap.computeIfAbsent(instance, k -> new ConcurrentLinkedQueue<>()).add(m);
     }
   }
@@ -43,7 +43,7 @@ public ClassMethodMap(List<ITestNGMethod> methods, XmlMethodSelector xmlMethodSe
    * @param instance The test instance
    * @return true if it is the last of its class
    */
-  public boolean removeAndCheckIfLast(ITestNGMethod m, Object instance) {
+  public boolean removeAndCheckIfLast(ITestNGMethod m, IInstanceInfo<?> instance) {
     Collection<ITestNGMethod> l = classMap.get(instance);
     if (l == null) {
       throw new IllegalStateException(
@@ -60,19 +60,19 @@ public boolean removeAndCheckIfLast(ITestNGMethod m, Object instance) {
     return true;
   }
 
-  public Map<ITestClass, Set<Object>> getInvokedBeforeClassMethods() {
+  public Map<ITestClass, Set<IInstanceInfo<?>>> getInvokedBeforeClassMethods() {
     return beforeClassMethods;
   }
 
-  public Map<ITestClass, Set<Object>> getInvokedAfterClassMethods() {
+  public Map<ITestClass, Set<IInstanceInfo<?>>> getInvokedAfterClassMethods() {
     return afterClassMethods;
   }
 
   public void clear() {
-    for (Set<Object> instances : beforeClassMethods.values()) {
+    for (Set<IInstanceInfo<?>> instances : beforeClassMethods.values()) {
       instances.clear();
     }
-    for (Set<Object> instances : afterClassMethods.values()) {
+    for (Set<IInstanceInfo<?>> instances : afterClassMethods.values()) {
       instances.clear();
     }
     beforeClassMethods.clear();

testng-core/src/main/java/org/testng/DependencyMap.java

@@ -103,49 +103,47 @@ private static boolean belongToDifferentClassHierarchy(
 
   private static boolean hasInstance(
       ITestNGMethod baseClassMethod, ITestNGMethod derivedClassMethod) {
-    Object baseInstance = baseClassMethod.getInstance();
-    Object derivedInstance = derivedClassMethod.getInstance();
+    IInstanceInfo<?> baseInstance = (IInstanceInfo<?>) baseClassMethod.getInstance();
+    IInstanceInfo<?> derivedInstance = (IInstanceInfo<?>) derivedClassMethod.getInstance();
     boolean result = derivedInstance != null || baseInstance != null;
-    boolean params =
-        baseClassMethod.getFactoryMethod().flatMap(IFactoryMethod::getParameters).isPresent();
-
-    if (result && params && RuntimeBehavior.enforceThreadAffinity()) {
-      return hasSameParameters(baseClassMethod, derivedClassMethod);
+    if (RuntimeBehavior.enforceThreadAffinity()
+        && result
+        && baseClassMethod instanceof ITestClassInstance
+        && derivedClassMethod instanceof ITestClassInstance) {
+      return hasSameParameters(
+          (ITestClassInstance<?>) baseClassMethod, (ITestClassInstance<?>) derivedClassMethod);
     }
     return result;
   }
 
-  private static boolean hasSameParameters(
-      ITestNGMethod baseClassMethod, ITestNGMethod derivedClassMethod) {
-    Optional<IFactoryMethod> first = baseClassMethod.getFactoryMethod();
-    Optional<IFactoryMethod> second = derivedClassMethod.getFactoryMethod();
-    if (first.isPresent() && second.isPresent()) {
-      Optional<Object[]> firstParams = first.get().getParameters();
-      Optional<Object[]> secondParams = second.get().getParameters();
-      if (firstParams.isPresent() && secondParams.isPresent()) {
-        return firstParams.get()[0].equals(secondParams.get()[0]);
-      }
-      return false;
-    }
-    return false;
-  }
-
   private static boolean isSameInstance(
       ITestNGMethod baseClassMethod, ITestNGMethod derivedClassMethod) {
-    Object baseInstance = baseClassMethod.getInstance();
-    Object derivedInstance = derivedClassMethod.getInstance();
+    IInstanceInfo<?> baseInstance = (IInstanceInfo<?>) baseClassMethod.getInstance();
+    IInstanceInfo<?> derivedInstance = (IInstanceInfo<?>) derivedClassMethod.getInstance();
     boolean nonNullInstances = derivedInstance != null && baseInstance != null;
     if (!nonNullInstances) {
       return false;
     }
-    if (null != baseClassMethod.getFactoryMethodParamsInfo()
-        && RuntimeBehavior.enforceThreadAffinity()) {
+    if (RuntimeBehavior.enforceThreadAffinity()
+        && baseClassMethod instanceof ITestClassInstance
+        && derivedClassMethod instanceof ITestClassInstance) {
       return baseInstance.getClass().isAssignableFrom(derivedInstance.getClass())
-          && hasSameParameters(baseClassMethod, derivedClassMethod);
+          && hasSameParameters(
+              (ITestClassInstance<?>) baseClassMethod, (ITestClassInstance<?>) derivedClassMethod);
     }
     return baseInstance.getClass().isAssignableFrom(derivedInstance.getClass());
   }
 
+  private static boolean hasSameParameters(
+      ITestClassInstance<?> baseClassMethod, ITestClassInstance<?> derivedClassMethod) {
+    Object[] firstParams = baseClassMethod.getFactoryMethod().getParameters();
+    Object[] secondParams = derivedClassMethod.getFactoryMethod().getParameters();
+    if (firstParams.length == 0 || secondParams.length == 0) {
+      return false;
+    }
+    return firstParams[0].equals(secondParams[0]);
+  }
+
   private static String constructMethodNameUsingTestClass(
       String currentMethodName, ITestNGMethod m) {
     int lastIndex = currentMethodName.lastIndexOf('.');

testng-core/src/main/java/org/testng/InstanceOrderingMethodInterceptor.java

@@ -15,10 +15,10 @@ public List<IMethodInstance> intercept(List<IMethodInstance> methods, ITestConte
 
   /** The default method interceptor which sorts methods by instances (i.e. by class). */
   private List<IMethodInstance> groupMethodsByInstance(List<IMethodInstance> methods) {
-    List<Object> instanceList = Lists.newArrayList();
+    List<IInstanceInfo<?>> instanceList = Lists.newArrayList();
     Map<Object, List<IMethodInstance>> map = Maps.newLinkedHashMap();
     for (IMethodInstance mi : methods) {
-      Object instance = mi.getInstance();
+      IInstanceInfo<?> instance = (IInstanceInfo<?>) mi.getInstance();
       if (!instanceList.contains(instance)) {
         instanceList.add(instance);
       }