Streamline native injection w.r.t cloning

Closes #1994

Currently as part of supporting native injection
TestNG resorts to invoking the “clone()” method 
on the parameter being natively injected.

This becomes a problem because XmlTest is the only
internal TestNG object that implements the Cloneable
interface wherein it resorts to something like a deep
copy and adds the cloned instance to the XmlSuite.

This causes a lot of phony XmlTest objects to be 
added up to the suite and causes issues as detailed
in the bug.

Fixed this by introducing a marker interface to 
indicate to TestNG to skip cloning and use the object
as is. Annotated “XmlTest” with this annotation 
so that we dont clone xmltest but instead use it 
as is.

Yes this will expose the XmlTest object, but its 
already exposed to the end user via various different
means. So I guess we are ok here.

Author: krmahadevan

CHANGES.txt

@@ -1,4 +1,5 @@
 Current
+Fixed: GITHUB-1994: Prevent duplication of XmlTest objects when its used as a parameter for Native Injection (Krishnan Mahadevan)
 Fixed: GITHUB-165: @AfterGroups is not executed when group member fails or is skipped (Krishnan Mahadevan)
 Fixed: GITHUB-118: @BeforeGroups only called if group is specified explicitly (Krishnan Mahadevan)
 Fixed: GITHUB-182: Inherited test methods do not get expected group behavior (Krishnan Mahadevan)

src/main/java/org/testng/annotations/SkipCloning.java

@@ -0,0 +1,16 @@
+package org.testng.annotations;
+
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/**
+ * Marker interface which when used on a type will ensure that TestNG does not clone the object but
+ * instead uses it as is when TestNG resorts to dependency injection.
+ */
+@Retention(java.lang.annotation.RetentionPolicy.RUNTIME)
+@Target({ElementType.TYPE})
+public @interface SkipCloning {
+
+}

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

@@ -12,6 +12,7 @@
 import org.testng.Reporter;
 import org.testng.TestNGException;
 import org.testng.TestRunner;
+import org.testng.annotations.SkipCloning;
 import org.testng.collections.Lists;
 import org.testng.collections.Objects;
 
@@ -276,7 +277,7 @@ public void setParameters(Object[] parameters) {
     m_parameters = new Object[parameters.length];
     for (int i = 0; i < parameters.length; i++) {
       // Copy parameter if possible because user may change it later
-      if (parameters[i] instanceof Cloneable) {
+      if (canAttemptCloning(parameters[i]) ) {
         try {
           Method clone = parameters[i].getClass().getDeclaredMethod("clone");
           m_parameters[i] = clone.invoke(parameters[i]);
@@ -292,6 +293,17 @@ public void setParameters(Object[] parameters) {
     }
   }
 
+  private static boolean canAttemptCloning(Object parameter) {
+    if (parameter == null) {
+      return false;
+    }
+    SkipCloning skipCloning = parameter.getClass().getAnnotation(SkipCloning.class);
+    if (skipCloning != null) {
+      return false;
+    }
+    return parameter instanceof Cloneable;
+  }
+
   @Override
   public Object getInstance() {
     return IParameterInfo.embeddedInstance(this.m_method.getInstance());

src/main/java/org/testng/xml/XmlTest.java

@@ -9,13 +9,15 @@
 
 import org.testng.TestNG;
 import org.testng.TestNGException;
+import org.testng.annotations.SkipCloning;
 import org.testng.collections.Lists;
 import org.testng.collections.Maps;
 import org.testng.xml.dom.ParentSetter;
 
 import static org.testng.xml.XmlSuite.ParallelMode.skipDeprecatedValues;
 
 /** This class describes the tag <test> in testng.xml. */
+@SkipCloning
 public class XmlTest implements Cloneable {
 
   public static final int DEFAULT_TIMEOUT_MS = Integer.MAX_VALUE;
@@ -75,6 +77,31 @@ private void init(XmlSuite suite, int index) {
   // For YAML
   public XmlTest() {}
 
+  /**
+   * This constructor acts as a copy constructor. Please note that it does not automatically
+   * associate the copied {@link XmlTest} object with the current {@link XmlSuite} object and
+   * requires it to be done explicitly.
+   *
+   * @param xmlTest - The {@link XmlTest} object to copy from.
+   */
+  public XmlTest(XmlTest xmlTest) {
+    XmlTest result = new XmlTest();
+    result.setName(getName());
+    result.setIncludedGroups(getIncludedGroups());
+    result.setExcludedGroups(getExcludedGroups());
+    result.setJUnit(isJUnit());
+    result.setParallel(getParallel());
+    result.setVerbose(getVerbose());
+    result.setParameters(getLocalParameters());
+    result.setXmlPackages(getXmlPackages());
+    result.setTimeOut(getTimeOut());
+
+    Map<String, List<String>> metagroups = getMetaGroups();
+    for (Map.Entry<String, List<String>> group : metagroups.entrySet()) {
+      result.addMetaGroup(group.getKey(), group.getValue());
+    }
+  }
+
   public void setXmlPackages(List<XmlPackage> packages) {
     m_xmlPackages = Lists.newArrayList(packages);
   }
@@ -482,8 +509,11 @@ public String toXml(String indent) {
    * <p>The &lt;classes&gt; sub element is ignored for the moment.
    *
    * @return a clone of the current XmlTest
+   * @Deprecated - This method stands deprecated as of TestNG 7.0.0. Please make use of
+   * {@link XmlTest#XmlTest(XmlTest)} instead.
    */
   @Override
+  @Deprecated
   public Object clone() {
     XmlTest result = new XmlTest(getSuite());
 

src/test/java/test/parameters/ParameterTest.java

@@ -1,6 +1,5 @@
 package test.parameters;
 
-import org.testng.ITestNGListener;
 import org.testng.ITestResult;
 import org.testng.TestListenerAdapter;
 import org.testng.TestNG;
@@ -15,6 +14,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import test.parameters.issue1994.TestclassSample;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -130,4 +130,12 @@ public void testNativeInjectionAndParamsInjection() {
     testng.run();
     assertThat(listener.getPassedTests().isEmpty()).isFalse();
   }
+
+  @Test(description = "GITHUB-1994")
+  public void testToEnsureNativeInjectionDoesnotResortToCloning() {
+    XmlSuite xmlsuite = createXmlSuite("suite", "test", TestclassSample.class);
+    TestNG testng = create(xmlsuite);
+    testng.run();
+    assertThat(TestclassSample.count).isEqualTo(1);
+  }
 }

src/test/java/test/parameters/issue1994/TestclassSample.java

@@ -0,0 +1,32 @@
+package test.parameters.issue1994;
+
+import org.testng.ISuite;
+import org.testng.ISuiteListener;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Listeners;
+import org.testng.annotations.Test;
+import org.testng.xml.XmlTest;
+
+@Listeners(TestclassSample.class)
+public class TestclassSample implements ISuiteListener {
+
+  public static int count = 0;
+
+  @BeforeClass
+  public void beforeClass(XmlTest xmlTest) {
+  }
+
+  @Test
+  public void testMethod() {
+  }
+
+  @Override
+  public void onFinish(ISuite suite) {
+    setCount(suite.getXmlSuite().getTests().size());
+  }
+
+  private static void setCount(int count) {
+    TestclassSample.count = count;
+  }
+
+}