diff --git a/.travis.yml b/.travis.yml index 2cfe765e27..3fddd24b4d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,9 @@ language: java + +env: + global: + - GRADLE_OPTS=-Xmx512m + jdk: - oraclejdk8 - oraclejdk7 diff --git a/CHANGES.txt b/CHANGES.txt index b5e9578475..6ca609ef19 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ Current +Fixed: GITHUB-1232: Prevent TestNG from adding duplicate instances of the same listener (Krishnan Mahadevan) Fixed: GITHUB-1170: Fixing the test DataProviderTest.shouldNotThrowConcurrentModification (Krishnan Mahadevan) Fixed: GITHUB-1231: Make IExecutionListener implementation be the last reporter call before JVM exit(Krishnan Mahadevan) Fixed: GITHUB-1227: Prevent multiple instances of same Reporter from being injected into TestNG (Krishnan Mahadevan) diff --git a/build.gradle b/build.gradle index ad587913d5..15cf5b4f6b 100644 --- a/build.gradle +++ b/build.gradle @@ -113,6 +113,7 @@ test { // testLogging.showStandardStreams = true systemProperties = System.getProperties() systemProperties['test.resources.dir'] = 'build/resources/test/' + maxHeapSize = '1500m' } if (JavaVersion.current().isJava8Compatible()) { diff --git a/src/main/java/org/testng/SuiteRunner.java b/src/main/java/org/testng/SuiteRunner.java index 130ae67c58..2325c78858 100644 --- a/src/main/java/org/testng/SuiteRunner.java +++ b/src/main/java/org/testng/SuiteRunner.java @@ -104,6 +104,12 @@ public SuiteRunner(IConfiguration configuration, null /* class listeners */); } + /** + * @deprecated - This constructor stands deprecated. + */ + @Deprecated + //There are no external callers for this constructor but for TestNG. But since this method is a protected method + //we are following a proper deprecation strategy. protected SuiteRunner(IConfiguration configuration, XmlSuite suite, String outputDir, @@ -117,15 +123,28 @@ protected SuiteRunner(IConfiguration configuration, init(configuration, suite, outputDir, runnerFactory, useDefaultListeners, methodInterceptors, invokedMethodListeners, testListeners, classListeners); } + protected SuiteRunner(IConfiguration configuration, + XmlSuite suite, + String outputDir, + ITestRunnerFactory runnerFactory, + boolean useDefaultListeners, + List methodInterceptors, + Collection invokedMethodListeners, + Collection testListeners, + Collection classListeners) + { + init(configuration, suite, outputDir, runnerFactory, useDefaultListeners, methodInterceptors, invokedMethodListeners, testListeners, classListeners); + } + private void init(IConfiguration configuration, XmlSuite suite, String outputDir, ITestRunnerFactory runnerFactory, boolean useDefaultListeners, List methodInterceptors, - List invokedMethodListener, - List testListeners, - List classListeners) + Collection invokedMethodListener, + Collection testListeners, + Collection classListeners) { m_configuration = configuration; m_suite = suite; diff --git a/src/main/java/org/testng/TestNG.java b/src/main/java/org/testng/TestNG.java index 45494051e5..5b860ee48e 100644 --- a/src/main/java/org/testng/TestNG.java +++ b/src/main/java/org/testng/TestNG.java @@ -134,10 +134,10 @@ public class TestNG { private ITestRunnerFactory m_testRunnerFactory; // These listeners can be overridden from the command line - private List m_classListeners = Lists.newArrayList(); - private List m_testListeners = Lists.newArrayList(); - private List m_suiteListeners = Lists.newArrayList(); - private Map, IReporter> m_reporters = Maps.newHashMap(); + private final Map, IClassListener> m_classListeners = Maps.newHashMap(); + private final Map, ITestListener> m_testListeners = Maps.newHashMap(); + private final Map, ISuiteListener> m_suiteListeners = Maps.newHashMap(); + private final Map, IReporter> m_reporters = Maps.newHashMap(); protected static final int HAS_FAILURE = 1; protected static final int HAS_SKIPPED = 2; @@ -162,7 +162,8 @@ public class TestNG { private ITestObjectFactory m_objectFactory; - private List m_invokedMethodListeners = Lists.newArrayList(); + private final Map, IInvokedMethodListener> m_invokedMethodListeners = Maps + .newHashMap(); private Integer m_dataProviderThreadCount = null; @@ -178,9 +179,8 @@ public class TestNG { protected long m_end; protected long m_start; - private List m_executionListeners = Lists.newArrayList(); - - private List m_alterSuiteListeners= Lists.newArrayList(); + private final Map, IExecutionListener> m_executionListeners = Maps.newHashMap(); + private final Map, IAlterSuiteListener> m_alterSuiteListeners= Maps.newHashMap(); private boolean m_isInitialized = false; @@ -712,22 +712,33 @@ public void addListener(Object listener) { addListener((ITestNGListener) listener); } + private static void maybeAddListener(Map, E> map, Class type, E value) { + if (map.containsKey(value.getClass())) { + LOGGER.warn("Ignoring duplicate listener : " + value.getClass().getName()); + } else { + map.put(type, value); + } + } + public void addListener(ITestNGListener listener) { if (listener == null) { return; } if (listener instanceof ISuiteListener) { - m_suiteListeners.add((ISuiteListener) listener); + ISuiteListener suite = (ISuiteListener) listener; + maybeAddListener(m_suiteListeners, suite.getClass(), suite); } if (listener instanceof ITestListener) { - m_testListeners.add((ITestListener) listener); + ITestListener test = (ITestListener) listener; + maybeAddListener(m_testListeners, test.getClass(), test); } if (listener instanceof IClassListener) { - m_classListeners.add((IClassListener) listener); + IClassListener clazz = (IClassListener) listener; + maybeAddListener(m_classListeners, clazz.getClass(), clazz); } if (listener instanceof IReporter) { IReporter reporter = (IReporter) listener; - m_reporters.put(reporter.getClass(), reporter); + maybeAddListener(m_reporters, reporter.getClass(), reporter); } if (listener instanceof IAnnotationTransformer) { setAnnotationTransformer((IAnnotationTransformer) listener); @@ -736,7 +747,8 @@ public void addListener(ITestNGListener listener) { m_methodInterceptors.add((IMethodInterceptor) listener); } if (listener instanceof IInvokedMethodListener) { - m_invokedMethodListeners.add((IInvokedMethodListener) listener); + IInvokedMethodListener method = (IInvokedMethodListener) listener; + maybeAddListener(m_invokedMethodListeners, method.getClass(), method); } if (listener instanceof IHookable) { setHookable((IHookable) listener); @@ -745,13 +757,15 @@ public void addListener(ITestNGListener listener) { setConfigurable((IConfigurable) listener); } if (listener instanceof IExecutionListener) { - m_executionListeners.add((IExecutionListener) listener); + IExecutionListener execution = (IExecutionListener) listener; + maybeAddListener(m_executionListeners, execution.getClass(), execution); } if (listener instanceof IConfigurationListener) { getConfiguration().addConfigurationListener((IConfigurationListener) listener); } if (listener instanceof IAlterSuiteListener) { - m_alterSuiteListeners.add((IAlterSuiteListener) listener); + IAlterSuiteListener alter = (IAlterSuiteListener) listener; + maybeAddListener(m_alterSuiteListeners, alter.getClass(), alter); } } @@ -761,9 +775,7 @@ public void addListener(ITestNGListener listener) { // TODO remove later @Deprecated public void addListener(IInvokedMethodListener listener) { - if (!m_invokedMethodListeners.contains(listener)) { - addListener((ITestNGListener) listener); - } + addListener((ITestNGListener) listener); } /** @@ -772,9 +784,7 @@ public void addListener(IInvokedMethodListener listener) { // TODO remove later @Deprecated public void addListener(ISuiteListener listener) { - if (!m_suiteListeners.contains(listener)) { - addListener((ITestNGListener) listener); - } + addListener((ITestNGListener) listener); } /** @@ -783,9 +793,7 @@ public void addListener(ISuiteListener listener) { // TODO remove later @Deprecated public void addListener(ITestListener listener) { - if (!m_testListeners.contains(listener)) { - addListener((ITestNGListener) listener); - } + addListener((ITestNGListener) listener); } /** @@ -794,9 +802,7 @@ public void addListener(ITestListener listener) { // TODO remove later @Deprecated public void addListener(IClassListener listener) { - if (!m_classListeners.contains(listener)) { - addListener((ITestNGListener) listener); - } + addListener((ITestNGListener) listener); } /** @@ -805,9 +811,7 @@ public void addListener(IClassListener listener) { // TODO remove later @Deprecated public void addListener(IReporter listener) { - if (!m_reporters.containsValue(listener)) { - addListener((ITestNGListener) listener); - } + addListener((ITestNGListener) listener); } /** @@ -816,9 +820,7 @@ public void addListener(IReporter listener) { // TODO remove later @Deprecated public void addInvokedMethodListener(IInvokedMethodListener listener) { - if (!m_invokedMethodListeners.contains(listener)) { - addListener((ITestNGListener) listener); - } + addListener((ITestNGListener) listener); } public Set getReporters() { @@ -828,11 +830,11 @@ public Set getReporters() { } public List getTestListeners() { - return m_testListeners; + return Lists.newArrayList(m_testListeners.values()); } public List getSuiteListeners() { - return m_suiteListeners; + return Lists.newArrayList(m_suiteListeners.values()); } /** If m_verbose gets set, it will override the verbose setting in testng.xml */ @@ -936,7 +938,7 @@ private void addReporter(Class r) { } private void initializeDefaultListeners() { - m_testListeners.add(new ExitCodeListener(this)); + m_testListeners.put(ExitCodeListener.class, new ExitCodeListener(this)); if (m_useDefaultListeners) { addReporter(SuiteHTMLReporter.class); @@ -1132,8 +1134,8 @@ protected List runSuites() { } private void runSuiteAlterationListeners() { - for (List listeners - : Arrays.asList(m_alterSuiteListeners, m_configuration.getAlterSuiteListeners())) { + for (Collection listeners + : Arrays.asList(m_alterSuiteListeners.values(), m_configuration.getAlterSuiteListeners())) { for (IAlterSuiteListener l : listeners) { l.alter(m_suites); } @@ -1141,8 +1143,8 @@ private void runSuiteAlterationListeners() { } private void runExecutionListeners(boolean start) { - for (List listeners - : Arrays.asList(m_executionListeners, m_configuration.getExecutionListeners())) { + for (Collection listeners + : Arrays.asList(m_executionListeners.values(), m_configuration.getExecutionListeners())) { for (IExecutionListener l : listeners) { if (start) l.onExecutionStart(); else l.onExecutionFinish(); @@ -1368,11 +1370,11 @@ private SuiteRunner createSuiteRunner(XmlSuite xmlSuite) { m_testRunnerFactory, m_useDefaultListeners, m_methodInterceptors, - m_invokedMethodListeners, - m_testListeners, - m_classListeners); + m_invokedMethodListeners.values(), + m_testListeners.values(), + m_classListeners.values()); - for (ISuiteListener isl : m_suiteListeners) { + for (ISuiteListener isl : m_suiteListeners.values()) { result.addListener(isl); } @@ -2057,7 +2059,7 @@ public void setGroupByInstances(boolean b) { // private URLClassLoader m_serviceLoaderClassLoader; - private List m_serviceLoaderListeners = Lists.newArrayList(); + private Map, ITestNGListener> m_serviceLoaderListeners = Maps.newHashMap(); /* * Used to test ServiceClassLoader @@ -2070,14 +2072,16 @@ public void setServiceLoaderClassLoader(URLClassLoader ucl) { * Used to test ServiceClassLoader */ private void addServiceLoaderListener(ITestNGListener l) { - m_serviceLoaderListeners.add(l); + if (! m_serviceLoaderListeners.containsKey(l.getClass())) { + m_serviceLoaderListeners.put(l.getClass(), l); + } } /* * Used to test ServiceClassLoader */ public List getServiceLoaderListeners() { - return m_serviceLoaderListeners; + return Lists.newArrayList(m_serviceLoaderListeners.values()); } // diff --git a/src/test/java/test/reports/UniqueReporterInjectionTest.java b/src/test/java/test/reports/UniqueReporterInjectionTest.java index 1ff9fd9826..adc3ac9276 100644 --- a/src/test/java/test/reports/UniqueReporterInjectionTest.java +++ b/src/test/java/test/reports/UniqueReporterInjectionTest.java @@ -19,7 +19,9 @@ public void testPruningOfDuplicateReporter() { tng.setUseDefaultListeners(false); tng.addListener((ITestNGListener) new ReporterListenerForIssue1227()); tng.run(); - Assert.assertEquals(tng.getReporters().size(),1); + //Since we have another reporting listener that is injected via the service loader file + //reporting listeners size will now have to be two. + Assert.assertEquals(tng.getReporters().size(),2); Assert.assertEquals(ReporterListenerForIssue1227.counter, 1); } diff --git a/src/test/java/test/serviceloader/ServiceLoaderTest.java b/src/test/java/test/serviceloader/ServiceLoaderTest.java index 683ec002d5..80890d1bbd 100644 --- a/src/test/java/test/serviceloader/ServiceLoaderTest.java +++ b/src/test/java/test/serviceloader/ServiceLoaderTest.java @@ -43,7 +43,7 @@ public void serviceLoaderShouldWorkWithConfigurationListener() { TestNG tng = create(ServiceLoaderSampleTest.class); tng.run(); - Assert.assertEquals(1, tng.getServiceLoaderListeners().size()); + Assert.assertEquals(2, tng.getServiceLoaderListeners().size()); ListenerAssert.assertListenerType(tng.getServiceLoaderListeners(), MyConfigurationListener.class); } } diff --git a/src/test/java/test/testng1232/ListenerTemplate.java b/src/test/java/test/testng1232/ListenerTemplate.java new file mode 100644 index 0000000000..4e9e02dfc3 --- /dev/null +++ b/src/test/java/test/testng1232/ListenerTemplate.java @@ -0,0 +1,106 @@ +package test.testng1232; + +import org.testng.*; +import org.testng.xml.XmlSuite; + +import java.util.List; + +/** + * This class provides "void" implementations for all listener invocations so that one can tweak + * behavior of only those methods which need customization. (Mainly to circumvent verbosity in + * actual listener implementations) + */ +public class ListenerTemplate implements + IInvokedMethodListener, + IClassListener, + ITestListener, + ISuiteListener, + IAlterSuiteListener, + IExecutionListener, + IReporter { + + @Override + public void onBeforeClass(ITestClass testClass) { + + } + + @Override + public void onAfterClass(ITestClass testClass) { + + } + + @Override + public void onStart(ISuite suite) { + + } + + @Override + public void onFinish(ISuite suite) { + + } + + @Override + public void beforeInvocation(IInvokedMethod method, ITestResult testResult) { + + } + + @Override + public void afterInvocation(IInvokedMethod method, ITestResult testResult) { + + } + + @Override + public void onTestStart(ITestResult result) { + + } + + @Override + public void onTestSuccess(ITestResult result) { + + } + + @Override + public void onTestFailure(ITestResult result) { + + } + + @Override + public void onTestSkipped(ITestResult result) { + + } + + @Override + public void onTestFailedButWithinSuccessPercentage(ITestResult result) { + + } + + @Override + public void onStart(ITestContext context) { + + } + + @Override + public void onFinish(ITestContext context) { + + } + + @Override + public void onExecutionStart() { + + } + + @Override + public void onExecutionFinish() { + + } + + @Override + public void alter(List suites) { + + } + + @Override + public void generateReport(List xmlSuites, List suites, String outputDirectory) { + + } +} diff --git a/src/test/java/test/testng1232/TestClassContainer.java b/src/test/java/test/testng1232/TestClassContainer.java new file mode 100644 index 0000000000..39ae473000 --- /dev/null +++ b/src/test/java/test/testng1232/TestClassContainer.java @@ -0,0 +1,24 @@ +package test.testng1232; + +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +/** + * This Class houses all the test classes that are required by {@link TestListenerInstances} + */ +public class TestClassContainer { + + public static class SimpleTestClass { + @Test + public void testMethod() { + } + } + + @Listeners(TestListenerFor1232.class) + public static class SimpleTestClassWithListener { + @Test + public void testMethod() { + } + } + +} diff --git a/src/test/java/test/testng1232/TestListenerFor1232.java b/src/test/java/test/testng1232/TestListenerFor1232.java new file mode 100644 index 0000000000..6b0121271a --- /dev/null +++ b/src/test/java/test/testng1232/TestListenerFor1232.java @@ -0,0 +1,71 @@ +package test.testng1232; + +import org.testng.*; +import org.testng.collections.Maps; +import org.testng.xml.XmlSuite; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +public class TestListenerFor1232 extends ListenerTemplate { + static Map counters = Maps.newHashMap(); + + static synchronized void resetCounters() { + counters = Maps.newHashMap(); + } + + @Override + public void beforeInvocation(IInvokedMethod method, ITestResult testResult) { + incrementCounter(CounterTypes.METHOD); + } + + @Override + public void onBeforeClass(ITestClass testClass) { + incrementCounter(CounterTypes.CLASS); + } + + @Override + public void onStart(ITestContext context) { + incrementCounter(CounterTypes.TEST); + } + + @Override + public void onStart(ISuite suite) { + incrementCounter(CounterTypes.SUITE); + } + + @Override + public void alter(List suites) { + incrementCounter(CounterTypes.ALTER_SUITE); + } + + @Override + public void onExecutionStart() { + incrementCounter(CounterTypes.EXECUTION); + } + + @Override + public void generateReport(List xmlSuites, List suites, String outputDirectory) { + incrementCounter(CounterTypes.REPORTER); + } + + private void incrementCounter(CounterTypes type) { + if (!counters.containsKey(type)){ + counters.put(type, new AtomicInteger(0)); + } + AtomicInteger value = counters.get(type); + value.incrementAndGet(); + counters.put(type, value); + } + + enum CounterTypes { + METHOD, + CLASS, + TEST, + SUITE, + ALTER_SUITE, + EXECUTION, + REPORTER + } +} diff --git a/src/test/java/test/testng1232/TestListenerInstances.java b/src/test/java/test/testng1232/TestListenerInstances.java new file mode 100644 index 0000000000..88dc605fe3 --- /dev/null +++ b/src/test/java/test/testng1232/TestListenerInstances.java @@ -0,0 +1,56 @@ +package test.testng1232; + +import org.testng.Assert; +import org.testng.ITestNGListener; +import org.testng.TestNG; +import org.testng.annotations.Test; +import org.testng.xml.XmlSuite; +import org.testng.xml.XmlTest; +import test.SimpleBaseTest; +import test.testng1232.TestListenerFor1232.CounterTypes; + +public class TestListenerInstances extends SimpleBaseTest { + + @Test + public void testIfOnlyOneListenerInstanceExists() { + runTestForTestClass(TestClassContainer.SimpleTestClass.class); + } + + @Test + public void testIfOnlyOneListenerInstanceExistsUsingAnnotations() { + runTestForTestClass(TestClassContainer.SimpleTestClassWithListener.class); + } + + @Test + public void testIfOnlyOneListenerInstanceExistsUsingListenerTag() { + runTestForTestClass(TestClassContainer.SimpleTestClass.class, true); + } + + private static void runTestForTestClass(Class clazz) { + runTestForTestClass(clazz, false); + } + + private static void runTestForTestClass(Class clazz, boolean injectListenerViaTag) { + TestNG tng = createTestNGInstanceFor(clazz, injectListenerViaTag); + TestListenerFor1232.resetCounters(); + TestListenerFor1232 listener = new TestListenerFor1232(); + tng.addListener((ITestNGListener) listener); + TestListenerFor1232 anotherListener = new TestListenerFor1232(); + tng.addListener((ITestNGListener) anotherListener); + tng.run(); + for (CounterTypes type : CounterTypes.values()) { + Assert.assertEquals(TestListenerFor1232.counters.get(type).intValue(), 1); + } + } + + private static TestNG createTestNGInstanceFor(Class clazz, boolean addListenerTag) { + XmlSuite xmlSuite = createXmlSuite("Suite"); + if (addListenerTag) { + xmlSuite.addListener(TestListenerFor1232.class.getName()); + } + XmlTest xmlTest = createXmlTest(xmlSuite, "Test"); + createXmlClass(xmlTest, clazz); + return create(xmlSuite); + } + +} diff --git a/src/test/resources/META-INF/services/org.testng.ITestNGListener b/src/test/resources/META-INF/services/org.testng.ITestNGListener index 9f76d9b54a..623aefa15b 100644 --- a/src/test/resources/META-INF/services/org.testng.ITestNGListener +++ b/src/test/resources/META-INF/services/org.testng.ITestNGListener @@ -1 +1,2 @@ -test.serviceloader.MyConfigurationListener \ No newline at end of file +test.serviceloader.MyConfigurationListener +test.testng1232.TestListenerFor1232 diff --git a/src/test/resources/testng.xml b/src/test/resources/testng.xml index e714687a95..696847b18b 100644 --- a/src/test/resources/testng.xml +++ b/src/test/resources/testng.xml @@ -121,6 +121,7 @@ +