Fixing Review comments

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

Hide file tree

Showing 7 changed files with 39 additions and 17 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(" +");
 
@@ -157,7 +158,9 @@ public Object getInstance() {
 
   @Override
   public UUID getInstanceId() {
-    return m_instance.getInstanceId();
+    return Optional.ofNullable(m_instance)
+        .map(IObject.IdentifiableObject::getInstanceId)
+        .orElse(null);
   }
 
   /** {@inheritDoc} */

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,19 @@
+package org.testng.internal;
+
+import java.util.UUID;
+
+public interface IInstanceIdentity {
+
+  /**
+   * @return - A <code>{@link UUID}</code> that represents a unique id which is associated with
+   *     every test class object.
+   */
+  UUID getInstanceId();
+
+  static Object getInstanceId(Object object) {
+    if (object instanceof IInstanceIdentity) {
+      return ((IInstanceIdentity) object).getInstanceId();
+    }
+    return object;
+  }
+}
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,12 +17,18 @@
  * 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();
 
+  private final UUID uuid;
+
   public WrappedTestNGMethod(ITestNGMethod testNGMethod) {
     this.testNGMethod = testNGMethod;
+    uuid =
+        (testNGMethod instanceof BaseTestMethod)
+            ? ((BaseTestMethod) testNGMethod).getInstanceId()
+            : UUID.randomUUID();
   }
 
   @Override
@@ -367,7 +373,7 @@ public String getQualifiedName() {
 
   @Override
   public UUID getInstanceId() {
-    return testNGMethod.getInstanceId();
+    return uuid;
   }
 
   @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
@@ -8,7 +8,6 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
-import java.util.UUID;
 import javax.annotation.Nonnull;
 import org.testng.ClassMethodMap;
 import org.testng.IClassListener;
@@ -123,7 +122,7 @@ && doesTaskHavePreRequisites()
 
     for (IMethodInstance testMethodInstance : m_methodInstances) {
       ITestNGMethod testMethod = testMethodInstance.getMethod();
-      UUID key = Objects.requireNonNull(testMethod.getInstanceId());
+      Object 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,8 @@ private static void record() {
     ITestResult itr = Reporter.getCurrentTestResult();
     ITestNGMethod itm = itr.getMethod();
     objectMap
-        .computeIfAbsent(itm.getInstanceId(), k -> ConcurrentHashMap.newKeySet())
+        .computeIfAbsent(
+            (UUID) 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,8 @@ private static void record() {
     ITestResult itr = Reporter.getCurrentTestResult();
     ITestNGMethod itm = itr.getMethod();
     objectMap
-        .computeIfAbsent(itm.getInstanceId(), k -> ConcurrentHashMap.newKeySet())
+        .computeIfAbsent(
+            (UUID) IInstanceIdentity.getInstanceId(itm), k -> ConcurrentHashMap.newKeySet())
         .add(itm.getInstance());
   }
 }