Fixing review comments

krmahadevan · Apr 27, 2024 · 97c1d4b · 97c1d4b
1 parent fda8744
commit 97c1d4b
Show file tree

Hide file tree

Showing 9 changed files with 81 additions and 23 deletions.
diff --git a/testng-core-api/src/main/java/org/testng/IFactoryMethod.java b/testng-core-api/src/main/java/org/testng/IFactoryMethod.java
@@ -0,0 +1,12 @@
+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();
+}
diff --git a/testng-core-api/src/main/java/org/testng/ITestClassInstance.java b/testng-core-api/src/main/java/org/testng/ITestClassInstance.java
@@ -24,9 +24,6 @@ public interface ITestClassInstance {
    */
   int getInvocationIndex();
 
-  /** @return - The parameters associated with the factory method as an array. */
-  Object[] getParameters();
-
   static Object embeddedInstance(Object original) {
     if (original instanceof ITestClassInstance) {
       return ((ITestClassInstance) original).getInstance();

diff --git a/testng-core-api/src/main/java/org/testng/ITestNGMethod.java b/testng-core-api/src/main/java/org/testng/ITestNGMethod.java
@@ -2,6 +2,7 @@
 
 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;
@@ -269,6 +270,14 @@ 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.

diff --git a/testng-core-api/src/main/java/org/testng/internal/IParameterInfo.java b/testng-core-api/src/main/java/org/testng/internal/IParameterInfo.java
@@ -1,11 +1,20 @@
 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 extends ITestClassInstance {
+  /**
+   * @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.
+   */
+  @Deprecated
+  Object[] getParameters();
+}
diff --git a/testng-core/src/main/java/org/testng/DependencyMap.java b/testng-core/src/main/java/org/testng/DependencyMap.java
@@ -107,8 +107,7 @@ private static boolean hasInstance(
     Object derivedInstance = derivedClassMethod.getInstance();
     boolean result = derivedInstance != null || baseInstance != null;
     boolean params =
-        null != baseClassMethod.getFactoryMethodParamsInfo()
-            && null != derivedClassMethod.getFactoryMethodParamsInfo().getParameters();
+        baseClassMethod.getFactoryMethod().flatMap(IFactoryMethod::getParameters).isPresent();
 
     if (result && params && RuntimeBehavior.enforceThreadAffinity()) {
       return hasSameParameters(baseClassMethod, derivedClassMethod);
@@ -118,10 +117,17 @@ private static boolean hasInstance(
 
   private static boolean hasSameParameters(
       ITestNGMethod baseClassMethod, ITestNGMethod derivedClassMethod) {
-    return baseClassMethod
-        .getFactoryMethodParamsInfo()
-        .getParameters()[0]
-        .equals(derivedClassMethod.getFactoryMethodParamsInfo().getParameters()[0]);
+    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(

diff --git a/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java b/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java
@@ -17,6 +17,7 @@
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import org.testng.IClass;
+import org.testng.IFactoryMethod;
 import org.testng.IRetryAnalyzer;
 import org.testng.ITestClass;
 import org.testng.ITestClassInstance;
@@ -104,6 +105,10 @@ public BaseTestMethod(
     m_instance = instance;
   }
 
+  protected final IObject.IdentifiableObject identifiableObject() {
+    return m_instance;
+  }
+
   /** {@inheritDoc} */
   @Override
   public boolean isAlwaysRun() {
@@ -300,6 +305,19 @@ public void setTimeOut(long timeOut) {
     m_timeOut = timeOut;
   }
 
+  @Override
+  public Optional<IFactoryMethod> getFactoryMethod() {
+    IObject.IdentifiableObject identifiable = identifiableObject();
+    if (identifiable == null) {
+      return Optional.empty();
+    }
+    Object instance = identifiableObject().getInstance();
+    if (instance instanceof ParameterInfo) {
+      return Optional.of(() -> Optional.of(((ParameterInfo) instance).getParameters()));
+    }
+    return ITestNGMethod.super.getFactoryMethod();
+  }
+
   /**
    * {@inheritDoc}
    *
@@ -539,11 +557,10 @@ public String getSimpleName() {
   }
 
   private String instanceParameters() {
-    IParameterInfo instance = getFactoryMethodParamsInfo();
-    if (instance != null) {
-      return ", instance params:" + Arrays.toString(instance.getParameters());
-    }
-    return "";
+    return getFactoryMethod()
+        .flatMap(IFactoryMethod::getParameters)
+        .map(it -> ", instance params:" + Arrays.toString(it))
+        .orElse("");
   }
 
   protected String getSignature() {

diff --git a/testng-core/src/main/java/org/testng/internal/MethodSorting.java b/testng-core/src/main/java/org/testng/internal/MethodSorting.java
@@ -5,6 +5,7 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.UUID;
+import org.testng.IFactoryMethod;
 import org.testng.ITestNGMethod;
 
 public enum MethodSorting implements Comparator<ITestNGMethod> {
@@ -31,8 +32,10 @@ public int compare(ITestNGMethod o1, ITestNGMethod o2) {
               .thenComparing(Object::toString)
               .thenComparing(
                   method ->
-                      Optional.ofNullable(method.getFactoryMethodParamsInfo())
-                          .map(it -> Arrays.toString(it.getParameters()))
+                      method
+                          .getFactoryMethod()
+                          .flatMap(IFactoryMethod::getParameters)
+                          .map(Arrays::toString)
                           .orElse(""))
               .thenComparing(this::objectEquality);
       return comparator.compare(o1, o2);

diff --git a/testng-core/src/test/java/test/configuration/issue2426/MyMethodListener.java b/testng-core/src/test/java/test/configuration/issue2426/MyMethodListener.java
@@ -3,6 +3,7 @@
 import java.util.HashMap;
 import java.util.Map;
 import org.testng.IConfigurationListener;
+import org.testng.IFactoryMethod;
 import org.testng.ITestResult;
 import org.testng.annotations.*;
 
@@ -12,7 +13,11 @@ public class MyMethodListener implements IConfigurationListener {
 
   @Override
   public void onConfigurationSuccess(ITestResult tr) {
-    Object[] values = tr.getMethod().getFactoryMethodParamsInfo().getParameters();
+    Object[] values =
+        tr.getMethod()
+            .getFactoryMethod()
+            .flatMap(IFactoryMethod::getParameters)
+            .orElse(new Object[0]);
     if (tr.getMethod().isBeforeSuiteConfiguration()) {
       contents.put(BeforeSuite.class, values);
     }

diff --git a/testng-runner-api/src/main/java/org/testng/internal/TestResult.java b/testng-runner-api/src/main/java/org/testng/internal/TestResult.java
@@ -12,6 +12,7 @@
 import javax.annotation.Nonnull;
 import org.testng.IAttributes;
 import org.testng.IClass;
+import org.testng.IFactoryMethod;
 import org.testng.ITest;
 import org.testng.ITestClassInstance;
 import org.testng.ITestContext;
@@ -304,11 +305,10 @@ public Object getInstance() {
 
   @Override
   public Object[] getFactoryParameters() {
-    IParameterInfo instance = this.m_method.getFactoryMethodParamsInfo();
-    if (instance != null) {
-      return instance.getParameters();
-    }
-    return new Object[0];
+    return this.m_method
+        .getFactoryMethod()
+        .flatMap(IFactoryMethod::getParameters)
+        .orElse(new Object[0]);
   }
 
   @Override