Fix issue with @pending(executeSteps = true) that marked a scenario a…

…s failed instead of pending (fixes #402)
TNG · Sep 2, 2019 · 34dab3a · 34dab3a
1 parent 4481185
commit 34dab3a
Show file tree

Hide file tree

Showing 10 changed files with 152 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,9 @@
+# v0.18.1
+
+## Fixed Issues
+
+* Fix issue with @Pending(executeSteps = true) that marked a scenario as failed instead of pending [#402](https://github.com/TNG/JGiven/issues/402)
+
 # v0.18.0
 
 ## New Features

diff --git a/jgiven-core/src/main/java/com/tngtech/jgiven/annotation/Pending.java b/jgiven-core/src/main/java/com/tngtech/jgiven/annotation/Pending.java
@@ -61,6 +61,8 @@
      * <p>
      * This makes sense if one ensures that a not implemented feature
      * always leads to failing tests in the spirit of test-driven development.
+     * The test is completely implemented if no test is failing anymore, which means
+     * that the @Pending annotation can be removed.
      * <p>
      * If this is true, the <code>executeSteps</code> attribute is implicitly <code>true</code>.
      */

diff --git a/jgiven-core/src/main/java/com/tngtech/jgiven/exception/FailIfPassedException.java b/jgiven-core/src/main/java/com/tngtech/jgiven/exception/FailIfPassedException.java
@@ -7,6 +7,6 @@ public class FailIfPassedException extends RuntimeException {
     private static final long serialVersionUID = 1L;
 
     public FailIfPassedException() {
-        super( "Test succeeded, but failIfPassed set to true" );
+        super( "Test succeeded, but failIfPassed set to true. Now might be the right time to remove the @Pending annotation." );
     }
 }
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
@@ -437,7 +437,9 @@ public void failed( Throwable e ) {
         if( hasFailed() ) {
             log.error( e.getMessage(), e );
         } else {
-            listener.scenarioFailed( e );
+            if( !suppressExceptions ) {
+                listener.scenarioFailed( e );
+            }
             methodInterceptor.disableMethodExecution();
             failedException = e;
         }

diff --git a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/intercept/StepInterceptorImpl.java b/jgiven-core/src/main/java/com/tngtech/jgiven/impl/intercept/StepInterceptorImpl.java
@@ -10,6 +10,7 @@
 import java.util.List;
 import java.util.Stack;
 
+import com.tngtech.jgiven.impl.util.ThrowableUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -216,8 +217,7 @@ private void handleMethod( Object stageInstance, Method paramMethod, Object[] ar
     }
 
     private void handleThrowable( Throwable t ) throws Throwable {
-        if( t.getClass().getName().equals( "org.junit.AssumptionViolatedException" ) ||
-            t.getClass().getName().equals( "org.testng.SkipException")) {
+        if( ThrowableUtil.isAssumptionException(t) ) {
             throw t;
         }
 

diff --git a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/util/ThrowableUtil.java b/jgiven-core/src/main/java/com/tngtech/jgiven/impl/util/ThrowableUtil.java
@@ -0,0 +1,8 @@
+package com.tngtech.jgiven.impl.util;
+
+public class ThrowableUtil {
+    public static boolean isAssumptionException(Throwable t) {
+        return t.getClass().getName().equals( "org.junit.AssumptionViolatedException" )
+            || t.getClass().getName().equals( "org.testng.SkipException");
+    }
+}
diff --git a/jgiven-junit/src/test/java/com/tngtech/jgiven/junit/PendingTest.java b/jgiven-junit/src/test/java/com/tngtech/jgiven/junit/PendingTest.java
@@ -4,6 +4,8 @@
 
 import com.tngtech.jgiven.annotation.ExpectedScenarioState;
 import com.tngtech.jgiven.annotation.Pending;
+import com.tngtech.jgiven.annotation.ScenarioStage;
+import com.tngtech.jgiven.report.model.ExecutionStatus;
 import org.assertj.core.api.Assertions;
 import org.junit.AssumptionViolatedException;
 import org.junit.Ignore;
@@ -18,19 +20,64 @@
 
 public class PendingTest extends SimpleScenarioTest<PendingTest.PendingTestSteps> {
 
+    @ScenarioStage
+    PendingTest.PendingTestStepsWithRequiredField pendingTestStepsWithRequiredField;
+
     @Test
     @Pending
-    public void required_does_not_fail_for_pending_scenarios() throws Throwable {
-        when().some_action();
+    public void required_does_not_fail_for_pending_scenarios() {
+        pendingTestStepsWithRequiredField.some_action();
+
+        assertThat( getScenario().getScenarioCaseModel().getExecutionStatus() ).isEqualTo(ExecutionStatus.SCENARIO_PENDING);
     }
 
-    public static class PendingTestSteps {
+    @Test
+    @Pending(executeSteps = true)
+    public void failing_steps_are_reported_as_pending_if_execute_steps_is_true() {
+        when().some_failing_action();
+
+        assertThat( getScenario().getScenarioCaseModel().getExecutionStatus() ).isEqualTo(ExecutionStatus.SCENARIO_PENDING);
+    }
+
+    @Test
+    public void failing_steps_are_reported_as_pending_if_execute_steps_is_true_on_step_method() {
+        when().some_failing_action_with_pending_annotation();
+
+        assertThat( getScenario().getScenarioCaseModel().getExecutionStatus() ).isEqualTo(ExecutionStatus.SCENARIO_PENDING);
+    }
+
+    @Test
+    @Pending(failIfPass = true)
+    public void failing_tests_with_failIfPass_are_reported_as_pending() {
+        when().some_failing_action();
+
+        assertThat( getScenario().getScenarioCaseModel().getExecutionStatus() ).isEqualTo(ExecutionStatus.SCENARIO_PENDING);
+    }
+
+    public static class PendingTestStepsWithRequiredField {
         @ExpectedScenarioState(required = true)
         String someState;
 
-        public void some_action() {
+        public PendingTestStepsWithRequiredField some_action() {
+            return this;
+        }
+    }
+
+    public static class PendingTestSteps {
+        @ExpectedScenarioState
+        String someState;
+
+        public PendingTestSteps some_failing_action() {
+            assertThat(someState).isNotNull();
+            return this;
+        }
+
+        @Pending(executeSteps = true)
+        public PendingTestSteps some_failing_action_with_pending_annotation() {
             assertThat(someState).isNotNull();
+            return this;
         }
+
     }
 
 }
diff --git a/jgiven-junit5/src/test/java/com/tngtech/jgiven/junit5/test/PendingTest.java b/jgiven-junit5/src/test/java/com/tngtech/jgiven/junit5/test/PendingTest.java
@@ -0,0 +1,74 @@
+package com.tngtech.jgiven.junit5.test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.tngtech.jgiven.annotation.ScenarioStage;
+import com.tngtech.jgiven.junit5.SimpleScenarioTest;
+
+import com.tngtech.jgiven.annotation.ExpectedScenarioState;
+import com.tngtech.jgiven.annotation.Pending;
+import com.tngtech.jgiven.report.model.ExecutionStatus;
+import org.junit.jupiter.api.Test;
+
+public class PendingTest extends SimpleScenarioTest<PendingTest.PendingTestSteps> {
+
+    @ScenarioStage
+    PendingTest.PendingTestStepsWithRequiredField pendingTestStepsWithRequiredField;
+
+    @Test
+    @Pending
+    public void required_does_not_fail_for_pending_scenarios() {
+        pendingTestStepsWithRequiredField.some_action();
+
+        assertThat( getScenario().getScenarioCaseModel().getExecutionStatus() ).isEqualTo(ExecutionStatus.SCENARIO_PENDING);
+    }
+
+    @Test
+    @Pending(executeSteps = true)
+    public void failing_steps_are_reported_as_pending_if_execute_steps_is_true() {
+        when().some_failing_action();
+
+        assertThat( getScenario().getScenarioCaseModel().getExecutionStatus() ).isEqualTo(ExecutionStatus.SCENARIO_PENDING);
+    }
+
+    @Test
+    public void failing_steps_are_reported_as_pending_if_execute_steps_is_true_on_step_method() {
+        when().some_failing_action_with_pending_annotation();
+
+        assertThat( getScenario().getScenarioCaseModel().getExecutionStatus() ).isEqualTo(ExecutionStatus.SCENARIO_PENDING);
+    }
+
+    @Test
+    @Pending(failIfPass = true)
+    public void failing_tests_with_failIfPass_are_reported_as_pending() {
+        when().some_failing_action();
+
+        assertThat( getScenario().getScenarioCaseModel().getExecutionStatus() ).isEqualTo(ExecutionStatus.SCENARIO_PENDING);
+    }
+
+    public static class PendingTestStepsWithRequiredField {
+        @ExpectedScenarioState(required = true)
+        String someState;
+
+        public PendingTestStepsWithRequiredField some_action() {
+            return this;
+        }
+    }
+
+    public static class PendingTestSteps {
+        @ExpectedScenarioState
+        String someState;
+
+        public PendingTestSteps some_failing_action() {
+            assertThat(someState).isNotNull();
+            return this;
+        }
+
+        @Pending(executeSteps = true)
+        public PendingTestSteps some_failing_action_with_pending_annotation() {
+            assertThat(someState).isNotNull();
+            return this;
+        }
+
+    }
+}
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
@@ -60,7 +60,7 @@ public void passing_tests_annotated_with_NotImplementedYet_with_failIfPassed_set
             .and().the_test_is_annotated_with_NotImplementedYet()
             .with().failIfPassed_set_to_true();
         when().the_test_is_executed_with( testFramework );
-        then().the_test_fails_with_message( "Test succeeded, but failIfPassed set to true" );
+        then().the_test_fails_with_message( "Test succeeded, but failIfPassed set to true. Now might be the right time to remove the @Pending annotation." );
     }
 
     @Test

diff --git a/jgiven-tests/src/test/java/com/tngtech/jgiven/testframework/ThenTestFramework.java b/jgiven-tests/src/test/java/com/tngtech/jgiven/testframework/ThenTestFramework.java
@@ -19,16 +19,18 @@ public class ThenTestFramework<SELF extends ThenTestFramework<?>> extends ThenRe
     @ExpectedScenarioState
     TestExecutionResult result;
 
-    public void the_test_is_ignored() {
+    public SELF the_test_is_ignored() {
         // this is actually not correct, because it depends on the JUnit executor whether
         // a test is ignored if an AssumptionException is thrown.
         // The standard JUnit executor will report the test as passed and not ignored,
         // we thus only test for not failed here
         the_test_passes();
+        return self();
     }
 
-    public void the_test_passes() {
+    public SELF the_test_passes() {
         assertThat( result.getFailureCount() ).as( "failure count" ).isEqualTo( 0 );
+        return self();
     }
 
     public SELF $_tests_fail( int nFailedTests ) {