Minor refactoring around RunOrder enum

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)

Author: krmahadevan

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() {

testng-core-api/src/main/java/org/testng/internal/Systematiser.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);

testng-core/src/main/java/org/testng/TestNG.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<IInvokedMethodListener> invokedMethodListeners,
       List<IClassListener> classListeners,
       ISuiteRunnerListener suiteRunner) {
-    this.comparator = Systematiser.getComparator();
+    this.comparator = MethodSorting.basedOn();
     this.holder = new DataProviderHolder(configuration);
     init(
         configuration,

testng-core/src/main/java/org/testng/TestRunner.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 - <code>true</code> 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);
+  }
 }

testng-core/src/main/java/org/testng/internal/IInstanceIdentity.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<ITestNGMethod> comparator();
+}

testng-core/src/main/java/org/testng/internal/IOrderMethods.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<ITestNGMethod> {
+  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<ITestNGMethod> 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<ITestNGMethod> 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);
+  }
+}

testng-core/src/main/java/org/testng/internal/MethodSorting.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<ITestNGMethod> methodsDependedUpon =
-            MethodHelper.getMethodsDependedUpon(
-                current, allTestMethods, Systematiser.getComparator());
+            MethodHelper.getMethodsDependedUpon(current, allTestMethods, MethodSorting.basedOn());
 
         for (ITestNGMethod m : methodsDependedUpon) {
           if (m.isTest()) {

testng-core/src/main/java/org/testng/reporters/FailedReporter.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());