From 5407253c901e52438fd8d100ab537beff4072bb6 Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Wed, 12 Jan 2022 13:01:56 +0530 Subject: [PATCH] IHookable and IConfigurable callback discrepancy Closes #2704 Ensure that TestNG reports scenarios wherein User defines callbacks for configurations/test methods But fails to invoke those callbacks explicitly And also fails in shipping test status to a user recognized status (PASS|FAILURE|SKIP) --- CHANGES.txt | 1 + .../ConfigurationNotInvokedException.java | 25 +++++ .../src/main/java/org/testng/ITestResult.java | 22 +++++ .../org/testng/TestNotInvokedException.java | 24 +++++ .../org/testng/internal/MethodHelper.java | 7 +- .../internal/invokers/ConfigInvoker.java | 18 +++- .../invokers/InvokeMethodRunnable.java | 32 +++--- .../invokers/MethodInvocationHelper.java | 49 ++++++---- .../testng/internal/invokers/TestInvoker.java | 41 +++++--- .../src/test/java/test/hook/HookableTest.java | 98 ++++++++++++++++--- ...gurableFailureWithStatusAlteredSample.java | 20 ++++ .../HookFailureWithStatusAlteredSample.java | 25 +++++ .../samples/HookSuccessTimeoutSample.java | 16 ++- ...kSuccessTimeoutWithDataProviderSample.java | 31 ++++++ 14 files changed, 329 insertions(+), 80 deletions(-) create mode 100644 testng-core-api/src/main/java/org/testng/ConfigurationNotInvokedException.java create mode 100644 testng-core-api/src/main/java/org/testng/TestNotInvokedException.java create mode 100644 testng-core/src/test/java/test/hook/samples/ConfigurableFailureWithStatusAlteredSample.java create mode 100644 testng-core/src/test/java/test/hook/samples/HookFailureWithStatusAlteredSample.java create mode 100644 testng-core/src/test/java/test/hook/samples/HookSuccessTimeoutWithDataProviderSample.java diff --git a/CHANGES.txt b/CHANGES.txt index 5126623e59..a0ce8b008d 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,6 +1,7 @@ Current 7.6.0 Fixed: GITHUB-2709: Testnames not working together with suites in suite (Martin Aldrin) +Fixed: GITHUB-2704: IHookable and IConfigurable callback discrepancy (Krishnan Mahadevan) Fixed: GITHUB-2637: Upgrade to JDK11 as the minimum JDK requirements(Krishnan Mahadevan) 7.5 diff --git a/testng-core-api/src/main/java/org/testng/ConfigurationNotInvokedException.java b/testng-core-api/src/main/java/org/testng/ConfigurationNotInvokedException.java new file mode 100644 index 0000000000..169f0aefe5 --- /dev/null +++ b/testng-core-api/src/main/java/org/testng/ConfigurationNotInvokedException.java @@ -0,0 +1,25 @@ +package org.testng; + +/** + * Represents an exception that is thrown when a configuration method is not invoked. One of the + * use-cases when this can happen is when the user does the following: + * + * + */ +public class ConfigurationNotInvokedException extends TestNGException { + + public ConfigurationNotInvokedException(ITestNGMethod tm) { + super( + tm.getQualifiedName() + + " defines a callback via " + + IConfigurable.class.getName() + + " but neither the callback was invoked nor the status was altered to " + + String.join("|", ITestResult.finalStatuses())); + } +} diff --git a/testng-core-api/src/main/java/org/testng/ITestResult.java b/testng-core-api/src/main/java/org/testng/ITestResult.java index 08db76c588..00649a4615 100644 --- a/testng-core-api/src/main/java/org/testng/ITestResult.java +++ b/testng-core-api/src/main/java/org/testng/ITestResult.java @@ -1,5 +1,6 @@ package org.testng; +import java.util.Arrays; import java.util.Collections; import java.util.List; import org.testng.internal.thread.ThreadTimeoutException; @@ -114,6 +115,27 @@ default List getSkipCausedBy() { */ String id(); + /** + * @return - true if the current test result is either {@link ITestResult#STARTED} or + * {@link ITestResult#CREATED} + */ + default boolean isNotRunning() { + return getStatus() == STARTED || getStatus() == CREATED; + } + + /** + * @return - A list of all user facing statuses viz., + * + */ + static List finalStatuses() { + return Arrays.asList("SUCCESS", "FAILURE", "SKIP", "SUCCESS_PERCENTAGE_FAILURE"); + } + /** * @param result - The test result of a method * @return - true if the test failure was due to a timeout. diff --git a/testng-core-api/src/main/java/org/testng/TestNotInvokedException.java b/testng-core-api/src/main/java/org/testng/TestNotInvokedException.java new file mode 100644 index 0000000000..acf3b8663f --- /dev/null +++ b/testng-core-api/src/main/java/org/testng/TestNotInvokedException.java @@ -0,0 +1,24 @@ +package org.testng; + +/** + * Represents an exception that is thrown when a test method is not invoked. One of the use-cases + * when this can happen is when the user does the following: + * + *
    + *
  • User defines a test method + *
  • The class that houses the test method defines support for callbacks via {@link IHookable} + * implementation + *
  • User willfully skips invoking the callback and also fails at altering the test method's + * status via {@link ITestResult#setStatus(int)} + *
