Support params in Factory in ITestResult

Closes #1041

CHANGES.txt

@@ -1,4 +1,5 @@
 Current
+Fixed: GITHUB-1041: Factory data-provider parameters not displayed in test-result (Krishnan Mahadevan)
 Fixed: GITHUB-1893: Streamline invocation of "init" method within TestResult to be private (Krishnan Mahadevan)
 Fixed: GITHUB-1892: Configurable InvokedMethodListener (Krishnan Mahadevan)
 Fixed: GITHUB-435: Apply <packages> at suite level to all tests (Siegmar Alber)

src/main/java/org/testng/ITestNGMethod.java

@@ -211,4 +211,8 @@ public interface ITestNGMethod extends Cloneable {
   default boolean isDataDriven() {
     return false;
   }
+
+  default Object getRawInstance() {
+    return null;
+  }
 }

src/main/java/org/testng/ITestResult.java

@@ -68,6 +68,12 @@ public interface ITestResult extends IAttributes, Comparable<ITestResult> {
   /** The instance on which this method was run. */
   Object getInstance();
 
+  /**
+   * @return - A parameter array that was passed to a factory method (or) an empty object array
+   * otherwise.
+   */
+  Object[] getFactoryParameters();
+
   /**
    * If this result's related instance implements ITest or use @Test(testName=...), returns its test
    * name, otherwise returns null.

src/main/java/org/testng/TestClass.java

@@ -78,6 +78,7 @@ private void initTestClassesAndInstances() {
     //
     Object[] instances = getInstances(true);
     for (Object instance : instances) {
+      instance = IParameterInfo.embeddedInstance(instance);
       if (instance instanceof ITest) {
         testName = ((ITest) instance).getTestName();
         break;
@@ -107,7 +108,8 @@ private void initMethods() {
     ITestNGMethod[] methods = testMethodFinder.getTestMethods(m_testClass, xmlTest);
     m_testMethods = createTestMethods(methods);
 
-    for (Object instance : iClass.getInstances(false)) {
+    for (Object eachInstance : iClass.getInstances(false)) {
+      Object instance = IParameterInfo.embeddedInstance(eachInstance);
       m_beforeSuiteMethods =
           ConfigurationMethod.createSuiteConfigurationMethods(
               testMethodFinder.getBeforeSuiteMethods(m_testClass),

src/main/java/org/testng/internal/BaseTestMethod.java

@@ -131,7 +131,7 @@ public String getMethodName() {
 
   @Override
   public Object getInstance() {
-    return m_instance;
+    return IParameterInfo.embeddedInstance(m_instance);
   }
 
   /** {@inheritDoc} */
@@ -319,7 +319,7 @@ public boolean equals(Object obj) {
             ? other.m_testClass == null
             : other.m_testClass != null
                 && m_testClass.getRealClass().equals(other.m_testClass.getRealClass())
-                && m_instance == other.getInstance();
+                && getInstance() == other.getInstance();
 
     return isEqual && getConstructorOrMethod().equals(other.getConstructorOrMethod());
   }
@@ -456,7 +456,8 @@ private String computeSignature() {
         .append("[pri:")
         .append(getPriority())
         .append(", instance:")
-        .append(m_instance)
+        .append(getInstance())
+        .append(instanceParameters())
         .append("]");
 
     return result.toString();
@@ -466,6 +467,14 @@ public String getSimpleName() {
     return m_method.getDeclaringClass().getSimpleName() + "." + m_method.getName();
   }
 
+  private String instanceParameters() {
+    Object instance = getRawInstance();
+    if (instance instanceof IParameterInfo) {
+      return ", instance params:" + Arrays.toString(((IParameterInfo) instance).getParameters());
+    }
+    return "";
+  }
+
   protected String getSignature() {
     if (m_signature == null) {
       m_signature = computeSignature();
@@ -715,4 +724,8 @@ public Map<String, String> findMethodParameters(XmlTest test) {
   public String getQualifiedName() {
     return getRealClass().getName() + "." + getMethodName();
   }
+
+  public Object getRawInstance() {
+    return m_instance;
+  }
 }

src/main/java/org/testng/internal/ClassImpl.java

@@ -197,7 +197,7 @@ public Object[] getInstances(boolean create) {
     int m_instanceCount = m_instances.size();
     m_instanceHashCodes = new long[m_instanceCount];
     for (int i = 0; i < m_instanceCount; i++) {
-      m_instanceHashCodes[i] = m_instances.get(i).hashCode();
+      m_instanceHashCodes[i] = computeHashCode(m_instances.get(i));
     }
     return result;
   }
@@ -211,4 +211,8 @@ public String toString() {
   public void addInstance(Object instance) {
     m_instances.add(instance);
   }
+
+  private static int computeHashCode(Object instance) {
+    return IParameterInfo.embeddedInstance(instance).hashCode();
+  }
 }

src/main/java/org/testng/internal/FactoryMethod.java

@@ -8,6 +8,7 @@
 import java.util.Map;
 import java.util.Set;
 
+import java.util.stream.Collectors;
 import org.testng.IDataProviderListener;
 import org.testng.IInstanceInfo;
 import org.testng.IObjectFactory;
@@ -127,8 +128,8 @@ private static String[] getAllGroups(
     return groups.toArray(new String[0]);
   }
 
-  public Object[] invoke() {
-    List<Object> result = Lists.newArrayList();
+  public IParameterInfo[] invoke() {
+    List<IParameterInfo> result = Lists.newArrayList();
 
     Map<String, String> allParameterNames = Maps.newHashMap();
     Parameters.MethodParameters methodParameters =
@@ -166,12 +167,14 @@ public Object[] invoke() {
         if (com.getMethod() != null) {
           Object[] testInstances = (Object[]) com.getMethod().invoke(m_instance, parameters);
           if (indices == null || indices.isEmpty()) {
-            result.addAll(Arrays.asList(testInstances));
+            result.addAll(Arrays.stream(testInstances).map(instance ->
+              new ParameterInfo(instance, parameters)
+            ).collect(Collectors.toList()));
           } else {
             for (Integer index : indices) {
               int i = index - position;
               if (i >= 0 && i < testInstances.length) {
-                result.add(testInstances[i]);
+                result.add(new ParameterInfo(testInstances[i], parameters));
               }
             }
           }
@@ -188,7 +191,7 @@ public Object[] invoke() {
               throw new IllegalStateException(
                   "Unsupported ITestObjectFactory " + objectFactory.getClass());
             }
-            result.add(instance);
+            result.add(new ParameterInfo(instance,  parameters));
           }
           position++;
         }
@@ -204,7 +207,7 @@ public Object[] invoke() {
           t);
     }
 
-    return result.toArray(new Object[0]);
+    return result.toArray(new IParameterInfo[0]);
   }
 
   @Override

src/main/java/org/testng/internal/IParameterInfo.java

@@ -0,0 +1,25 @@
+package org.testng.internal;
+
+/**
+ * Represents the ability to retrieve the parameters associated with a factory method.
+ */
+public interface IParameterInfo {
+
+  /**
+   * @return - The actual instance associated with a factory method
+   */
+  Object getInstance();
+
+  /**
+   * @return - The parameters associated with the factory method as an array.
+   */
+  Object[] getParameters();
+
+  static Object embeddedInstance(Object original) {
+    if (original instanceof IParameterInfo) {
+      return ((IParameterInfo) original).getInstance();
+    }
+    return original;
+  }
+
+}

src/main/java/org/testng/internal/ParameterInfo.java

@@ -0,0 +1,21 @@
+package org.testng.internal;
+
+public class ParameterInfo implements IParameterInfo {
+  private Object instance;
+  private Object[] parameters;
+
+  public ParameterInfo(Object instance, Object[] parameters) {
+    this.instance = instance;
+    this.parameters = parameters;
+  }
+
+  @Override
+  public Object getInstance() {
+    return instance;
+  }
+
+  @Override
+  public Object[] getParameters() {
+    return parameters;
+  }
+}

src/main/java/org/testng/internal/TestNGClassFinder.java

@@ -34,7 +34,7 @@ public class TestNGClassFinder extends BaseClassFinder {
 
   private static final String PREFIX = "[TestNGClassFinder]";
   private final ITestContext m_testContext;
-  private final Map<Class<?>, List<Object>> m_instanceMap = Maps.newHashMap();
+  private final Map<Class<?>, List<Object>>  m_instanceMap = Maps.newHashMap();
   private final Map<Class<? extends IDataProviderListener>, IDataProviderListener>
       m_dataProviderListeners;
   private final ITestObjectFactory objectFactory;
@@ -183,13 +183,14 @@ private ClassInfoMap processFactory(IClass ic, ConstructorOrMethod factoryMethod
             "The factory " + fm + " returned a null instance" + "at index " + i);
       }
       Class<?> oneMoreClass;
-      if (IInstanceInfo.class.isAssignableFrom(o.getClass())) {
-        IInstanceInfo<?> ii = (IInstanceInfo) o;
+      Object objToInspect = IParameterInfo.embeddedInstance(o);
+      if (IInstanceInfo.class.isAssignableFrom(objToInspect.getClass())) {
+        IInstanceInfo<?> ii = (IInstanceInfo) objToInspect;
         addInstance(ii);
         oneMoreClass = ii.getInstanceClass();
       } else {
         addInstance(o);
-        oneMoreClass = o.getClass();
+        oneMoreClass = objToInspect.getClass();
       }
       if (!classExists(oneMoreClass)) {
         moreClasses.addClass(oneMoreClass);
@@ -315,7 +316,7 @@ private <T> void addInstance(IInstanceInfo<T> ii) {
   }
 
   private void addInstance(Object o) {
-    addInstance(o.getClass(), o);
+    addInstance(IParameterInfo.embeddedInstance(o).getClass(), o);
   }
 
   // Class<S> should be replaced by Class<? extends T> but java doesn't fail as expected

src/main/java/org/testng/internal/TestResult.java

@@ -281,7 +281,16 @@ public void setParameters(Object[] parameters) {
 
   @Override
   public Object getInstance() {
-    return this.m_method.getInstance();
+    return IParameterInfo.embeddedInstance(this.m_method.getInstance());
+  }
+
+  @Override
+  public Object[] getFactoryParameters() {
+    Object instance = this.m_method.getRawInstance();
+    if (instance instanceof IParameterInfo) {
+      return ((IParameterInfo) instance).getParameters();
+    }
+    return new Object[0];
   }
 
   @Override

src/main/java/org/testng/reporters/EmailableReporter2.java

@@ -436,22 +436,10 @@ private void writeScenario(int scenarioIndex, String label, ITestResult result)
     // Write test parameters (if any)
     Object[] parameters = result.getParameters();
     int parameterCount = (parameters == null ? 0 : parameters.length);
-    if (parameterCount > 0) {
-      writer.print("<tr class=\"param\">");
-      for (int i = 1; i <= parameterCount; i++) {
-        writer.print("<th>Parameter #");
-        writer.print(i);
-        writer.print("</th>");
-      }
-      writer.print("</tr><tr class=\"param stripe\">");
-      for (Object parameter : parameters) {
-        writer.print("<td>");
-        writer.print(Utils.escapeHtml(Utils.toString(parameter)));
-        writer.print("</td>");
-      }
-      writer.print("</tr>");
-      hasRows = true;
-    }
+    hasRows = dumpParametersInfo("Factory Parameter", result.getFactoryParameters());
+    parameters = result.getParameters();
+    parameterCount = (parameters == null ? 0 : parameters.length);
+    hasRows = dumpParametersInfo("Parameter", result.getParameters());
 
     // Write reporter messages (if any)
     List<String> reporterMessages = Reporter.getOutput(result);
@@ -506,6 +494,27 @@ private void writeScenario(int scenarioIndex, String label, ITestResult result)
     writer.println("<p class=\"totop\"><a href=\"#summary\">back to summary</a></p>");
   }
 
+  private boolean dumpParametersInfo(String prefix, Object[] parameters) {
+    int parameterCount = (parameters == null ? 0 : parameters.length);
+    if (parameterCount == 0) {
+      return false;
+    }
+    writer.print("<tr class=\"param\">");
+    for (int i = 1; i <= parameterCount; i++) {
+      writer.print(String.format("<th>%s #", prefix));
+      writer.print(i);
+      writer.print("</th>");
+    }
+    writer.print("</tr><tr class=\"param stripe\">");
+    for (Object parameter : parameters) {
+      writer.print("<td>");
+      writer.print(Utils.escapeHtml(Utils.toString(parameter)));
+      writer.print("</td>");
+    }
+    writer.print("</tr>");
+    return true;
+  }
+
   protected void writeReporterMessages(List<String> reporterMessages) {
     writer.print("<div class=\"messages\">");
     Iterator<String> iterator = reporterMessages.iterator();

src/test/java/test/factory/issue1041/FactoryAnnotatedConstructorExample.java

@@ -0,0 +1,55 @@
+package test.factory.issue1041;
+
+import java.util.List;
+import java.util.Objects;
+import org.testng.Assert;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Factory;
+import org.testng.annotations.Test;
+import org.testng.collections.Lists;
+
+public class FactoryAnnotatedConstructorExample {
+  static List<FactoryAnnotatedConstructorExample> objects = Lists.newArrayList();
+
+  private int data;
+
+  @Factory(dataProvider = "dp")
+  public FactoryAnnotatedConstructorExample(int data) {
+    this.data = data;
+    addInstance(this);
+  }
+
+  private static void addInstance(FactoryAnnotatedConstructorExample instance) {
+    objects.add(instance);
+  }
+
+  @DataProvider(name = "dp")
+  public static Object[][] getData() {
+    return new Object[][]{
+        {1},
+        {2}
+    };
+  }
+
+  @Test
+  public void testMethod() {
+    Assert.assertTrue(data > 0);
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
+    FactoryAnnotatedConstructorExample that = (FactoryAnnotatedConstructorExample) o;
+    return data == that.data;
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(data);
+  }
+}