From 7efcb8f31138f33736af30493543d73ff8164160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Sch=C3=A4fer?= Date: Mon, 2 Sep 2019 16:48:57 +0200 Subject: [PATCH] Change exception handling for TestNG (#312) --- .../tngtech/jgiven/impl/ScenarioExecutor.java | 50 ++++++++++++++++--- .../impl/intercept/StepInterceptorImpl.java | 24 ++++++++- .../jgiven/testng/ScenarioTestListener.java | 30 +++++++---- .../tngtech/jgiven/testng/PendingTest.java | 50 +++++++++++++++++++ .../com/tngtech/jgiven/testng/RetryTest.java | 24 +++++++++ .../com/tngtech/jgiven/testng/TestNgTest.java | 19 ++++++- .../jgiven/junit/JUnitExecutorTest.java | 34 +++++++++++++ .../TestFrameworkExecutionTest.java | 33 ------------ .../tngtech/jgiven/testng/TestNgExecutor.java | 4 +- 9 files changed, 213 insertions(+), 55 deletions(-) create mode 100644 jgiven-testng/src/test/java/com/tngtech/jgiven/testng/PendingTest.java create mode 100644 jgiven-testng/src/test/java/com/tngtech/jgiven/testng/RetryTest.java diff --git a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/ScenarioExecutor.java b/jgiven-core/src/main/java/com/tngtech/jgiven/impl/ScenarioExecutor.java index 40b40724bd..bb75f6f647 100644 --- a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/ScenarioExecutor.java +++ b/jgiven-core/src/main/java/com/tngtech/jgiven/impl/ScenarioExecutor.java @@ -11,6 +11,7 @@ import java.util.List; import java.util.Map; +import com.tngtech.jgiven.report.model.InvocationMode; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -67,10 +68,27 @@ public enum State { protected final StageTransitionHandler stageTransitionHandler = new StageTransitionHandlerImpl(); protected final StepInterceptorImpl methodInterceptor = new StepInterceptorImpl( this, listener, stageTransitionHandler ); + /** + * Set if an exception was thrown during the execution of the scenario and + * suppressStepExceptions is true. + */ private Throwable failedException; + private boolean failIfPass; + + /** + * Whether exceptions caught while executing steps should be thrown at the end + * of the scenario. Only relevant if suppressStepExceptions is true, because otherwise + * the exceptions are not caught at all. + */ private boolean suppressExceptions; + /** + * Whether exceptions thrown while executing steps should be suppressed or not. + * Only relevant for normal executions of scenarios. + */ + private boolean suppressStepExceptions = true; + public ScenarioExecutor() { injector.injectValueByType( ScenarioExecutor.class, this ); injector.injectValueByType( CurrentStep.class, new StepAccessImpl() ); @@ -437,8 +455,8 @@ public void failed( Throwable e ) { if( hasFailed() ) { log.error( e.getMessage(), e ); } else { - if( !suppressExceptions ) { - listener.scenarioFailed( e ); + if (!failIfPass) { + listener.scenarioFailed(e); } methodInterceptor.disableMethodExecution(); failedException = e; @@ -468,9 +486,12 @@ public void startScenario( Class testClass, Method method, List testClass, Method method, List namedArguments = ParameterNameUtil.mapArgumentsWithParameterNames( paramMethod, @@ -222,7 +237,12 @@ private void handleThrowable( Throwable t ) throws Throwable { } listener.stepMethodFailed( t ); + scenarioExecutor.failed( t ); + + if (!suppressExceptions) { + throw t; + } } private void handleMethodFinished( long durationInNanos, boolean hasNestedSteps ) { diff --git a/jgiven-testng/src/main/java/com/tngtech/jgiven/testng/ScenarioTestListener.java b/jgiven-testng/src/main/java/com/tngtech/jgiven/testng/ScenarioTestListener.java index c74f98e446..a43fa11355 100644 --- a/jgiven-testng/src/main/java/com/tngtech/jgiven/testng/ScenarioTestListener.java +++ b/jgiven-testng/src/main/java/com/tngtech/jgiven/testng/ScenarioTestListener.java @@ -5,11 +5,10 @@ import java.lang.reflect.Method; import java.util.List; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; -import org.testng.ITestContext; -import org.testng.ITestListener; -import org.testng.ITestResult; +import com.google.common.base.Throwables; +import com.tngtech.jgiven.exception.FailIfPassedException; +import org.testng.*; import com.tngtech.jgiven.base.ScenarioTestBase; import com.tngtech.jgiven.impl.ScenarioBase; @@ -46,6 +45,14 @@ public void onTestStart( ITestResult paramITestResult ) { ReportModel reportModel = getReportModel( paramITestResult, instance.getClass() ); scenario.setModel( reportModel ); + + // TestNG does not work well when catching step exceptions, so we have to disable that feature + // this mainly means that steps following a failing step are not reported in JGiven + scenario.getExecutor().setSuppressStepExceptions(false); + + // avoid rethrowing exceptions as they are already thrown by the steps + scenario.getExecutor().setSuppressExceptions(true); + scenario.getExecutor().injectStages( instance ); Method method = paramITestResult.getMethod().getConstructorOrMethod().getMethod(); @@ -90,15 +97,19 @@ public void onTestFailure( ITestResult paramITestResult ) { } @Override - public void onTestSkipped( ITestResult paramITestResult ) {} + public void onTestSkipped( ITestResult testResult ) {} - private void testFinished( ITestResult paramITestResult ) { + private void testFinished( ITestResult testResult ) { try { - ScenarioBase scenario = getScenario( paramITestResult ); + ScenarioBase scenario = getScenario(testResult); scenario.finished(); + } catch( FailIfPassedException ex ) { + testResult.setStatus(ITestResult.FAILURE); + testResult.setThrowable(ex); + testResult.getTestContext().getPassedTests().removeResult(testResult); + testResult.getTestContext().getFailedTests().addResult(testResult, testResult.getMethod()); } catch( Throwable throwable ) { - paramITestResult.setThrowable( throwable ); - paramITestResult.setStatus( ITestResult.FAILURE ); + Throwables.propagate(throwable); } finally { ScenarioHolder.get().removeScenarioOfCurrentThread(); } @@ -128,5 +139,4 @@ private ConcurrentHashMap getReportModels(ITestContext para private List getArgumentsFrom( Method method, ITestResult paramITestResult ) { return ParameterNameUtil.mapArgumentsWithParameterNames( method, asList( paramITestResult.getParameters() ) ); } - } diff --git a/jgiven-testng/src/test/java/com/tngtech/jgiven/testng/PendingTest.java b/jgiven-testng/src/test/java/com/tngtech/jgiven/testng/PendingTest.java new file mode 100644 index 0000000000..3e1df91cb3 --- /dev/null +++ b/jgiven-testng/src/test/java/com/tngtech/jgiven/testng/PendingTest.java @@ -0,0 +1,50 @@ +package com.tngtech.jgiven.testng; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.tngtech.jgiven.annotation.Pending; +import com.tngtech.jgiven.report.model.ExecutionStatus; +import org.assertj.core.api.Assertions; +import org.testng.SkipException; +import org.testng.annotations.Test; + +import com.tngtech.jgiven.annotation.Description; +import com.tngtech.jgiven.report.model.ScenarioCaseModel; +import com.tngtech.jgiven.report.model.StepStatus; + +@Description( "Pending annotation is handled correctly" ) +public class PendingTest extends SimpleScenarioTest { + + @Test + @Pending + public void pending_annotation_should_catch_exceptions() { + given().something(); + when().something_fails(); + then().nothing_happens(); + + ScenarioCaseModel aCase = getScenario().getScenarioCaseModel(); + assertThat( aCase.getExecutionStatus() ).isEqualTo( ExecutionStatus.SCENARIO_PENDING ); + } + + @Test + @Pending(executeSteps = true) + public void pending_annotation_should_catch_exceptions_when_executing_steps() { + given().something(); + when().something_fails(); + then().nothing_happens(); + + ScenarioCaseModel aCase = getScenario().getScenarioCaseModel(); + assertThat( aCase.getExecutionStatus() ).isEqualTo( ExecutionStatus.SCENARIO_PENDING ); + } + + @Test + public void pending_annotation_on_failing_steps_should_catch_exceptions() { + given().something(); + when().something_fails_with_pending_annotation(); + then().nothing_happens(); + + ScenarioCaseModel aCase = getScenario().getScenarioCaseModel(); + assertThat( aCase.getExecutionStatus() ).isEqualTo( ExecutionStatus.SOME_STEPS_PENDING ); + } + +} diff --git a/jgiven-testng/src/test/java/com/tngtech/jgiven/testng/RetryTest.java b/jgiven-testng/src/test/java/com/tngtech/jgiven/testng/RetryTest.java new file mode 100644 index 0000000000..290a8326e1 --- /dev/null +++ b/jgiven-testng/src/test/java/com/tngtech/jgiven/testng/RetryTest.java @@ -0,0 +1,24 @@ +package com.tngtech.jgiven.testng; + +import org.testng.IRetryAnalyzer; +import org.testng.ITestResult; +import org.testng.annotations.Test; + +@Test +public class RetryTest extends SimpleScenarioTest { + + int count = 0; + + @Test(retryAnalyzer = MyAnalyzer.class) + public void failing_with_retry_test() { + when().something_should_$_fail(count++ == 0); + } + + public static class MyAnalyzer implements IRetryAnalyzer { + int count = 0; + @Override + public boolean retry(ITestResult result) { + return count++ == 0; + } + } +} diff --git a/jgiven-testng/src/test/java/com/tngtech/jgiven/testng/TestNgTest.java b/jgiven-testng/src/test/java/com/tngtech/jgiven/testng/TestNgTest.java index 4d2bce7eb4..6dcecab768 100644 --- a/jgiven-testng/src/test/java/com/tngtech/jgiven/testng/TestNgTest.java +++ b/jgiven-testng/src/test/java/com/tngtech/jgiven/testng/TestNgTest.java @@ -4,6 +4,9 @@ import java.util.List; +import com.tngtech.jgiven.annotation.Format; +import com.tngtech.jgiven.annotation.Pending; +import com.tngtech.jgiven.format.NotFormatter; import org.testng.annotations.Test; import com.tngtech.jgiven.Stage; @@ -86,6 +89,18 @@ public TestSteps something_fails() { throw new IllegalStateException( "Something failed" ); } + @Pending + public TestSteps something_fails_with_pending_annotation() { + throw new IllegalStateException( "Something failed" ); + } + + public TestSteps something_should_$_fail(@Format(NotFormatter.class) boolean shouldFail) { + if (shouldFail) { + throw new IllegalStateException("Something failed"); + } + return this; + } + public TestSteps you_get_sugar_milk() { assertThat( result ).isEqualTo( "SugarMilk" ); return this; @@ -113,7 +128,9 @@ public void ingredient( String someIngredient ) { public void mixed_with( String something ) {} - public void something() {} + public TestSteps something() { + return this; + } public void skipped_exception_is_thrown() { throw new org.testng.SkipException( "should be skipped" ); diff --git a/jgiven-tests/src/test/java/com/tngtech/jgiven/junit/JUnitExecutorTest.java b/jgiven-tests/src/test/java/com/tngtech/jgiven/junit/JUnitExecutorTest.java index fe612cf3d2..637c205695 100644 --- a/jgiven-tests/src/test/java/com/tngtech/jgiven/junit/JUnitExecutorTest.java +++ b/jgiven-tests/src/test/java/com/tngtech/jgiven/junit/JUnitExecutorTest.java @@ -38,4 +38,38 @@ public void exception_in_scenario_is_not_hidden_by_exception_in_JUnit_after_meth then().the_test_fails_with_message( "assertion failed in test step" ); } + @Test + public void steps_following_failing_steps_are_reported_as_skipped() { + given().a_failing_test_with_$_steps( 3 ) + .and().step_$_fails( 1 ); + when().the_test_is_executed_with_JUnit(); + then().step_$_is_reported_as_failed( 1 ) + .and().step_$_is_reported_as_skipped( 2 ) + .and().step_$_is_reported_as_skipped( 3 ); + } + + @Test + public void after_stage_methods_of_stages_following_failing_stages_are_ignored() { + given().a_failing_test_with_$_steps( 2 ) + .and().the_test_has_$_failing_stages( 2 ) + .and().stage_$_has_a_failing_after_stage_method( 2 ) + .and().step_$_fails( 1 ); + when().the_test_is_executed_with_JUnit(); + then().the_test_fails() + .and().step_$_is_reported_as_failed( 1 ) + .and().step_$_is_reported_as_skipped( 2 ); + } + + @Test + public void all_steps_of_stages_following_failing_stages_are_ignored() { + given().a_failing_test_with_$_steps( 2 ) + .and().the_test_has_$_failing_stages( 2 ) + .and().step_$_fails( 1 ); + when().the_test_is_executed_with_JUnit(); + then().the_test_fails() + .and().step_$_is_reported_as_failed( 1 ) + .and().step_$_is_reported_as_skipped( 2 ); + } + + } diff --git a/jgiven-tests/src/test/java/com/tngtech/jgiven/testframework/TestFrameworkExecutionTest.java b/jgiven-tests/src/test/java/com/tngtech/jgiven/testframework/TestFrameworkExecutionTest.java index 725d71a6ee..b8f5bf3aba 100644 --- a/jgiven-tests/src/test/java/com/tngtech/jgiven/testframework/TestFrameworkExecutionTest.java +++ b/jgiven-tests/src/test/java/com/tngtech/jgiven/testframework/TestFrameworkExecutionTest.java @@ -84,16 +84,6 @@ public void failing_tests_annotated_with_NotImplementedYet_with_executeSteps_set then().the_test_is_ignored(); } - @Test - public void steps_following_failing_steps_are_reported_as_skipped() { - given().a_failing_test_with_$_steps( 3 ) - .and().step_$_fails( 1 ); - when().the_test_is_executed_with( testFramework ); - then().step_$_is_reported_as_failed( 1 ) - .and().step_$_is_reported_as_skipped( 2 ) - .and().step_$_is_reported_as_skipped( 3 ); - } - @Test public void passing_steps_before_failing_steps_are_reported_as_passed() { given().a_failing_test_with_$_steps( 2 ) @@ -111,29 +101,6 @@ public void the_error_message_of_a_failing_step_is_reported() { .and().an_error_message_is_stored_in_the_report(); } - @Test - public void all_steps_of_stages_following_failing_stages_are_ignored() { - given().a_failing_test_with_$_steps( 2 ) - .and().the_test_has_$_failing_stages( 2 ) - .and().step_$_fails( 1 ); - when().the_test_is_executed_with( testFramework ); - then().the_test_fails() - .and().step_$_is_reported_as_failed( 1 ) - .and().step_$_is_reported_as_skipped( 2 ); - } - - @Test - public void after_stage_methods_of_stages_following_failing_stages_are_ignored() { - given().a_failing_test_with_$_steps( 2 ) - .and().the_test_has_$_failing_stages( 2 ) - .and().stage_$_has_a_failing_after_stage_method( 2 ) - .and().step_$_fails( 1 ); - when().the_test_is_executed_with( testFramework ); - then().the_test_fails() - .and().step_$_is_reported_as_failed( 1 ) - .and().step_$_is_reported_as_skipped( 2 ); - } - @Test @FeatureTags public void tag_annotations_appear_in_the_report_model() { diff --git a/jgiven-tests/src/test/java/com/tngtech/jgiven/testng/TestNgExecutor.java b/jgiven-tests/src/test/java/com/tngtech/jgiven/testng/TestNgExecutor.java index 248b53fe7a..c4e0ee8ed2 100644 --- a/jgiven-tests/src/test/java/com/tngtech/jgiven/testng/TestNgExecutor.java +++ b/jgiven-tests/src/test/java/com/tngtech/jgiven/testng/TestNgExecutor.java @@ -15,6 +15,8 @@ import com.tngtech.jgiven.testframework.TestExecutionResult; import com.tngtech.jgiven.testframework.TestExecutor; +import static com.tngtech.jgiven.testng.ScenarioTestListener.SCENARIO_ATTRIBUTE; + public class TestNgExecutor extends TestExecutor { public static String methodName; @@ -64,7 +66,7 @@ public void onTestSkipped( ITestResult tr ) { private void setTestResult( ITestResult tr ) { testResults.add( tr ); - reportModel = ((ScenarioBase)tr.getAttribute ("jgiven::scenario")).getModel(); + reportModel = ((ScenarioBase)tr.getAttribute (SCENARIO_ATTRIBUTE)).getModel(); } }