+ */ +public class TestNotInvokedException extends TestNGException { + public TestNotInvokedException(ITestNGMethod tm) { + super( + tm.getQualifiedName() + + " defines a callback via " + + IHookable.class.getName() + + " but neither the callback was invoked nor the status was altered to " + + String.join("|", ITestResult.finalStatuses())); + } +} diff --git a/testng-core/src/main/java/org/testng/internal/MethodHelper.java b/testng-core/src/main/java/org/testng/internal/MethodHelper.java index c6d0c3c765..4986d314c8 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodHelper.java +++ b/testng-core/src/main/java/org/testng/internal/MethodHelper.java @@ -484,11 +484,8 @@ private static MatchResults matchMethod(ITestNGMethod[] methods, String regexp) String thisMethodName = thisMethod.getName(); String methodName = usePackage ? calculateMethodCanonicalName(method) : thisMethodName; Pair cacheKey = Pair.create(regexp, methodName); - Boolean match = MATCH_CACHE.get(cacheKey); - if (match == null) { - match = pattern.matcher(methodName).matches(); - MATCH_CACHE.put(cacheKey, match); - } + boolean match = + MATCH_CACHE.computeIfAbsent(cacheKey, key -> pattern.matcher(methodName).matches()); if (match) { results.matchedMethods.add(method); results.foundAtLeastAMethod = true; diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java index 6e92f9adc0..52411e8a3d 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java @@ -11,6 +11,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import org.testng.ConfigurationNotInvokedException; import org.testng.IClass; import org.testng.IConfigurable; import org.testng.IInvokedMethodListener; @@ -374,15 +375,24 @@ private void invokeConfigurationMethod( testResult.setStatus(ITestResult.SUCCESS); return; } - if (configurableInstance != null) { - MethodInvocationHelper.invokeConfigurable( - targetInstance, params, configurableInstance, method.getMethod(), testResult); + boolean willfullyIgnored = false; + boolean usesConfigurableInstance = configurableInstance != null; + if (usesConfigurableInstance) { + willfullyIgnored = + !MethodInvocationHelper.invokeConfigurable( + targetInstance, params, configurableInstance, method.getMethod(), testResult); } else { MethodInvocationHelper.invokeMethodConsideringTimeout( tm, method, targetInstance, params, testResult); } + boolean testStatusRemainedUnchanged = testResult.isNotRunning(); + if (usesConfigurableInstance && willfullyIgnored && testStatusRemainedUnchanged) { + throw new ConfigurationNotInvokedException(tm); + } testResult.setStatus(ITestResult.SUCCESS); - } catch (InvocationTargetException | IllegalAccessException ex) { + } catch (ConfigurationNotInvokedException + | InvocationTargetException + | IllegalAccessException ex) { throwConfigurationFailure(testResult, ex); testResult.setStatus(ITestResult.FAILURE); throw ex; diff --git a/testng-core/src/main/java/org/testng/internal/invokers/InvokeMethodRunnable.java b/testng-core/src/main/java/org/testng/internal/invokers/InvokeMethodRunnable.java index 61518dfcae..7ac48b7a04 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/InvokeMethodRunnable.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/InvokeMethodRunnable.java @@ -1,5 +1,6 @@ package org.testng.internal.invokers; +import java.util.Optional; import java.util.concurrent.Callable; import org.testng.IHookable; import org.testng.ITestNGMethod; @@ -7,7 +8,7 @@ import org.testng.internal.ConstructorOrMethod; /** A Runnable Method invoker. */ -public class InvokeMethodRunnable implements Callable { +public class InvokeMethodRunnable implements Callable { private final ITestNGMethod m_method; private final Object m_instance; private final Object[] m_parameters; @@ -27,51 +28,54 @@ public InvokeMethodRunnable( m_testResult = testResult; } - public void run() throws TestNGRuntimeException { + public boolean run() throws TestNGRuntimeException { try { - call(); + return call(); } catch (Exception e) { throw new TestNGRuntimeException(e); } } - private void runOne() { + private boolean runOne() { + boolean invoked; try { RuntimeException t = null; try { ConstructorOrMethod m = m_method.getConstructorOrMethod(); if (m_hookable == null) { + invoked = true; MethodInvocationHelper.invokeMethod(m.getMethod(), m_instance, m_parameters); } else { - MethodInvocationHelper.invokeHookable( - m_instance, m_parameters, m_hookable, m.getMethod(), m_testResult); + invoked = + MethodInvocationHelper.invokeHookable( + m_instance, m_parameters, m_hookable, m.getMethod(), m_testResult); } } catch (Throwable e) { - Throwable cause = e.getCause(); - if (cause == null) { - cause = e; - } + invoked = true; + Throwable cause = Optional.ofNullable(e.getCause()).orElse(e); t = new TestNGRuntimeException(cause); } if (null != t) { Thread.currentThread().interrupt(); throw t; } + return invoked; } finally { m_method.incrementCurrentInvocationCount(); } } @Override - public Void call() throws Exception { + public Boolean call() throws Exception { + boolean flag = true; if (m_method.getInvocationTimeOut() > 0) { for (int i = 0; i < m_method.getInvocationCount(); i++) { - runOne(); + flag = flag && runOne(); } } else { - runOne(); + flag = runOne(); } - return null; + return flag; } public static class TestNGRuntimeException extends RuntimeException { diff --git a/testng-core/src/main/java/org/testng/internal/invokers/MethodInvocationHelper.java b/testng-core/src/main/java/org/testng/internal/invokers/MethodInvocationHelper.java index e78c332f74..b1dab83b5e 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/MethodInvocationHelper.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/MethodInvocationHelper.java @@ -17,6 +17,7 @@ import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import org.testng.IConfigurable; import org.testng.IConfigureCallBack; import org.testng.IHookCallBack; @@ -25,6 +26,7 @@ import org.testng.ITestNGMethod; import org.testng.ITestResult; import org.testng.TestNGException; +import org.testng.TestNotInvokedException; import org.testng.internal.ConstructorOrMethod; import org.testng.internal.MethodHelper; import org.testng.internal.Utils; @@ -235,7 +237,7 @@ private static List getParameters( return parameters; } - protected static void invokeHookable( + protected static boolean invokeHookable( final Object testInstance, final Object[] parameters, final IHookable hookable, @@ -243,12 +245,14 @@ protected static void invokeHookable( final ITestResult testResult) throws Throwable { final Throwable[] error = new Throwable[1]; + AtomicReference wasCalled = new AtomicReference<>(false); IHookCallBack callback = new IHookCallBack() { @Override public void runTestMethod(ITestResult tr) { try { + wasCalled.set(true); invokeMethod(thisMethod, testInstance, parameters); error[0] = null; tr.setThrowable(null); @@ -267,6 +271,7 @@ public Object[] getParameters() { if (error[0] != null) { throw error[0]; } + return wasCalled.get(); } /** @@ -279,7 +284,7 @@ protected static void invokeWithTimeout( invokeWithTimeout(tm, instance, parameterValues, testResult, null); } - protected static void invokeWithTimeout( + protected static boolean invokeWithTimeout( ITestNGMethod tm, Object instance, Object[] parameterValues, @@ -291,13 +296,13 @@ protected static void invokeWithTimeout( != XmlSuite.ParallelMode.TESTS) { // We are already running in our own executor, don't create another one (or we will // lose the time out of the enclosing executor). - invokeWithTimeoutWithNoExecutor(tm, instance, parameterValues, testResult, hookable); + return invokeWithTimeoutWithNoExecutor(tm, instance, parameterValues, testResult, hookable); } else { - invokeWithTimeoutWithNewExecutor(tm, instance, parameterValues, testResult, hookable); + return invokeWithTimeoutWithNewExecutor(tm, instance, parameterValues, testResult, hookable); } } - private static void invokeWithTimeoutWithNoExecutor( + private static boolean invokeWithTimeoutWithNoExecutor( ITestNGMethod tm, Object instance, Object[] parameterValues, @@ -312,6 +317,7 @@ private static void invokeWithTimeoutWithNoExecutor( AtomicBoolean finished = new AtomicBoolean(false); AtomicBoolean interruptByMonitor = new AtomicBoolean(false); Thread monitorThread = null; + boolean wasInvoked = false; try { Thread currentThread = Thread.currentThread(); monitorThread = @@ -328,7 +334,7 @@ private static void invokeWithTimeoutWithNoExecutor( } }); monitorThread.start(); - imr.run(); + wasInvoked = imr.run(); notTimedout = System.currentTimeMillis() <= startTime + realTimeOut; if (notTimedout) { testResult.setStatus(ITestResult.SUCCESS); @@ -336,6 +342,7 @@ private static void invokeWithTimeoutWithNoExecutor( testResult.setThrowable(new ThreadTimeoutException(tm, realTimeOut)); testResult.setStatus(ITestResult.FAILURE); } + return wasInvoked; } catch (Exception ex) { if (notTimedout && !interruptByMonitor.get()) { Throwable e = ex.getCause(); @@ -344,9 +351,12 @@ private static void invokeWithTimeoutWithNoExecutor( } testResult.setThrowable(e); } else { - testResult.setThrowable(new ThreadTimeoutException(tm, realTimeOut)); + if (!(ex instanceof TestNotInvokedException)) { + testResult.setThrowable(new ThreadTimeoutException(tm, realTimeOut)); + } } testResult.setStatus(ITestResult.FAILURE); + return wasInvoked; } finally { finished.set(true); if (monitorThread != null && monitorThread.isAlive()) { @@ -355,7 +365,7 @@ private static void invokeWithTimeoutWithNoExecutor( } } - private static void invokeWithTimeoutWithNewExecutor( + private static boolean invokeWithTimeoutWithNewExecutor( ITestNGMethod tm, Object instance, Object[] parameterValues, @@ -366,7 +376,7 @@ private static void invokeWithTimeoutWithNewExecutor( InvokeMethodRunnable imr = new InvokeMethodRunnable(tm, instance, parameterValues, hookable, testResult); - Future future = exec.submit(imr); + Future future = exec.submit(imr); exec.shutdown(); long realTimeOut = MethodHelper.calculateTimeOut(tm); boolean finished = exec.awaitTermination(realTimeOut, TimeUnit.MILLISECONDS); @@ -381,21 +391,19 @@ private static void invokeWithTimeoutWithNewExecutor( exec.shutdownNow(); testResult.setThrowable(exception); testResult.setStatus(ITestResult.FAILURE); + return false; } else { Utils.log( "Invoker " + Thread.currentThread().hashCode(), 3, "Method " + tm.getMethodName() + " completed within the time-out " + tm.getTimeOut()); - - // We don't need the result from the future but invoking get() on it - // will trigger the exception that was thrown, if any - try { - future.get(); - } catch (ExecutionException e) { - throw new ThreadExecutionException(e.getCause()); - } - + } + try { + boolean flag = future.get(); testResult.setStatus(ITestResult.SUCCESS); // if no exception till here then SUCCESS. + return flag; + } catch (ExecutionException e) { + throw new ThreadExecutionException(e.getCause()); } } @@ -420,7 +428,7 @@ private static StackTraceElement[] getRunningMethodStackTrace(ExecutorService ex return stackTrace; } - protected static void invokeConfigurable( + protected static boolean invokeConfigurable( final Object instance, final Object[] parameters, final IConfigurable configurableInstance, @@ -428,12 +436,14 @@ protected static void invokeConfigurable( final ITestResult testResult) throws Throwable { final Throwable[] error = new Throwable[1]; + AtomicReference wasCalled = new AtomicReference<>(false); IConfigureCallBack callback = new IConfigureCallBack() { @Override public void runConfigurationMethod(ITestResult tr) { try { + wasCalled.set(true); invokeMethod(thisMethod, instance, parameters); error[0] = null; tr.setThrowable(null); @@ -452,5 +462,6 @@ public Object[] getParameters() { if (error[0] != null) { throw error[0]; } + return wasCalled.get(); } } diff --git a/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java index 25a6f15bad..f4a4ce4262 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java @@ -40,6 +40,7 @@ import org.testng.SuiteRunner; import org.testng.TestException; import org.testng.TestNGException; +import org.testng.TestNotInvokedException; import org.testng.collections.CollectionUtils; import org.testng.collections.Lists; import org.testng.collections.Maps; @@ -660,28 +661,40 @@ private ITestResult invokeMethod( ? (IHookable) arguments.getInstance() : m_configuration.getHookable(); + boolean willfullyIgnored = false; + boolean usesHookableInstance = hookableInstance != null; if (MethodHelper.calculateTimeOut(arguments.getTestMethod()) <= 0) { - if (hookableInstance != null) { - MethodInvocationHelper.invokeHookable( - arguments.getInstance(), - arguments.getParameterValues(), - hookableInstance, - thisMethod, - testResult); + if (usesHookableInstance) { + willfullyIgnored = + !MethodInvocationHelper.invokeHookable( + arguments.getInstance(), + arguments.getParameterValues(), + hookableInstance, + thisMethod, + testResult); } else { // Not a IHookable, invoke directly MethodInvocationHelper.invokeMethod( thisMethod, arguments.getInstance(), arguments.getParameterValues()); } - setTestStatus(testResult, ITestResult.SUCCESS); + if (!willfullyIgnored) { + setTestStatus(testResult, ITestResult.SUCCESS); + } } else { // Method with a timeout - MethodInvocationHelper.invokeWithTimeout( - arguments.getTestMethod(), - arguments.getInstance(), - arguments.getParameterValues(), - testResult, - hookableInstance); + willfullyIgnored = + !MethodInvocationHelper.invokeWithTimeout( + arguments.getTestMethod(), + arguments.getInstance(), + arguments.getParameterValues(), + testResult, + hookableInstance); + } + boolean testStatusRemainedUnchanged = testResult.isNotRunning(); + if (usesHookableInstance && willfullyIgnored && testStatusRemainedUnchanged) { + TestNotInvokedException tn = new TestNotInvokedException(arguments.tm); + testResult.setThrowable(tn); + setTestStatus(testResult, ITestResult.FAILURE); } } catch (InvocationTargetException ite) { testResult.setThrowable(ite.getCause()); diff --git a/testng-core/src/test/java/test/hook/HookableTest.java b/testng-core/src/test/java/test/hook/HookableTest.java index df7a7008a1..f46b23cf2f 100644 --- a/testng-core/src/test/java/test/hook/HookableTest.java +++ b/testng-core/src/test/java/test/hook/HookableTest.java @@ -6,6 +6,7 @@ import java.util.ArrayList; import java.util.List; import java.util.UUID; +import java.util.function.Consumer; import java.util.stream.Collectors; import org.assertj.core.api.SoftAssertions; import org.testng.IInvokedMethod; @@ -17,12 +18,15 @@ import org.testng.annotations.Test; import test.SimpleBaseTest; import test.hook.samples.ConfigurableFailureSample; +import test.hook.samples.ConfigurableFailureWithStatusAlteredSample; import test.hook.samples.ConfigurableSuccessSample; import test.hook.samples.ConfigurableSuccessWithListenerSample; import test.hook.samples.HookFailureSample; +import test.hook.samples.HookFailureWithStatusAlteredSample; import test.hook.samples.HookSuccessDynamicParametersSample; import test.hook.samples.HookSuccessSample; import test.hook.samples.HookSuccessTimeoutSample; +import test.hook.samples.HookSuccessTimeoutWithDataProviderSample; import test.hook.samples.HookSuccessWithListenerSample; public class HookableTest extends SimpleBaseTest { @@ -49,9 +53,11 @@ public void hookSuccess(Class clazz, String flow, boolean assertAttributes) { each -> { assertions.assertThat(each.getAttribute(HOOK_INVOKED_ATTRIBUTE)).isNotNull(); assertions.assertThat(each.getAttribute(HOOK_METHOD_INVOKED_ATTRIBUTE)).isNotNull(); - Object[] parameters = (Object[]) each.getAttribute(HOOK_METHOD_PARAMS_ATTRIBUTE); - assertions.assertThat(parameters).hasSize(1); - assertions.assertThat(parameters[0]).isInstanceOf(UUID.class); + if (each.getMethod().isDataDriven()) { + Object[] parameters = (Object[]) each.getAttribute(HOOK_METHOD_PARAMS_ATTRIBUTE); + assertions.assertThat(parameters).hasSize(1); + assertions.assertThat(parameters[0]).isInstanceOf(UUID.class); + } }); assertions.assertAll(); } @@ -60,7 +66,12 @@ public void hookSuccess(Class clazz, String flow, boolean assertAttributes) { public Object[][] getTestClasses() { return new Object[][] { {HookSuccessSample.class, "Happy Flow", true}, - {HookSuccessTimeoutSample.class, "With Timeouts (GITHUB-599)", true}, + { + HookSuccessTimeoutWithDataProviderSample.class, + "DataProvider Test With Timeouts (GITHUB-599)", + true + }, + {HookSuccessTimeoutSample.class, "Regular test With Timeouts (GITHUB-599)", true}, {HookSuccessDynamicParametersSample.class, "With Dynamic Parameters (GITHUB-862)", false} }; } @@ -81,16 +92,32 @@ public void hookFailure() { TestResultsCollector listener = new TestResultsCollector(); tng.addListener(listener); tng.run(); - assertThat(listener.getPassedMethodNames()).isNotNull(); - SoftAssertions assertions = new SoftAssertions(); - listener - .getInvoked() - .forEach( - each -> { - assertions.assertThat(each.getAttribute(HOOK_INVOKED_ATTRIBUTE)).isNotNull(); - assertions.assertThat(each.getAttribute(HOOK_METHOD_INVOKED_ATTRIBUTE)).isNull(); - }); - assertions.assertAll(); + assertThat(listener.getPassedMethodNames()).isEmpty(); + assertThat(listener.getFailedTests()).hasSize(1); + ITestResult failedTestResult = listener.getFailedTests().get(0); + assertThat(failedTestResult.getAttribute(HOOK_INVOKED_ATTRIBUTE)).isNotNull(); + assertThat(failedTestResult.getAttribute(HOOK_METHOD_INVOKED_ATTRIBUTE)).isNull(); + } + + @Test + public void hookFailureWithStatusAltered() { + TestNG tng = create(HookFailureWithStatusAlteredSample.class); + TestResultsCollector listener = new TestResultsCollector(); + tng.addListener(listener); + tng.run(); + assertThat(listener.getPassedMethodNames()).hasSize(1); + Consumer> verifier = + list -> { + SoftAssertions assertions = new SoftAssertions(); + list.forEach( + each -> { + assertions.assertThat(each.getAttribute(HOOK_INVOKED_ATTRIBUTE)).isNotNull(); + assertions.assertThat(each.getAttribute(HOOK_METHOD_INVOKED_ATTRIBUTE)).isNull(); + }); + assertions.assertAll(); + }; + verifier.accept(listener.getPassed()); + verifier.accept(listener.getInvoked()); } @Test(dataProvider = "getConfigClasses") @@ -133,6 +160,29 @@ public void configurableFailure() { TestResultsCollector listener = new TestResultsCollector(); tng.addListener(listener); tng.run(); + assertThat(listener.getPassedConfigNames()).isEmpty(); + assertThat(listener.getFailedConfigs()).hasSize(1); + ITestResult failedConfigResult = listener.getFailedConfigs().get(0); + assertThat(failedConfigResult.getAttribute(HOOK_INVOKED_ATTRIBUTE)).isNotNull(); + assertThat(failedConfigResult.getAttribute(HOOK_METHOD_INVOKED_ATTRIBUTE)).isNull(); + assertThat(listener.getSkippedConfigs()).hasSize(3); + SoftAssertions assertions = new SoftAssertions(); + listener + .getSkippedConfigs() + .forEach( + each -> { + assertions.assertThat(each.getAttribute(HOOK_INVOKED_ATTRIBUTE)).isNull(); + assertions.assertThat(each.getAttribute(HOOK_METHOD_INVOKED_ATTRIBUTE)).isNull(); + }); + assertions.assertAll(); + } + + @Test + public void configurableFailureWithStatusAltered() { + TestNG tng = create(ConfigurableFailureWithStatusAlteredSample.class); + TestResultsCollector listener = new TestResultsCollector(); + tng.addListener(listener); + tng.run(); assertThat(listener.getPassedConfigNames()).containsExactly("bs", "bt", "bc", "bm"); assertThat(listener.getPassedConfigs()).hasSize(4); SoftAssertions assertions = new SoftAssertions(); @@ -168,6 +218,19 @@ public List getPassedConfigs() { return passedConfigs; } + public List getFailedConfigs() { + return invoked.stream() + .filter(each -> each.getStatus() == ITestResult.FAILURE) + .collect(Collectors.toList()); + } + + public List getSkippedConfigs() { + return invoked.stream() + .filter(each -> !each.getMethod().isTest()) + .filter(each -> each.getStatus() == ITestResult.SKIP) + .collect(Collectors.toList()); + } + public List getPassed() { return passed; } @@ -176,6 +239,13 @@ public List getInvoked() { return invoked; } + public List getFailedTests() { + return invoked.stream() + .filter(each -> each.getMethod().isTest()) + .filter(each -> each.getStatus() == ITestResult.FAILURE) + .collect(Collectors.toList()); + } + public List getPassedMethodNames() { return asString(passed); } diff --git a/testng-core/src/test/java/test/hook/samples/ConfigurableFailureWithStatusAlteredSample.java b/testng-core/src/test/java/test/hook/samples/ConfigurableFailureWithStatusAlteredSample.java new file mode 100644 index 0000000000..1c5dc02e93 --- /dev/null +++ b/testng-core/src/test/java/test/hook/samples/ConfigurableFailureWithStatusAlteredSample.java @@ -0,0 +1,20 @@ +package test.hook.samples; + +import static test.hook.HookableTest.HOOK_INVOKED_ATTRIBUTE; + +import org.testng.IConfigureCallBack; +import org.testng.ITestResult; +import org.testng.annotations.Test; + +public class ConfigurableFailureWithStatusAlteredSample extends BaseConfigurableSample { + + @Override + public void run(IConfigureCallBack callBack, ITestResult testResult) { + testResult.setAttribute(HOOK_INVOKED_ATTRIBUTE, "true"); + // Not calling the callback + testResult.setStatus(ITestResult.SUCCESS); + } + + @Test + public void hookWasNotRun() {} +} diff --git a/testng-core/src/test/java/test/hook/samples/HookFailureWithStatusAlteredSample.java b/testng-core/src/test/java/test/hook/samples/HookFailureWithStatusAlteredSample.java new file mode 100644 index 0000000000..15be531854 --- /dev/null +++ b/testng-core/src/test/java/test/hook/samples/HookFailureWithStatusAlteredSample.java @@ -0,0 +1,25 @@ +package test.hook.samples; + +import static test.hook.HookableTest.HOOK_INVOKED_ATTRIBUTE; +import static test.hook.HookableTest.HOOK_METHOD_INVOKED_ATTRIBUTE; + +import org.testng.IHookCallBack; +import org.testng.IHookable; +import org.testng.ITestResult; +import org.testng.Reporter; +import org.testng.annotations.Test; + +public class HookFailureWithStatusAlteredSample implements IHookable { + + @Override + public void run(IHookCallBack callBack, ITestResult testResult) { + testResult.setAttribute(HOOK_INVOKED_ATTRIBUTE, "true"); + // Not invoking the callback: the method should not be run + testResult.setStatus(ITestResult.SUCCESS); + } + + @Test + public void verify() { + Reporter.getCurrentTestResult().setAttribute(HOOK_METHOD_INVOKED_ATTRIBUTE, "true"); + } +} diff --git a/testng-core/src/test/java/test/hook/samples/HookSuccessTimeoutSample.java b/testng-core/src/test/java/test/hook/samples/HookSuccessTimeoutSample.java index e53706e3eb..29a153ec8d 100644 --- a/testng-core/src/test/java/test/hook/samples/HookSuccessTimeoutSample.java +++ b/testng-core/src/test/java/test/hook/samples/HookSuccessTimeoutSample.java @@ -1,13 +1,14 @@ package test.hook.samples; -import static test.hook.HookableTest.*; +import static test.hook.HookableTest.HOOK_INVOKED_ATTRIBUTE; +import static test.hook.HookableTest.HOOK_METHOD_INVOKED_ATTRIBUTE; +import static test.hook.HookableTest.HOOK_METHOD_PARAMS_ATTRIBUTE; import java.util.UUID; import org.testng.IHookCallBack; import org.testng.IHookable; import org.testng.ITestResult; import org.testng.Reporter; -import org.testng.annotations.DataProvider; import org.testng.annotations.Test; public class HookSuccessTimeoutSample implements IHookable { @@ -19,13 +20,8 @@ public void run(IHookCallBack callBack, ITestResult testResult) { callBack.runTestMethod(testResult); } - @DataProvider - public Object[][] dp() { - return new Object[][] {new Object[] {UUID.randomUUID()}}; - } - - @Test(dataProvider = "dp", timeOut = 100) - public void verify(UUID uuid) { - Reporter.getCurrentTestResult().setAttribute(HOOK_METHOD_INVOKED_ATTRIBUTE, uuid); + @Test(timeOut = 100) + public void verify() { + Reporter.getCurrentTestResult().setAttribute(HOOK_METHOD_INVOKED_ATTRIBUTE, UUID.randomUUID()); } } diff --git a/testng-core/src/test/java/test/hook/samples/HookSuccessTimeoutWithDataProviderSample.java b/testng-core/src/test/java/test/hook/samples/HookSuccessTimeoutWithDataProviderSample.java new file mode 100644 index 0000000000..04194e31f8 --- /dev/null +++ b/testng-core/src/test/java/test/hook/samples/HookSuccessTimeoutWithDataProviderSample.java @@ -0,0 +1,31 @@ +package test.hook.samples; + +import static test.hook.HookableTest.*; + +import java.util.UUID; +import org.testng.IHookCallBack; +import org.testng.IHookable; +import org.testng.ITestResult; +import org.testng.Reporter; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class HookSuccessTimeoutWithDataProviderSample implements IHookable { + + @Override + public void run(IHookCallBack callBack, ITestResult testResult) { + testResult.setAttribute(HOOK_INVOKED_ATTRIBUTE, "true"); + testResult.setAttribute(HOOK_METHOD_PARAMS_ATTRIBUTE, callBack.getParameters()); + callBack.runTestMethod(testResult); + } + + @DataProvider + public Object[][] dp() { + return new Object[][] {new Object[] {UUID.randomUUID()}}; + } + + @Test(dataProvider = "dp", timeOut = 100) + public void verify(UUID uuid) { + Reporter.getCurrentTestResult().setAttribute(HOOK_METHOD_INVOKED_ATTRIBUTE, uuid); + } +}