From 2e5d8bf1da64d00c0ae54bed893a420aeec2c481 Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Sat, 30 Mar 2024 18:20:11 +0530 Subject: [PATCH] Minor refactoring around RunOrder enum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renamed and re-organised some internal classes The run order based on instances was using Hash codes to determine execution order. Now that we have Unique identity aware test class object containers, we should be first trying to use that and if it’s not available only then fall Back to using object hash codes (This is for instance aware comparators) --- .../org/testng/internal/RuntimeBehavior.java | 2 +- .../org/testng/internal/Systematiser.java | 123 ------------------ .../src/main/java/org/testng/TestNG.java | 4 +- .../src/main/java/org/testng/TestRunner.java | 4 +- .../testng/internal/IInstanceIdentity.java | 11 ++ .../org/testng/internal/IOrderMethods.java | 15 +++ .../org/testng/internal/MethodSorting.java | 85 ++++++++++++ .../org/testng/reporters/FailedReporter.java | 5 +- testng-core/src/test/java/test/BaseTest.java | 6 +- 9 files changed, 121 insertions(+), 134 deletions(-) delete mode 100644 testng-core-api/src/main/java/org/testng/internal/Systematiser.java create mode 100644 testng-core/src/main/java/org/testng/internal/IOrderMethods.java create mode 100644 testng-core/src/main/java/org/testng/internal/MethodSorting.java diff --git a/testng-core-api/src/main/java/org/testng/internal/RuntimeBehavior.java b/testng-core-api/src/main/java/org/testng/internal/RuntimeBehavior.java index 74f9e581fe..33a26e5c26 100644 --- a/testng-core-api/src/main/java/org/testng/internal/RuntimeBehavior.java +++ b/testng-core-api/src/main/java/org/testng/internal/RuntimeBehavior.java @@ -93,7 +93,7 @@ public static boolean useStrictParameterMatching() { } public static String orderMethodsBasedOn() { - return System.getProperty("testng.order", Systematiser.Order.INSTANCES.getValue()); + return System.getProperty("testng.order"); } public static String getTestClasspath() { diff --git a/testng-core-api/src/main/java/org/testng/internal/Systematiser.java b/testng-core-api/src/main/java/org/testng/internal/Systematiser.java deleted file mode 100644 index 183a562821..0000000000 --- a/testng-core-api/src/main/java/org/testng/internal/Systematiser.java +++ /dev/null @@ -1,123 +0,0 @@ -package org.testng.internal; - -import java.util.Arrays; -import java.util.Comparator; -import java.util.Objects; -import org.testng.ITestNGMethod; - -/** Helps determine how should {@link ITestNGMethod} be ordered by TestNG. */ -public final class Systematiser { - - private static final Comparator COMPARE_INSTANCES = - Comparator.comparingInt(ITestNGMethod::getPriority) - .thenComparing(method -> method.getRealClass().getName()) - .thenComparing(ITestNGMethod::getMethodName) - .thenComparing(Object::toString) - .thenComparing( - method -> { - IParameterInfo paramsInfo = method.getFactoryMethodParamsInfo(); - // TODO: avoid toString in parameter comparison - return paramsInfo == null ? "" : Arrays.toString(paramsInfo.getParameters()); - }) - .thenComparingInt(method -> Objects.hashCode(method.getInstance())); - - private Systematiser() { - // Utility class. Defeat instantiation. - } - - /** - * @return - A {@link Comparator} that helps TestNG sort {@link ITestNGMethod}s in a specific - * order. Currently the following two orders are supported :
- *
    - *
  1. Based on the name of methods - *
  2. Based on the toString() implementation that resides within the test - * class - *
- */ - public static Comparator getComparator() { - Comparator comparator; - String text = RuntimeBehavior.orderMethodsBasedOn(); - - Order order = Order.parse(text); - switch (order) { - case METHOD_NAMES: - comparator = - new Comparator() { - @Override - public int compare(ITestNGMethod o1, ITestNGMethod o2) { - String n1 = o1.getMethodName(); - String n2 = o2.getMethodName(); - return n1.compareTo(n2); - } - - @Override - public String toString() { - return "Method_Names"; - } - }; - break; - - case NONE: - // Disables sorting by providing a dummy comparator which always regards two elements as - // equal. - comparator = - new Comparator() { - @Override - public int compare(ITestNGMethod o1, ITestNGMethod o2) { - return 0; - } - - @Override - public String toString() { - return "No_Sorting"; - } - }; - break; - - case INSTANCES: - default: - comparator = - new Comparator() { - @Override - public int compare(ITestNGMethod o1, ITestNGMethod o2) { - return COMPARE_INSTANCES.compare(o1, o2); - } - - @Override - public String toString() { - return "Instance_Names"; - } - }; - } - - return comparator; - } - - enum Order { - METHOD_NAMES("methods"), - INSTANCES("instances"), - NONE("none"); - - Order(String value) { - this.value = value; - } - - private final String value; - - public String getValue() { - return value; - } - - public static Order parse(String value) { - if (value == null || value.trim().isEmpty()) { - return INSTANCES; - } - for (Order each : values()) { - if (each.getValue().equalsIgnoreCase(value)) { - return each; - } - } - return INSTANCES; - } - } -} diff --git a/testng-core/src/main/java/org/testng/TestNG.java b/testng-core/src/main/java/org/testng/TestNG.java index b3938e0db8..0a72fcd573 100644 --- a/testng-core/src/main/java/org/testng/TestNG.java +++ b/testng-core/src/main/java/org/testng/TestNG.java @@ -33,11 +33,11 @@ import org.testng.internal.ExitCode; import org.testng.internal.IConfiguration; import org.testng.internal.ListenerOrderDeterminer; +import org.testng.internal.MethodSorting; import org.testng.internal.ObjectBag; import org.testng.internal.OverrideProcessor; import org.testng.internal.ReporterConfig; import org.testng.internal.RuntimeBehavior; -import org.testng.internal.Systematiser; import org.testng.internal.Utils; import org.testng.internal.Version; import org.testng.internal.annotations.DefaultAnnotationTransformer; @@ -1362,7 +1362,7 @@ private SuiteRunner createSuiteRunner(XmlSuite xmlSuite) { container, m_classListeners.values(), holder, - Systematiser.getComparator()); + MethodSorting.basedOn()); for (ISuiteListener isl : m_suiteListeners.values()) { result.addListener(isl); diff --git a/testng-core/src/main/java/org/testng/TestRunner.java b/testng-core/src/main/java/org/testng/TestRunner.java index 3a3538eea4..8f8085c076 100644 --- a/testng-core/src/main/java/org/testng/TestRunner.java +++ b/testng-core/src/main/java/org/testng/TestRunner.java @@ -39,10 +39,10 @@ import org.testng.internal.ListenerOrderDeterminer; import org.testng.internal.MethodGroupsHelper; import org.testng.internal.MethodHelper; +import org.testng.internal.MethodSorting; import org.testng.internal.ResultMap; import org.testng.internal.RunInfo; import org.testng.internal.RuntimeBehavior; -import org.testng.internal.Systematiser; import org.testng.internal.TestListenerHelper; import org.testng.internal.TestMethodComparator; import org.testng.internal.TestMethodContainer; @@ -217,7 +217,7 @@ public TestRunner( Collection invokedMethodListeners, List classListeners, ISuiteRunnerListener suiteRunner) { - this.comparator = Systematiser.getComparator(); + this.comparator = MethodSorting.basedOn(); this.holder = new DataProviderHolder(configuration); init( configuration, diff --git a/testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java b/testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java index 75f1c4aa10..9b9efe4942 100644 --- a/testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java +++ b/testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java @@ -1,5 +1,7 @@ package org.testng.internal; +import java.util.Arrays; +import java.util.Objects; import java.util.UUID; public interface IInstanceIdentity { @@ -16,4 +18,13 @@ static Object getInstanceId(Object object) { } return object; } + + /** + * @param objects - The objects to inspect + * @return - true if all the objects passed are of type {@link IInstanceIdentity} + */ + static boolean isIdentityAware(Object... objects) { + return Arrays.stream(Objects.requireNonNull(objects)) + .allMatch(it -> it instanceof IInstanceIdentity); + } } diff --git a/testng-core/src/main/java/org/testng/internal/IOrderMethods.java b/testng-core/src/main/java/org/testng/internal/IOrderMethods.java new file mode 100644 index 0000000000..e7ea314876 --- /dev/null +++ b/testng-core/src/main/java/org/testng/internal/IOrderMethods.java @@ -0,0 +1,15 @@ +package org.testng.internal; + +import java.util.Comparator; +import org.testng.ITestNGMethod; + +/** + * Helps produce a {@link Comparator} that can be used to determine order of execution for a bunch + * of {@link ITestNGMethod} methods. + */ +@FunctionalInterface +public interface IOrderMethods { + + /** @return - The {@link Comparator} to be used. */ + Comparator comparator(); +} diff --git a/testng-core/src/main/java/org/testng/internal/MethodSorting.java b/testng-core/src/main/java/org/testng/internal/MethodSorting.java new file mode 100644 index 0000000000..c2e455922e --- /dev/null +++ b/testng-core/src/main/java/org/testng/internal/MethodSorting.java @@ -0,0 +1,85 @@ +package org.testng.internal; + +import java.util.Arrays; +import java.util.Comparator; +import java.util.Objects; +import java.util.Optional; +import java.util.UUID; +import org.testng.ITestNGMethod; + +public enum MethodSorting implements Comparator { + METHOD_NAMES("methods") { + @Override + public int compare(ITestNGMethod o1, ITestNGMethod o2) { + String n1 = o1.getMethodName(); + String n2 = o2.getMethodName(); + return n1.compareTo(n2); + } + + @Override + public String toString() { + return "Method_Names"; + } + }, + INSTANCES("instances") { + @Override + public int compare(ITestNGMethod o1, ITestNGMethod o2) { + Comparator comparator = + Comparator.comparingInt(ITestNGMethod::getPriority) + .thenComparing(method -> method.getRealClass().getName()) + .thenComparing(ITestNGMethod::getMethodName) + .thenComparing(Object::toString) + .thenComparing( + method -> + Optional.ofNullable(method.getFactoryMethodParamsInfo()) + .map(it -> Arrays.toString(it.getParameters())) + .orElse("")) + .thenComparing(this::objectEquality); + return comparator.compare(o1, o2); + } + + private int objectEquality(ITestNGMethod a, ITestNGMethod b) { + Object one = IInstanceIdentity.getInstanceId(a.getInstance()); + Object two = IInstanceIdentity.getInstanceId(b.getInstance()); + if (IInstanceIdentity.isIdentityAware(one, two)) { + return ((UUID) one).compareTo((UUID) two); + } + return Integer.compare(Objects.hashCode(one), Objects.hashCode(two)); + } + + @Override + public String toString() { + return "Instance_Names"; + } + }, + NONE("none") { + @Override + public int compare(ITestNGMethod o1, ITestNGMethod o2) { + return 0; + } + + @Override + public String toString() { + return "No_Sorting"; + } + }; + + MethodSorting(String value) { + this.value = value; + } + + private final String value; + + public static Comparator basedOn() { + String text = RuntimeBehavior.orderMethodsBasedOn(); + return MethodSorting.parse(text); + } + + private static MethodSorting parse(String input) { + String text = Optional.ofNullable(input).orElse(""); + return Arrays.stream(values()) + .filter(it -> it.value.equalsIgnoreCase(text)) + .findFirst() + .orElse(INSTANCES); + } +} diff --git a/testng-core/src/main/java/org/testng/reporters/FailedReporter.java b/testng-core/src/main/java/org/testng/reporters/FailedReporter.java index 16b5e4aff4..068af38082 100644 --- a/testng-core/src/main/java/org/testng/reporters/FailedReporter.java +++ b/testng-core/src/main/java/org/testng/reporters/FailedReporter.java @@ -24,8 +24,8 @@ import org.testng.internal.ConstructorOrMethod; import org.testng.internal.LiteWeightTestNGMethod; import org.testng.internal.MethodHelper; +import org.testng.internal.MethodSorting; import org.testng.internal.RuntimeBehavior; -import org.testng.internal.Systematiser; import org.testng.internal.Utils; import org.testng.xml.XmlClass; import org.testng.xml.XmlInclude; @@ -111,8 +111,7 @@ private void generateXmlTest( } methodsToReRun.add(current); List methodsDependedUpon = - MethodHelper.getMethodsDependedUpon( - current, allTestMethods, Systematiser.getComparator()); + MethodHelper.getMethodsDependedUpon(current, allTestMethods, MethodSorting.basedOn()); for (ITestNGMethod m : methodsDependedUpon) { if (m.isTest()) { diff --git a/testng-core/src/test/java/test/BaseTest.java b/testng-core/src/test/java/test/BaseTest.java index 04d9212421..0700962c66 100644 --- a/testng-core/src/test/java/test/BaseTest.java +++ b/testng-core/src/test/java/test/BaseTest.java @@ -23,7 +23,7 @@ import org.testng.collections.Maps; import org.testng.internal.Configuration; import org.testng.internal.IConfiguration; -import org.testng.internal.Systematiser; +import org.testng.internal.MethodSorting; import org.testng.reporters.JUnitXMLReporter; import org.testng.reporters.TestHTMLReporter; import org.testng.xml.XmlClass; @@ -168,7 +168,7 @@ protected void run() { m_suite, m_outputDirectory, m_testRunnerFactory, - Systematiser.getComparator()); + MethodSorting.basedOn()); suiteRunner.run(); } @@ -346,7 +346,7 @@ public TestRunner newTestRunner( false, listeners, classListeners, - Systematiser.getComparator(), + MethodSorting.basedOn(), m_baseTest.suiteRunner); testRunner.addListener(new TestHTMLReporter());