Support ordering of listeners

Closes testng-team#2916

CHANGES.txt

@@ -1,5 +1,6 @@
 Current (7.10.0)
-Fixed: GITHUB-3033: Moved ant support under own repository https://github.com/testng-team/testng-ant
+New:   GITHUB-2916: Allow users to define ordering for TestNG listeners (Krishnan Mahadevan)
+Fixed: GITHUB-3033: Moved ant support under own repository https://github.com/testng-team/testng-ant (Julien Herr)
 Fixed: GITHUB-3064: TestResult lost if failure creating RetryAnalyzer (Krishnan Mahadevan)
 Fixed: GITHUB-3048: ConcurrentModificationException when injecting values (Krishnan Mahadevan)
 Fixed: GITHUB-3050: Race condition when creating Guice Modules (Krishnan Mahadevan)

testng-core/src/main/java/org/testng/CommandLineArgs.java

@@ -54,6 +54,14 @@ public class CommandLineArgs {
               + " implementing ITestListener or ISuiteListener")
   public String listener;
 
+  public static final String LISTENER_COMPARATOR = "-listenercomparator";
+
+  @Parameter(
+      names = LISTENER_COMPARATOR,
+      description =
+          "An implementation of ListenerComparator that will be used by TestNG to determine order of execution for listeners")
+  public String listenerComparator;
+
   public static final String METHOD_SELECTORS = "-methodselectors";
 
   @Parameter(

testng-core/src/main/java/org/testng/DataProviderHolder.java

@@ -2,9 +2,13 @@
 
 import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import org.testng.collections.Lists;
 import org.testng.collections.Maps;
 import org.testng.collections.Sets;
+import org.testng.internal.IConfiguration;
 
 /**
  * A holder class that is aimed at acting as a container for holding various different aspects of a
@@ -15,12 +19,32 @@ public class DataProviderHolder {
   private final Map<Class<?>, IDataProviderListener> listeners = Maps.newConcurrentMap();
   private final Collection<IDataProviderInterceptor> interceptors = Sets.newHashSet();
 
+  private final IConfiguration configuration;
+
+  public DataProviderHolder(IConfiguration configuration) {
+    this.configuration = configuration;
+  }
+
   public Collection<IDataProviderListener> getListeners() {
-    return Collections.unmodifiableCollection(listeners.values());
+    List<IDataProviderListener> original = Lists.newArrayList(listeners.values());
+    ListenerComparator comparator = getConfiguration().getListenerComparator();
+    if (comparator != null) {
+      original.sort(comparator::compare);
+    }
+    return Collections.unmodifiableCollection(original);
+  }
+
+  public IConfiguration getConfiguration() {
+    return Objects.requireNonNull(configuration);
   }
 
   public Collection<IDataProviderInterceptor> getInterceptors() {
-    return Collections.unmodifiableCollection(interceptors);
+    List<IDataProviderInterceptor> original = Lists.newArrayList(interceptors);
+    ListenerComparator comparator = getConfiguration().getListenerComparator();
+    if (comparator != null) {
+      original.sort(comparator::compare);
+    }
+    return Collections.unmodifiableCollection(original);
   }
 
   public void addListeners(Collection<IDataProviderListener> listeners) {

testng-core/src/main/java/org/testng/ListenerComparator.java

@@ -0,0 +1,26 @@
+package org.testng;
+
+/**
+ * Listener interface that can be used to determine listener execution order. This interface will
+ * NOT be used to determine execution order for {@link IReporter} implementations.
+ *
+ * <p>An implementation can be plugged into TestNG either via:
+ *
+ * <ol>
+ *   <li>{@link TestNG#setListenerComparator(ListenerComparator)} if you are using the {@link
+ *       TestNG} APIs.
+ *   <li>Via the configuration parameter <code>-listenercomparator</code> if you are using a build
+ *       tool
+ * </ol>
+ */
+@FunctionalInterface
+public interface ListenerComparator {
+
+  /**
+   * @param l1 - First {@link ITestNGListener} object to be compared.
+   * @param l2 - Second {@link ITestNGListener} object to be compared.
+   * @return - a negative integer, zero, or a positive integer as the first argument is less than,
+   *     equal to, or greater than the second.
+   */
+  int compare(ITestNGListener l1, ITestNGListener l2);
+}

testng-core/src/main/java/org/testng/SuiteRunner.java

@@ -41,14 +41,14 @@ public class SuiteRunner implements ISuite, ISuiteRunnerListener {
       Maps.newLinkedHashMap();
 
   private String outputDir;
-  private XmlSuite xmlSuite;
+  private final XmlSuite xmlSuite;
   private Injector parentInjector;
 
   private final List<ITestListener> testListeners = Lists.newArrayList();
   private final Map<Class<? extends IClassListener>, IClassListener> classListeners =
       Maps.newLinkedHashMap();
-  private ITestRunnerFactory tmpRunnerFactory;
-  private final DataProviderHolder holder = new DataProviderHolder();
+  private final ITestRunnerFactory tmpRunnerFactory;
+  private final DataProviderHolder holder;
 
   private boolean useDefaultListeners = true;
 
@@ -57,19 +57,19 @@ public class SuiteRunner implements ISuite, ISuiteRunnerListener {
 
   // The configuration
   // Note: adjust test.multiplelisteners.SimpleReporter#generateReport test if renaming the field
-  private IConfiguration configuration;
+  private final IConfiguration configuration;
 
   private ITestObjectFactory objectFactory;
   private Boolean skipFailedInvocationCounts = Boolean.FALSE;
   private final List<IReporter> reporters = Lists.newArrayList();
 
-  private Map<Class<? extends IInvokedMethodListener>, IInvokedMethodListener>
+  private final Map<Class<? extends IInvokedMethodListener>, IInvokedMethodListener>
       invokedMethodListeners;
 
   private final SuiteRunState suiteState = new SuiteRunState();
   private final IAttributes attributes = new Attributes();
   private final Set<IExecutionVisualiser> visualisers = Sets.newHashSet();
-  private ITestListener exitCodeListener;
+  private final ITestListener exitCodeListener;
 
   public SuiteRunner(
       IConfiguration configuration,
@@ -97,37 +97,11 @@ public SuiteRunner(
         null /* invoked method listeners */,
         new TestListenersContainer() /* test listeners */,
         null /* class listeners */,
-        new DataProviderHolder(),
+        new DataProviderHolder(configuration),
         comparator);
   }
 
   protected SuiteRunner(
-      IConfiguration configuration,
-      XmlSuite suite,
-      String outputDir,
-      ITestRunnerFactory runnerFactory,
-      boolean useDefaultListeners,
-      List<IMethodInterceptor> methodInterceptors,
-      Collection<IInvokedMethodListener> invokedMethodListeners,
-      TestListenersContainer container,
-      Collection<IClassListener> classListeners,
-      DataProviderHolder holder,
-      Comparator<ITestNGMethod> comparator) {
-    init(
-        configuration,
-        suite,
-        outputDir,
-        runnerFactory,
-        useDefaultListeners,
-        methodInterceptors,
-        invokedMethodListeners,
-        container,
-        classListeners,
-        holder,
-        comparator);
-  }
-
-  private void init(
       IConfiguration configuration,
       XmlSuite suite,
       String outputDir,
@@ -137,12 +111,12 @@ private void init(
       Collection<IInvokedMethodListener> invokedMethodListener,
       TestListenersContainer container,
       Collection<IClassListener> classListeners,
-      DataProviderHolder attribs,
+      DataProviderHolder holder,
       Comparator<ITestNGMethod> comparator) {
     if (comparator == null) {
       throw new IllegalArgumentException("comparator must not be null");
     }
-    this.holder.merge(attribs);
+    this.holder = holder;
     this.configuration = configuration;
     this.xmlSuite = suite;
     this.useDefaultListeners = useDefaultListeners;
@@ -262,14 +236,17 @@ public ITestListener getExitCodeListener() {
   }
 
   private void invokeListeners(boolean start) {
+    List<ISuiteListener> original = Lists.newArrayList(listeners.values());
+    ListenerComparator comparator = this.configuration.getListenerComparator();
+    if (comparator != null) {
+      original.sort(comparator::compare);
+    }
     if (start) {
-      for (ISuiteListener sl :
-          ListenerOrderDeterminer.order(Lists.newArrayList(listeners.values()))) {
+      for (ISuiteListener sl : ListenerOrderDeterminer.order(original)) {
         sl.onStart(this);
       }
     } else {
-      List<ISuiteListener> suiteListenersReversed =
-          ListenerOrderDeterminer.reversedOrder(listeners.values());
+      List<ISuiteListener> suiteListenersReversed = ListenerOrderDeterminer.reversedOrder(original);
       for (ISuiteListener sl : suiteListenersReversed) {
         sl.onFinish(this);
       }
@@ -298,7 +275,8 @@ private ITestRunnerFactory buildRunnerFactory(Comparator<ITestNGMethod> comparat
               this);
     } else {
       factory =
-          new ProxyTestRunnerFactory(testListeners.toArray(new ITestListener[0]), tmpRunnerFactory);
+          new ProxyTestRunnerFactory(
+              testListeners.toArray(new ITestListener[0]), tmpRunnerFactory, configuration);
     }
 
     return factory;
@@ -627,7 +605,7 @@ public TestRunner newTestRunner(
         Collection<IInvokedMethodListener> listeners,
         List<IClassListener> classListeners,
         Map<Class<? extends IDataProviderListener>, IDataProviderListener> dataProviderListeners) {
-      DataProviderHolder holder = new DataProviderHolder();
+      DataProviderHolder holder = new DataProviderHolder(this.configuration);
       holder.addListeners(dataProviderListeners.values());
       return newTestRunner(suite, test, listeners, classListeners, holder);
     }
@@ -684,9 +662,13 @@ private static class ProxyTestRunnerFactory implements ITestRunnerFactory {
     private final ITestListener[] failureGenerators;
     private final ITestRunnerFactory target;
 
-    public ProxyTestRunnerFactory(ITestListener[] failureListeners, ITestRunnerFactory target) {
+    private final IConfiguration configuration;
+
+    public ProxyTestRunnerFactory(
+        ITestListener[] failureListeners, ITestRunnerFactory target, IConfiguration configuration) {
       failureGenerators = failureListeners;
       this.target = target;
+      this.configuration = configuration;
     }
 
     @Override
@@ -705,7 +687,7 @@ public TestRunner newTestRunner(
         Collection<IInvokedMethodListener> listeners,
         List<IClassListener> classListeners,
         Map<Class<? extends IDataProviderListener>, IDataProviderListener> dataProviderListeners) {
-      DataProviderHolder holder = new DataProviderHolder();
+      DataProviderHolder holder = new DataProviderHolder(configuration);
       holder.addListeners(dataProviderListeners.values());
       return newTestRunner(suite, test, listeners, classListeners, holder);
     }

testng-core/src/main/java/org/testng/TestNG.java

@@ -273,6 +273,14 @@ public void setUseDefaultListeners(boolean useDefaultListeners) {
     m_useDefaultListeners = useDefaultListeners;
   }
 
+  public void setListenerComparator(ListenerComparator listenerComparator) {
+    this.m_configuration.setListenerComparator(listenerComparator);
+  }
+
+  public ListenerComparator getListenerComparator() {
+    return m_configuration.getListenerComparator();
+  }
+
   /**
    * Sets a jar containing a testng.xml file.
    *
@@ -1130,14 +1138,21 @@ protected List<ISuite> runSuites() {
   }
 
   private void runSuiteAlterationListeners() {
-    for (IAlterSuiteListener l : m_alterSuiteListeners.values()) {
+    List<IAlterSuiteListener> original = Lists.newArrayList(m_alterSuiteListeners.values());
+    Optional.ofNullable(m_configuration.getListenerComparator())
+        .ifPresent(it -> original.sort(it::compare));
+
+    for (IAlterSuiteListener l : original) {
       l.alter(m_suites);
     }
   }
 
   private void runExecutionListeners(boolean start) {
-    List<IExecutionListener> executionListeners =
-        ListenerOrderDeterminer.order(m_configuration.getExecutionListeners());
+    List<IExecutionListener> original = m_configuration.getExecutionListeners();
+    Optional.ofNullable(m_configuration.getListenerComparator())
+        .ifPresent(it -> original.sort(it::compare));
+
+    List<IExecutionListener> executionListeners = ListenerOrderDeterminer.order(original);
     if (start) {
       for (IExecutionListener l : executionListeners) {
         l.onExecutionStart();
@@ -1367,7 +1382,7 @@ private void createSuiteRunners(SuiteRunnerMap suiteRunnerMap /* OUT */, XmlSuit
 
   /** Creates a suite runner and configures its initial state */
   private SuiteRunner createSuiteRunner(XmlSuite xmlSuite) {
-    DataProviderHolder holder = new DataProviderHolder();
+    DataProviderHolder holder = new DataProviderHolder(m_configuration);
     holder.addListeners(m_dataProviderListeners.values());
     holder.addInterceptors(m_dataProviderInterceptors.values());
     TestListenersContainer container =
@@ -1484,6 +1499,13 @@ protected void configure(CommandLineArgs cla) {
 
     Optional.ofNullable(cla.generateResultsPerSuite).ifPresent(this::setGenerateResultsPerSuite);
 
+    Optional.ofNullable(cla.listenerComparator)
+        .map(ClassHelper::forName)
+        .filter(ListenerComparator.class::isAssignableFrom)
+        .map(it -> m_objectFactory.newInstance(it))
+        .map(it -> (ListenerComparator) it)
+        .ifPresent(this::setListenerComparator);
+
     if (cla.verbose != null) {
       setVerbose(cla.verbose);
     }