Streamline native injection w.r.t cloning

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)
 New: Added a method in Assertion class to allow downstream TestNG consumers to override the error message (Ryan Laseter)
 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)

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;
+  }
+
+}