From 97c1d4b1b05ab8341cf56c2c8fbe97a44a6e2339 Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Sat, 27 Apr 2024 11:38:54 +0530 Subject: [PATCH] Fixing review comments --- .../main/java/org/testng/IFactoryMethod.java | 12 +++++++++ .../java/org/testng/ITestClassInstance.java | 3 --- .../main/java/org/testng/ITestNGMethod.java | 9 +++++++ .../org/testng/internal/IParameterInfo.java | 11 +++++++- .../main/java/org/testng/DependencyMap.java | 18 ++++++++----- .../org/testng/internal/BaseTestMethod.java | 27 +++++++++++++++---- .../org/testng/internal/MethodSorting.java | 7 +++-- .../issue2426/MyMethodListener.java | 7 ++++- .../java/org/testng/internal/TestResult.java | 10 +++---- 9 files changed, 81 insertions(+), 23 deletions(-) create mode 100644 testng-core-api/src/main/java/org/testng/IFactoryMethod.java diff --git a/testng-core-api/src/main/java/org/testng/IFactoryMethod.java b/testng-core-api/src/main/java/org/testng/IFactoryMethod.java new file mode 100644 index 0000000000..6b11bef48c --- /dev/null +++ b/testng-core-api/src/main/java/org/testng/IFactoryMethod.java @@ -0,0 +1,12 @@ +package org.testng; + +import java.util.Optional; + +/** Represents a factory method */ +public interface IFactoryMethod { + + /** + * @return - Returns parameters associated with a factory method wrapped within a {@link Optional} + */ + Optional getParameters(); +} diff --git a/testng-core-api/src/main/java/org/testng/ITestClassInstance.java b/testng-core-api/src/main/java/org/testng/ITestClassInstance.java index 9972ee3e7a..0de0e9f348 100644 --- a/testng-core-api/src/main/java/org/testng/ITestClassInstance.java +++ b/testng-core-api/src/main/java/org/testng/ITestClassInstance.java @@ -24,9 +24,6 @@ public interface ITestClassInstance { */ int getInvocationIndex(); - /** @return - The parameters associated with the factory method as an array. */ - Object[] getParameters(); - static Object embeddedInstance(Object original) { if (original instanceof ITestClassInstance) { return ((ITestClassInstance) original).getInstance(); diff --git a/testng-core-api/src/main/java/org/testng/ITestNGMethod.java b/testng-core-api/src/main/java/org/testng/ITestNGMethod.java index bff0385de1..add31af349 100644 --- a/testng-core-api/src/main/java/org/testng/ITestNGMethod.java +++ b/testng-core-api/src/main/java/org/testng/ITestNGMethod.java @@ -2,6 +2,7 @@ import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.concurrent.Callable; import org.testng.annotations.CustomAttribute; @@ -269,6 +270,14 @@ default IParameterInfo getFactoryMethodParamsInfo() { return null; } + /** + * @return - A {@link IFactoryMethod} implementation that contains attributes associated with a + * factory method, wrapped within an {@link Optional}. + */ + default Optional getFactoryMethod() { + return Optional.empty(); + } + /** * @return - An array of {@link CustomAttribute} that represents the custom attributes associated * with a test. diff --git a/testng-core-api/src/main/java/org/testng/internal/IParameterInfo.java b/testng-core-api/src/main/java/org/testng/internal/IParameterInfo.java index 0a6f49c418..99ed97cf5f 100644 --- a/testng-core-api/src/main/java/org/testng/internal/IParameterInfo.java +++ b/testng-core-api/src/main/java/org/testng/internal/IParameterInfo.java @@ -1,6 +1,7 @@ package org.testng.internal; import org.testng.ITestClassInstance; +import org.testng.ITestNGMethod; /** * Represents the ability to retrieve the parameters associated with a factory method. @@ -8,4 +9,12 @@ * @deprecated - This interface stands deprecated as of TestNG 7.11.0. */ @Deprecated -public interface IParameterInfo extends ITestClassInstance {} +public interface IParameterInfo extends ITestClassInstance { + /** + * @return - The parameters associated with the factory method as an array. + * @deprecated - This method stands deprecated as of TestNG 7.11.0 Please use {@link + * ITestNGMethod#getFactoryMethod()} to retrieve the parameters. + */ + @Deprecated + Object[] getParameters(); +} diff --git a/testng-core/src/main/java/org/testng/DependencyMap.java b/testng-core/src/main/java/org/testng/DependencyMap.java index 7d834ff7d2..d998f73d54 100644 --- a/testng-core/src/main/java/org/testng/DependencyMap.java +++ b/testng-core/src/main/java/org/testng/DependencyMap.java @@ -107,8 +107,7 @@ private static boolean hasInstance( Object derivedInstance = derivedClassMethod.getInstance(); boolean result = derivedInstance != null || baseInstance != null; boolean params = - null != baseClassMethod.getFactoryMethodParamsInfo() - && null != derivedClassMethod.getFactoryMethodParamsInfo().getParameters(); + baseClassMethod.getFactoryMethod().flatMap(IFactoryMethod::getParameters).isPresent(); if (result && params && RuntimeBehavior.enforceThreadAffinity()) { return hasSameParameters(baseClassMethod, derivedClassMethod); @@ -118,10 +117,17 @@ private static boolean hasInstance( private static boolean hasSameParameters( ITestNGMethod baseClassMethod, ITestNGMethod derivedClassMethod) { - return baseClassMethod - .getFactoryMethodParamsInfo() - .getParameters()[0] - .equals(derivedClassMethod.getFactoryMethodParamsInfo().getParameters()[0]); + Optional first = baseClassMethod.getFactoryMethod(); + Optional second = derivedClassMethod.getFactoryMethod(); + if (first.isPresent() && second.isPresent()) { + Optional firstParams = first.get().getParameters(); + Optional secondParams = second.get().getParameters(); + if (firstParams.isPresent() && secondParams.isPresent()) { + return firstParams.get()[0].equals(secondParams.get()[0]); + } + return false; + } + return false; } private static boolean isSameInstance( diff --git a/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java b/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java index 6a342c8e72..28765c8350 100644 --- a/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java +++ b/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java @@ -17,6 +17,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; import org.testng.IClass; +import org.testng.IFactoryMethod; import org.testng.IRetryAnalyzer; import org.testng.ITestClass; import org.testng.ITestClassInstance; @@ -104,6 +105,10 @@ public BaseTestMethod( m_instance = instance; } + protected final IObject.IdentifiableObject identifiableObject() { + return m_instance; + } + /** {@inheritDoc} */ @Override public boolean isAlwaysRun() { @@ -300,6 +305,19 @@ public void setTimeOut(long timeOut) { m_timeOut = timeOut; } + @Override + public Optional getFactoryMethod() { + IObject.IdentifiableObject identifiable = identifiableObject(); + if (identifiable == null) { + return Optional.empty(); + } + Object instance = identifiableObject().getInstance(); + if (instance instanceof ParameterInfo) { + return Optional.of(() -> Optional.of(((ParameterInfo) instance).getParameters())); + } + return ITestNGMethod.super.getFactoryMethod(); + } + /** * {@inheritDoc} * @@ -539,11 +557,10 @@ public String getSimpleName() { } private String instanceParameters() { - IParameterInfo instance = getFactoryMethodParamsInfo(); - if (instance != null) { - return ", instance params:" + Arrays.toString(instance.getParameters()); - } - return ""; + return getFactoryMethod() + .flatMap(IFactoryMethod::getParameters) + .map(it -> ", instance params:" + Arrays.toString(it)) + .orElse(""); } protected String getSignature() { diff --git a/testng-core/src/main/java/org/testng/internal/MethodSorting.java b/testng-core/src/main/java/org/testng/internal/MethodSorting.java index c2e455922e..b7e6e2fe13 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodSorting.java +++ b/testng-core/src/main/java/org/testng/internal/MethodSorting.java @@ -5,6 +5,7 @@ import java.util.Objects; import java.util.Optional; import java.util.UUID; +import org.testng.IFactoryMethod; import org.testng.ITestNGMethod; public enum MethodSorting implements Comparator { @@ -31,8 +32,10 @@ public int compare(ITestNGMethod o1, ITestNGMethod o2) { .thenComparing(Object::toString) .thenComparing( method -> - Optional.ofNullable(method.getFactoryMethodParamsInfo()) - .map(it -> Arrays.toString(it.getParameters())) + method + .getFactoryMethod() + .flatMap(IFactoryMethod::getParameters) + .map(Arrays::toString) .orElse("")) .thenComparing(this::objectEquality); return comparator.compare(o1, o2); diff --git a/testng-core/src/test/java/test/configuration/issue2426/MyMethodListener.java b/testng-core/src/test/java/test/configuration/issue2426/MyMethodListener.java index 8d32e31518..a123f8405c 100644 --- a/testng-core/src/test/java/test/configuration/issue2426/MyMethodListener.java +++ b/testng-core/src/test/java/test/configuration/issue2426/MyMethodListener.java @@ -3,6 +3,7 @@ import java.util.HashMap; import java.util.Map; import org.testng.IConfigurationListener; +import org.testng.IFactoryMethod; import org.testng.ITestResult; import org.testng.annotations.*; @@ -12,7 +13,11 @@ public class MyMethodListener implements IConfigurationListener { @Override public void onConfigurationSuccess(ITestResult tr) { - Object[] values = tr.getMethod().getFactoryMethodParamsInfo().getParameters(); + Object[] values = + tr.getMethod() + .getFactoryMethod() + .flatMap(IFactoryMethod::getParameters) + .orElse(new Object[0]); if (tr.getMethod().isBeforeSuiteConfiguration()) { contents.put(BeforeSuite.class, values); } diff --git a/testng-runner-api/src/main/java/org/testng/internal/TestResult.java b/testng-runner-api/src/main/java/org/testng/internal/TestResult.java index 89b5020aac..0817dd11eb 100644 --- a/testng-runner-api/src/main/java/org/testng/internal/TestResult.java +++ b/testng-runner-api/src/main/java/org/testng/internal/TestResult.java @@ -12,6 +12,7 @@ import javax.annotation.Nonnull; import org.testng.IAttributes; import org.testng.IClass; +import org.testng.IFactoryMethod; import org.testng.ITest; import org.testng.ITestClassInstance; import org.testng.ITestContext; @@ -304,11 +305,10 @@ public Object getInstance() { @Override public Object[] getFactoryParameters() { - IParameterInfo instance = this.m_method.getFactoryMethodParamsInfo(); - if (instance != null) { - return instance.getParameters(); - } - return new Object[0]; + return this.m_method + .getFactoryMethod() + .flatMap(IFactoryMethod::getParameters) + .orElse(new Object[0]); } @Override