Fixing Review comments

Author: krmahadevan

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

@@ -3,7 +3,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.UUID;
 import java.util.concurrent.Callable;
 import org.testng.annotations.CustomAttribute;
 import org.testng.internal.ConstructorOrMethod;
@@ -297,12 +296,4 @@ default Class<?>[] getParameterTypes() {
   default boolean isIgnoreFailure() {
     return false;
   }
-
-  /**
-   * @return - A <code>{@link UUID}</code> that represents a unique id which is associated with
-   *     every test class object.
-   */
-  default UUID getInstanceId() {
-    return null;
-  }
 }

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

@@ -35,7 +35,8 @@
 import org.testng.xml.XmlTest;
 
 /** Superclass to represent both @Test and @Configuration methods. */
-public abstract class BaseTestMethod implements ITestNGMethod, IInvocationStatus {
+public abstract class BaseTestMethod
+    implements ITestNGMethod, IInvocationStatus, IInstanceIdentity {
 
   private static final Pattern SPACE_SEPARATOR_PATTERN = Pattern.compile(" +");
 

testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java

@@ -0,0 +1,30 @@
+package org.testng.internal;
+
+import java.util.Optional;
+import java.util.UUID;
+import org.testng.log4testng.Logger;
+
+public interface IInstanceIdentity {
+
+  /**
+   * @return - A <code>{@link UUID}</code> that represents a unique id which is associated with
+   *     every test class object.
+   */
+  UUID getInstanceId();
+
+  String fmt = "[Attention] %s does not implement %s. Generating an on the fly UUID.";
+  Logger logger = Logger.getLogger(IInstanceIdentity.class);
+
+  static UUID getInstanceId(Object object) {
+    if (object instanceof IInstanceIdentity) {
+      return ((IInstanceIdentity) object).getInstanceId();
+    }
+    if (logger.isDebugEnabled()) {
+      String className =
+          Optional.ofNullable(object).map(it -> it.getClass().getName()).orElse("[Unknown Class]");
+      String msg = String.format(fmt, className, IInstanceIdentity.class.getName());
+      Logger.getLogger(IInstanceIdentity.class).debug(msg);
+    }
+    return UUID.randomUUID();
+  }
+}

testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java

@@ -17,7 +17,7 @@
  * generates a unique hashcode that is different from the original {@link ITestNGMethod} instance
  * that it wraps.
  */
-public class WrappedTestNGMethod implements ITestNGMethod {
+public class WrappedTestNGMethod implements ITestNGMethod, IInstanceIdentity {
   private final ITestNGMethod testNGMethod;
   private final int multiplicationFactor = new Random().nextInt();
 
@@ -367,7 +367,7 @@ public String getQualifiedName() {
 
   @Override
   public UUID getInstanceId() {
-    return testNGMethod.getInstanceId();
+    return IInstanceIdentity.getInstanceId(testNGMethod);
   }
 
   @Override

testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java

@@ -123,7 +123,7 @@ && doesTaskHavePreRequisites()
 
     for (IMethodInstance testMethodInstance : m_methodInstances) {
       ITestNGMethod testMethod = testMethodInstance.getMethod();
-      UUID key = Objects.requireNonNull(testMethod.getInstanceId());
+      UUID key = Objects.requireNonNull(IInstanceIdentity.getInstanceId(testMethod));
       if (canInvokeBeforeClassMethods()) {
         try (KeyAwareAutoCloseableLock.AutoReleasable ignored = lock.lockForObject(key)) {
           invokeBeforeClassMethods(testMethod.getTestClass(), testMethodInstance);

testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java

@@ -15,6 +15,7 @@
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Factory;
 import org.testng.annotations.Test;
+import org.testng.internal.IInstanceIdentity;
 
 public class FactoryTestCase {
 
@@ -60,7 +61,7 @@ private static void record() {
     ITestResult itr = Reporter.getCurrentTestResult();
     ITestNGMethod itm = itr.getMethod();
     objectMap
-        .computeIfAbsent(itm.getInstanceId(), k -> ConcurrentHashMap.newKeySet())
+        .computeIfAbsent(IInstanceIdentity.getInstanceId(itm), k -> ConcurrentHashMap.newKeySet())
         .add(itm.getInstance());
   }
 }

testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java

@@ -13,6 +13,7 @@
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
+import org.testng.internal.IInstanceIdentity;
 
 public class SampleTestCase {
   public static Map<UUID, Set<Object>> objectMap = new ConcurrentHashMap<>();
@@ -61,7 +62,7 @@ private static void record() {
     ITestResult itr = Reporter.getCurrentTestResult();
     ITestNGMethod itm = itr.getMethod();
     objectMap
-        .computeIfAbsent(itm.getInstanceId(), k -> ConcurrentHashMap.newKeySet())
+        .computeIfAbsent(IInstanceIdentity.getInstanceId(itm), k -> ConcurrentHashMap.newKeySet())
         .add(itm.getInstance());
   }
 }