Remove internal listeners from user listeners (#2872)

* Remove TestRunner.ConfigurationListener from user listeners
* Remove SuiteRunner from user listeners
* Formatting the code

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

@@ -204,7 +204,6 @@ public <T> T newInstance(Constructor<T> constructor, Object... parameters) {
         Optional.ofNullable(invokedMethodListener).orElse(Collections.emptyList())) {
       invokedMethodListeners.put(listener.getClass(), listener);
     }
-    invokedMethodListeners.put(getClass(), this);
 
     skipFailedInvocationCounts = suite.skipFailedInvocationCounts();
     if (null != testListeners) {
@@ -289,7 +288,8 @@ private ITestRunnerFactory buildRunnerFactory(Comparator<ITestNGMethod> comparat
               testListeners.toArray(new ITestListener[0]),
               useDefaultListeners,
               skipFailedInvocationCounts,
-              comparator);
+              comparator,
+              this);
     } else {
       factory =
           new ProxyTestRunnerFactory(testListeners.toArray(new ITestListener[0]), tmpRunnerFactory);
@@ -588,18 +588,21 @@ private static class DefaultTestRunnerFactory implements ITestRunnerFactory {
     private final boolean skipFailedInvocationCounts;
     private final IConfiguration configuration;
     private final Comparator<ITestNGMethod> comparator;
+    private final SuiteRunner suiteRunner;
 
     public DefaultTestRunnerFactory(
         IConfiguration configuration,
         ITestListener[] failureListeners,
         boolean useDefaultListeners,
         boolean skipFailedInvocationCounts,
-        Comparator<ITestNGMethod> comparator) {
+        Comparator<ITestNGMethod> comparator,
+        SuiteRunner suiteRunner) {
       this.configuration = configuration;
       this.failureGenerators = failureListeners;
       this.useDefaultListeners = useDefaultListeners;
       this.skipFailedInvocationCounts = skipFailedInvocationCounts;
       this.comparator = comparator;
+      this.suiteRunner = suiteRunner;
     }
 
     @Override
@@ -645,7 +648,8 @@ public TestRunner newTestRunner(
               listeners,
               classListeners,
               comparator,
-              holder);
+              holder,
+              suiteRunner);
 
       if (useDefaultListeners) {
         testRunner.addListener(new TestHTMLReporter());

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

@@ -173,7 +173,8 @@ protected TestRunner(
       Collection<IInvokedMethodListener> invokedMethodListeners,
       List<IClassListener> classListeners,
       Comparator<ITestNGMethod> comparator,
-      DataProviderHolder otherHolder) {
+      DataProviderHolder otherHolder,
+      SuiteRunner suiteRunner) {
     this.comparator = comparator;
     this.holder.merge(otherHolder);
     init(
@@ -184,7 +185,8 @@ protected TestRunner(
         finder,
         skipFailedInvocationCounts,
         invokedMethodListeners,
-        classListeners);
+        classListeners,
+        suiteRunner);
   }
 
   public TestRunner(
@@ -194,7 +196,8 @@ public TestRunner(
       boolean skipFailedInvocationCounts,
       Collection<IInvokedMethodListener> invokedMethodListeners,
       List<IClassListener> classListeners,
-      Comparator<ITestNGMethod> comparator) {
+      Comparator<ITestNGMethod> comparator,
+      SuiteRunner suiteRunner) {
     this.comparator = comparator;
     init(
         configuration,
@@ -204,7 +207,8 @@ public TestRunner(
         suite.getAnnotationFinder(),
         skipFailedInvocationCounts,
         invokedMethodListeners,
-        classListeners);
+        classListeners,
+        suiteRunner);
   }
 
   /* /!\ This constructor is used by testng-remote, any changes related to it please contact with testng-team. */
@@ -214,7 +218,8 @@ public TestRunner(
       XmlTest test,
       boolean skipFailedInvocationCounts,
       Collection<IInvokedMethodListener> invokedMethodListeners,
-      List<IClassListener> classListeners) {
+      List<IClassListener> classListeners,
+      SuiteRunner suiteRunner) {
     this.comparator = Systematiser.getComparator();
     init(
         configuration,
@@ -224,7 +229,8 @@ public TestRunner(
         suite.getAnnotationFinder(),
         skipFailedInvocationCounts,
         invokedMethodListeners,
-        classListeners);
+        classListeners,
+        suiteRunner);
   }
 
   private void init(
@@ -235,7 +241,8 @@ private void init(
       IAnnotationFinder annotationFinder,
       boolean skipFailedInvocationCounts,
       Collection<IInvokedMethodListener> invokedMethodListeners,
-      List<IClassListener> classListeners) {
+      List<IClassListener> classListeners,
+      SuiteRunner suiteRunner) {
     m_configuration = configuration;
     m_xmlTest = test;
     m_suite = suite;
@@ -277,7 +284,9 @@ private void init(
             skipFailedInvocationCounts,
             invokedMethodListeners,
             classListeners,
-            holder);
+            holder,
+            m_confListener,
+            suiteRunner);
 
     if (test.getParallel() != null) {
       log("Running the tests in '" + test.getName() + "' with parallel mode:" + test.getParallel());
@@ -337,7 +346,6 @@ private void init() {
     }
 
     initListeners();
-    addConfigurationListener(m_confListener);
     for (IConfigurationListener cl : m_configuration.getConfigurationListeners()) {
       addConfigurationListener(cl);
     }

testng-core/src/main/java/org/testng/internal/TestListenerHelper.java

@@ -22,7 +22,11 @@ private TestListenerHelper() {
   }
 
   public static void runPreConfigurationListeners(
-      ITestResult tr, ITestNGMethod tm, List<IConfigurationListener> listeners) {
+      ITestResult tr,
+      ITestNGMethod tm,
+      List<IConfigurationListener> listeners,
+      IConfigurationListener internal) {
+    internal.beforeConfiguration(tr);
     for (IConfigurationListener icl : listeners) {
       icl.beforeConfiguration(tr);
       try {
@@ -34,8 +38,12 @@ public static void runPreConfigurationListeners(
   }
 
   public static void runPostConfigurationListeners(
-      ITestResult tr, ITestNGMethod tm, List<IConfigurationListener> listeners) {
+      ITestResult tr,
+      ITestNGMethod tm,
+      List<IConfigurationListener> listeners,
+      IConfigurationListener internal) {
     List<IConfigurationListener> listenersreversed = Lists.newReversedArrayList(listeners);
+    listenersreversed.add(internal);
     for (IConfigurationListener icl : listenersreversed) {
       switch (tr.getStatus()) {
         case ITestResult.SKIP:

testng-core/src/main/java/org/testng/internal/invokers/BaseInvoker.java

@@ -10,6 +10,7 @@
 import org.testng.ITestResult;
 import org.testng.SkipException;
 import org.testng.SuiteRunState;
+import org.testng.SuiteRunner;
 import org.testng.collections.Lists;
 import org.testng.collections.Maps;
 import org.testng.internal.IConfiguration;
@@ -33,17 +34,21 @@ class BaseInvoker {
   // Instead we now would be using this special object.
   protected final Object NULL_OBJECT = new Object();
 
+  private final SuiteRunner suiteRunner;
+
   public BaseInvoker(
       ITestResultNotifier notifier,
       Collection<IInvokedMethodListener> invokedMethodListeners,
       ITestContext testContext,
       SuiteRunState suiteState,
-      IConfiguration configuration) {
+      IConfiguration configuration,
+      SuiteRunner suiteRunner) {
     this.m_notifier = notifier;
     this.m_invokedMethodListeners = invokedMethodListeners;
     this.m_testContext = testContext;
     this.m_suiteState = suiteState;
     this.m_configuration = configuration;
+    this.suiteRunner = suiteRunner;
   }
 
   protected IAnnotationFinder annotationFinder() {
@@ -67,6 +72,9 @@ protected void runInvokedMethodListeners(
         isAfterInvocation
             ? Lists.newReversedArrayList(m_invokedMethodListeners)
             : m_invokedMethodListeners;
+    if (!isAfterInvocation) {
+      suiteRunner.beforeInvocation(invokedMethod, testResult);
+    }
     for (IInvokedMethodListener currentListener : listeners) {
       try {
         invoker.invokeListener(currentListener, invokedMethod);
@@ -82,6 +90,9 @@ protected void runInvokedMethodListeners(
         testResult.setThrowable(e);
       }
     }
+    if (isAfterInvocation) {
+      suiteRunner.afterInvocation(invokedMethod, testResult);
+    }
   }
 
   private boolean noListenersPresent() {

testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java

@@ -14,12 +14,14 @@
 import org.testng.ConfigurationNotInvokedException;
 import org.testng.IClass;
 import org.testng.IConfigurable;
+import org.testng.IConfigurationListener;
 import org.testng.IInvokedMethodListener;
 import org.testng.ITestContext;
 import org.testng.ITestNGMethod;
 import org.testng.ITestResult;
 import org.testng.Reporter;
 import org.testng.SuiteRunState;
+import org.testng.SuiteRunner;
 import org.testng.TestNGException;
 import org.testng.annotations.IConfigurationAnnotation;
 import org.testng.collections.Maps;
@@ -54,16 +56,21 @@ class ConfigInvoker extends BaseInvoker implements IConfigInvoker {
   /** Group failures must be synced as the Invoker is accessed concurrently */
   private final Map<String, Boolean> m_beforegroupsFailures = Maps.newConcurrentMap();
 
+  private final IConfigurationListener internalConfigurationListener;
+
   public ConfigInvoker(
       ITestResultNotifier notifier,
       Collection<IInvokedMethodListener> invokedMethodListeners,
       ITestContext testContext,
       SuiteRunState suiteState,
-      IConfiguration configuration) {
-    super(notifier, invokedMethodListeners, testContext, suiteState, configuration);
+      IConfiguration configuration,
+      IConfigurationListener internalConfigurationListener,
+      SuiteRunner suiteRunner) {
+    super(notifier, invokedMethodListeners, testContext, suiteState, configuration, suiteRunner);
     this.m_continueOnFailedConfiguration =
         testContext.getSuite().getXmlSuite().getConfigFailurePolicy()
             == XmlSuite.FailurePolicy.CONTINUE;
+    this.internalConfigurationListener = internalConfigurationListener;
   }
 
   /**
@@ -403,10 +410,10 @@ private IConfigurable computeConfigurableInstance(
   private void runConfigurationListeners(ITestResult tr, ITestNGMethod tm, boolean before) {
     if (before) {
       TestListenerHelper.runPreConfigurationListeners(
-          tr, tm, m_notifier.getConfigurationListeners());
+          tr, tm, m_notifier.getConfigurationListeners(), internalConfigurationListener);
     } else {
       TestListenerHelper.runPostConfigurationListeners(
-          tr, tm, m_notifier.getConfigurationListeners());
+          tr, tm, m_notifier.getConfigurationListeners(), internalConfigurationListener);
     }
   }
 

testng-core/src/main/java/org/testng/internal/invokers/Invoker.java

@@ -6,10 +6,12 @@
 import org.testng.DataProviderHolder;
 import org.testng.IClass;
 import org.testng.IClassListener;
+import org.testng.IConfigurationListener;
 import org.testng.IInvokedMethodListener;
 import org.testng.ITestContext;
 import org.testng.ITestNGMethod;
 import org.testng.SuiteRunState;
+import org.testng.SuiteRunner;
 import org.testng.internal.IConfiguration;
 import org.testng.internal.ITestResultNotifier;
 
@@ -37,9 +39,18 @@ public Invoker(
       boolean skipFailedInvocationCounts,
       Collection<IInvokedMethodListener> invokedMethodListeners,
       List<IClassListener> classListeners,
-      DataProviderHolder holder) {
+      DataProviderHolder holder,
+      IConfigurationListener internalConfigurationListener,
+      SuiteRunner suiteRunner) {
     m_configInvoker =
-        new ConfigInvoker(notifier, invokedMethodListeners, testContext, state, configuration);
+        new ConfigInvoker(
+            notifier,
+            invokedMethodListeners,
+            testContext,
+            state,
+            configuration,
+            internalConfigurationListener,
+            suiteRunner);
     m_testInvoker =
         new TestInvoker(
             notifier,
@@ -50,7 +61,8 @@ public Invoker(
             holder,
             classListeners,
             skipFailedInvocationCounts,
-            m_configInvoker);
+            m_configInvoker,
+            suiteRunner);
   }
 
   public ConfigInvoker getConfigInvoker() {

testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java

@@ -69,8 +69,15 @@ public TestInvoker(
       DataProviderHolder holder,
       List<IClassListener> m_classListeners,
       boolean m_skipFailedInvocationCounts,
-      ConfigInvoker invoker) {
-    super(m_notifier, m_invokedMethodListeners, m_testContext, m_suiteState, m_configuration);
+      ConfigInvoker invoker,
+      SuiteRunner suiteRunner) {
+    super(
+        m_notifier,
+        m_invokedMethodListeners,
+        m_testContext,
+        m_suiteState,
+        m_configuration,
+        suiteRunner);
     this.holder = holder;
     this.m_classListeners = m_classListeners;
     this.m_skipFailedInvocationCounts = m_skipFailedInvocationCounts;

testng-core/src/test/java/org/testng/TestRunnerTest.java

@@ -3,6 +3,7 @@
 import static java.util.Collections.emptyList;
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.stream.Collectors;
 import org.testng.annotations.BeforeMethod;
@@ -85,10 +86,28 @@ private TestRunner createTestRunner(Class<?> testClass) {
     XmlClass xmlClass = new XmlClass(testClass.getName());
     xmlTest.getXmlClasses().add(xmlClass);
     String outputDir = "build/reports/tests/test";
-    ITestRunnerFactory factory =
-        (suite, test, listeners, classListeners) ->
-            new TestRunner(configuration, suite, test, false, listeners, classListeners);
-    ISuite suite = new SuiteRunner(configuration, xmlSuite, outputDir, factory, (o1, o2) -> 0);
+    LocalFactory factory = new LocalFactory(configuration);
+    SuiteRunner suite = new SuiteRunner(configuration, xmlSuite, outputDir, factory, (o1, o2) -> 0);
+    factory.suiteRunner = suite;
     return factory.newTestRunner(suite, xmlTest, emptyList(), emptyList());
   }
+
+  private static class LocalFactory implements ITestRunnerFactory {
+    private final IConfiguration configuration;
+    private SuiteRunner suiteRunner;
+
+    private LocalFactory(IConfiguration configuration) {
+      this.configuration = configuration;
+    }
+
+    @Override
+    public TestRunner newTestRunner(
+        ISuite suite,
+        XmlTest test,
+        Collection<IInvokedMethodListener> listeners,
+        List<IClassListener> classListeners) {
+      return new TestRunner(
+          configuration, suite, test, false, listeners, classListeners, suiteRunner);
+    }
+  }
 }