Change exception handling for TestNG (#312)

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,21 +486,29 @@ public void startScenario( Class<?> testClass, Method method, List<NamedArgument
 
             if( annotation.failIfPass() ) {
                 failIfPass();
-            } else if( !annotation.executeSteps() ) {
-                methodInterceptor.disableMethodExecution();
-                executeLifeCycleMethods = false;
+            } else {
+                methodInterceptor.setDefaultInvocationMode(InvocationMode.PENDING);
+                if( !annotation.executeSteps() ) {
+                    methodInterceptor.disableMethodExecution();
+                    executeLifeCycleMethods = false;
+                }
             }
             suppressExceptions = true;
         } else if( method.isAnnotationPresent( NotImplementedYet.class ) ) {
             NotImplementedYet annotation = method.getAnnotation( NotImplementedYet.class );
 
             if( annotation.failIfPass() ) {
                 failIfPass();
-            } else if( !annotation.executeSteps() ) {
-                methodInterceptor.disableMethodExecution();
-                executeLifeCycleMethods = false;
+            } else {
+                methodInterceptor.setDefaultInvocationMode(InvocationMode.PENDING);
+                if( !annotation.executeSteps() ) {
+                    methodInterceptor.disableMethodExecution();
+                    executeLifeCycleMethods = false;
+                }
             }
             suppressExceptions = true;
+        } else {
+            methodInterceptor.setSuppressExceptions(suppressStepExceptions);
         }
 
     }
@@ -496,6 +522,14 @@ public void failIfPass() {
         failIfPass = true;
     }
 
+    public void setSuppressStepExceptions(boolean suppressStepExceptions) {
+        this.suppressStepExceptions = suppressStepExceptions;
+    }
+
+    public void setSuppressExceptions( boolean suppressExceptions ) {
+        this.suppressExceptions = suppressExceptions;
+    }
+
     public void addSection( String sectionTitle ) {
         listener.sectionAdded( sectionTitle );
     }

jgiven-core/src/main/java/com/tngtech/jgiven/impl/intercept/StepInterceptorImpl.java

@@ -43,6 +43,8 @@ public class StepInterceptorImpl implements StepInterceptor {
 
     private int maxStepDepth = INITIAL_MAX_STEP_DEPTH;
 
+    private InvocationMode defaultInvocationMode = InvocationMode.NORMAL;
+
     /**
      * Whether methods should be intercepted or not
      */
@@ -53,6 +55,11 @@ public class StepInterceptorImpl implements StepInterceptor {
      */
     private boolean methodExecutionEnabled = true;
 
+    /**
+     * Whether all exceptions should be suppressed and not be rethrown
+     */
+    private boolean suppressExceptions = true;
+
     public StepInterceptorImpl(ScenarioExecutor scenarioExecutor, ScenarioListener listener, StageTransitionHandler stageTransitionHandler) {
         this.scenarioExecutor = scenarioExecutor;
         this.listener = listener;
@@ -191,7 +198,7 @@ protected InvocationMode getInvocationMode( Object receiver, Method method ) {
             return PENDING;
         }
 
-        return NORMAL;
+        return defaultInvocationMode;
     }
 
     public void enableMethodInterception(boolean b ) {
@@ -208,7 +215,15 @@ public boolean enableMethodExecution( boolean b ) {
         return previousMethodExecution;
     }
 
-    private void handleMethod( Object stageInstance, Method paramMethod, Object[] arguments, InvocationMode mode,
+    public void setSuppressExceptions(boolean b) {
+        suppressExceptions = b;
+    }
+
+    public void setDefaultInvocationMode(InvocationMode defaultInvocationMode) {
+        this.defaultInvocationMode = defaultInvocationMode;
+    }
+
+    private void handleMethod(Object stageInstance, Method paramMethod, Object[] arguments, InvocationMode mode,
                               boolean hasNestedSteps ) throws Throwable {
 
         List<NamedArgument> 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 ) {

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<String, ReportModel> getReportModels(ITestContext para
     private List<NamedArgument> getArgumentsFrom( Method method, ITestResult paramITestResult ) {
         return ParameterNameUtil.mapArgumentsWithParameterNames( method, asList( paramITestResult.getParameters() ) );
     }
-
 }

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<TestNgTest.TestSteps> {
+
+    @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 );
+    }
+
+}

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<TestNgTest.TestSteps> {
+
+    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;
+        }
+    }
+}

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" );

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 );
+    }
+
+
 }