Fixing Review comments

testng-team · Mar 14, 2024 · afc7b6c · afc7b6c
1 parent a3ec3a3
commit afc7b6c
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 15 deletions.
diff --git a/testng-core-api/src/main/java/org/testng/ITestNGMethod.java b/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;
-  }
 }
diff --git a/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java b/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java
@@ -35,7 +35,8 @@
 import org.testng.xml.XmlTest;
 
 /** Superclass to represent both &#64;Test and &#64;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(" +");
 

diff --git a/testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java b/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.debug(msg);
+    }
+    return UUID.randomUUID();
+  }
+}
diff --git a/testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java b/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

diff --git a/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java b/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);

diff --git a/testng-core/src/test/java/test/factory/issue3079/FactoryTestCase.java b/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());
   }
 }
diff --git a/testng-core/src/test/java/test/factory/issue3079/SampleTestCase.java b/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());
   }
 }