Ensure All tests run all the time

Closes #2841

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

@@ -347,6 +347,10 @@ private void privateRun() {
       // -- cbeust
       invoker = tr.getInvoker();
 
+      // Add back the configuration listeners that may have gotten altered after
+      // our suite level listeners were invoked.
+      this.configuration.getConfigurationListeners().forEach(tr::addConfigurationListener);
+
       for (ITestNGMethod m : tr.getBeforeSuiteMethods()) {
         beforeSuiteMethods.put(m.getConstructorOrMethod().getMethod(), m);
       }

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

@@ -10,7 +10,6 @@
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
@@ -1160,14 +1159,12 @@ public void setVerbose(int n) {
   // TODO: This method needs to be removed and we need to be leveraging addListener().
   // Investigate and fix this.
   void addTestListener(ITestListener listener) {
-    Optional<ITestListener> found =
+    boolean found =
         m_testListeners.stream()
-            .filter(iTestListener -> iTestListener.getClass().equals(listener.getClass()))
-            .findAny();
-    if (found.isPresent()) {
-      return;
+            .anyMatch(iTestListener -> iTestListener.getClass().equals(listener.getClass()));
+    if (!found) {
+      m_testListeners.add(listener);
     }
-    m_testListeners.add(listener);
   }
 
   public void addListener(ITestNGListener listener) {
@@ -1214,7 +1211,11 @@ public void addListener(ITestNGListener listener) {
   }
 
   void addConfigurationListener(IConfigurationListener icl) {
-    m_configurationListeners.add(icl);
+    boolean alreadyAdded =
+        m_configurationListeners.stream().anyMatch(each -> each.getClass().equals(icl.getClass()));
+    if (!alreadyAdded) {
+      m_configurationListeners.add(icl);
+    }
   }
 
   private void dumpInvokedMethods() {

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

@@ -23,7 +23,7 @@ public boolean equals(Object o) {
     if (this == o) return true;
     if (o == null || getClass() != o.getClass()) return false;
     ClassBasedWrapper<?> wrapper = (ClassBasedWrapper<?>) o;
-    return object.getClass().equals(wrapper.getClass());
+    return object.getClass().equals(wrapper.object.getClass());
   }
 
   @Override

testng-core/src/test/groovy/test/groovy/SpockSample.groovy

@@ -1,9 +1,11 @@
 package test.groovy
 
+import org.junit.Test
 import spock.lang.Specification
 
 class SpockSample extends Specification {
 
+    @Test
     def "adding an element leads to size increase"() {
         setup: "a new stack instance is created"
         def stack = new Stack()
@@ -14,4 +16,4 @@ class SpockSample extends Specification {
         then:
         stack.size() == 1
     }
-}
+}

testng-core/src/test/java/test/InvokedMethodNameListener.java

@@ -58,7 +58,9 @@ public void beforeInvocation(IInvokedMethod method, ITestResult testResult) {
   @Override
   public void afterInvocation(IInvokedMethod method, ITestResult testResult) {
     List<String> methodNames =
-        mapping.computeIfAbsent(testResult.getMethod().getRealClass(), k -> Lists.newArrayList());
+        mapping.computeIfAbsent(
+            testResult.getMethod().getRealClass(),
+            k -> Collections.synchronizedList(Lists.newArrayList()));
     methodNames.add(method.getTestMethod().getMethodName());
     String name = getName(testResult);
     switch (testResult.getStatus()) {

testng-core/src/test/java/test/dataprovider/DataProviderTest.java

@@ -78,6 +78,7 @@ public void testDataProviderCanBeRetriedViaAnnotationTransformer() {
 
   @Test(description = "GITHUB-2819")
   public void testDataProviderRetryInstancesAreUniqueForEachDataDrivenTest() {
+    SimpleRetry.clearHashCodes();
     TestNG testng = create(TestClassWithMultipleRetryImplSample.class);
     DataProviderListenerForRetryAwareTests listener = new DataProviderListenerForRetryAwareTests();
     testng.addListener(listener);

testng-core/src/test/java/test/dataprovider/issue2819/SimpleRetry.java

@@ -3,19 +3,24 @@
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.UUID;
 import org.testng.IDataProviderMethod;
 import org.testng.IRetryDataProvider;
 
 public class SimpleRetry implements IRetryDataProvider {
 
-  private static final Set<Integer> hashCodes = new HashSet<>();
+  private static final Set<String> hashCodes = new HashSet<>();
 
-  public static Set<Integer> getHashCodes() {
+  public static Set<String> getHashCodes() {
     return Collections.unmodifiableSet(hashCodes);
   }
 
+  public static void clearHashCodes() {
+    hashCodes.clear();
+  }
+
   public SimpleRetry() {
-    hashCodes.add(hashCode());
+    hashCodes.add(UUID.randomUUID().toString());
   }
 
   private int counter = 0;

testng-core/src/test/java/test/listeners/github1130/GitHub1130Test.java

@@ -2,7 +2,6 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-import java.util.ArrayList;
 import org.testng.ITestNGListener;
 import org.testng.TestListenerAdapter;
 import org.testng.TestNG;
@@ -22,19 +21,20 @@ public void classListenerShouldBeOnlyInstantiatedOnce() {
   }
 
   private void checkGithub1130(TestNG tng) {
-    MyListener.count = 0;
-    MyListener.beforeSuiteCount = new ArrayList<>();
-    MyListener.beforeClassCount = new ArrayList<>();
     TestListenerAdapter adapter = new TestListenerAdapter();
     tng.addListener((ITestNGListener) adapter);
     tng.run();
     assertThat(adapter.getFailedTests()).isEmpty();
     assertThat(adapter.getSkippedTests()).isEmpty();
-    assertThat(MyListener.beforeSuiteCount.size()).isEqualTo(1);
-    assertThat(MyListener.beforeClassCount.size()).isEqualTo(2);
-    assertThat(MyListener.beforeSuiteCount.get(0))
-        .isEqualTo(MyListener.beforeClassCount.get(0))
-        .isEqualTo(MyListener.beforeClassCount.get(1));
-    assertThat(MyListener.count).isEqualTo(1);
+    // We cannot prevent TestNG from instantiating a listener multiple times
+    // because TestNG can throw away listener instances if it finds them already registered
+    // So the assertion should basically be able to check that ONLY 1 instance of the listener
+    // is being used, even though TestNG created multiple instances of it.
+    assertThat(MyListener.instance).isNotNull();
+    assertThat(MyListener.instance.beforeSuiteCount.size()).isEqualTo(1);
+    assertThat(MyListener.instance.beforeClassCount.size()).isEqualTo(2);
+    assertThat(MyListener.instance.beforeSuiteCount.get(0))
+        .isEqualTo(MyListener.instance.beforeClassCount.get(0))
+        .isEqualTo(MyListener.instance.beforeClassCount.get(1));
   }
 }

testng-core/src/test/java/test/listeners/github1130/MyListener.java

@@ -11,12 +11,14 @@
 
 public class MyListener implements ISuiteListener, IClassListener {
 
-  public static int count = 0;
-  public static List<String> beforeSuiteCount = new ArrayList<>();
-  public static List<String> beforeClassCount = new ArrayList<>();
+  public static MyListener instance;
+  public List<String> beforeSuiteCount = new ArrayList<>();
+  public List<String> beforeClassCount = new ArrayList<>();
 
   public MyListener() {
-    count++;
+    if (instance == null) {
+      instance = this;
+    }
   }
 
   public void onStart(ISuite suite) {

testng-core/src/test/java/test/listeners/github1296/GitHub1296Test.java

@@ -2,6 +2,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.entry;
+import static test.listeners.github1296.MyConfigurationListener.getCalls;
 
 import org.testng.TestNG;
 import org.testng.annotations.Test;
@@ -12,7 +13,6 @@ public class GitHub1296Test extends SimpleBaseTest {
 
   @Test(description = "https://github.com/cbeust/testng/issues/1296")
   public void test_number_of_call_of_configuration_listener() {
-    MyConfigurationListener.CALLS.clear();
     XmlSuite suite = createXmlSuite("Tests");
     createXmlTest(suite, "Test version", MyTest.class);
     createXmlTest(suite, "Test version 2", MyTest.class);
@@ -21,7 +21,7 @@ public void test_number_of_call_of_configuration_listener() {
 
     tng.run();
 
-    assertThat(MyConfigurationListener.CALLS)
+    assertThat(getCalls())
         .hasSize(3)
         .containsOnly(
             entry("Test version", 2), entry("Test version 2", 2), entry("Test version 3", 2));

testng-core/src/test/java/test/listeners/github1296/MyConfigurationListener.java

@@ -1,23 +1,30 @@
 package test.listeners.github1296;
 
-import java.util.HashMap;
 import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Collectors;
 import org.testng.IConfigurationListener;
 import org.testng.ITestResult;
 
 public class MyConfigurationListener implements IConfigurationListener {
 
-  public static final Map<String, Integer> CALLS = new HashMap<>();
+  private static final Map<String, AtomicInteger> CALLS = new ConcurrentHashMap<>();
+
+  public MyConfigurationListener() {
+    CALLS.clear();
+  }
+
+  public static Map<String, Integer> getCalls() {
+    return CALLS.entrySet().stream()
+        .collect(Collectors.toMap(Entry::getKey, each -> each.getValue().get()));
+  }
 
   @Override
   public void onConfigurationSuccess(ITestResult itr) {
     String xmlTestName = itr.getTestContext().getCurrentXmlTest().getName();
-    Integer count = CALLS.get(xmlTestName);
-    if (count == null) {
-      count = 0;
-    }
-    count++;
-    CALLS.put(xmlTestName, count);
+    CALLS.computeIfAbsent(xmlTestName, k -> new AtomicInteger()).incrementAndGet();
   }
 
   @Override

testng-core/src/test/java/test/reports/EmailableReporterTest.java

@@ -75,7 +75,7 @@ private void runTestViaMainMethod(String clazzName, String jvm) {
       if (jvm != null) {
         System.setProperty(jvm, filename);
       }
-      TestNG.main(args);
+      TestNG.privateMain(args, null);
     } catch (SecurityException t) {
       // Gobble Security exception
     } finally {

testng-core/testng-core-build.gradle.kts

@@ -37,9 +37,10 @@ dependencies {
     implementation(projects.testngReflectionUtils)
     implementation(projects.testngRunnerApi)
     implementation("org.webjars:jquery:_")
-
     testImplementation(projects.testngAsserts)
-    testImplementation("org.codehaus.groovy:groovy-all:_")
+    testImplementation("org.codehaus.groovy:groovy-all:_") {
+        exclude("org.testng", "testng")
+    }
     testImplementation("org.spockframework:spock-core:_")
     testImplementation("org.apache-extras.beanshell:bsh:_")
     testImplementation("org.mockito:mockito-core:_")
@@ -64,7 +65,6 @@ tasks.test {
     maxParallelForks = Runtime.getRuntime().availableProcessors().div(2)
     (testFramework.options as TestNGOptions).apply {
         suites("src/test/resources/testng.xml")
-        listeners.add("org.testng.reporters.FailedInformationOnConsoleReporter")
         maxHeapSize = "1500m"
     }
 }