refactor hook execution error handling to allow skipping tests

Author: daantimmer

cucumber_cpp/acceptance_test/MainCustom.cpp

@@ -5,6 +5,7 @@ int main(int argc, char** argv)
     cucumber_cpp::Application application{};
 
     application.CliParser().add_flag("--required", *application.ProgramContext().EmplaceAt<bool>("--required"))->required();
+    application.CliParser().add_flag("--failprogramhook", *application.ProgramContext().EmplaceAt<bool>("--failprogramhook"));
 
     return application.Run(argc, argv);
 }

cucumber_cpp/acceptance_test/hooks/Hooks.cpp

@@ -6,6 +6,9 @@
 HOOK_BEFORE_ALL()
 {
     std::cout << "HOOK_BEFORE_ALL\n";
+
+    if (context.Contains("--failprogramhook") && context.Get<bool>("--failprogramhook"))
+        ASSERT_THAT(false, testing::IsTrue());
 }
 
 HOOK_AFTER_ALL()
@@ -47,3 +50,9 @@ HOOK_BEFORE_SCENARIO("@throw_scenariohook")
 {
     throw std::string{ "error" };
 }
+
+HOOK_BEFORE_SCENARIO()
+{
+    if (context.Contains("--failprogramhook") && context.Get<bool>("--failprogramhook"))
+        std::cout << "should not be executed\n";
+}

cucumber_cpp/acceptance_test/test.bats

@@ -176,3 +176,12 @@ teardown() {
     assert_output --partial "skipped Given a given step"
     assert_output --partial "tests   : 0/1 passed"
 }
+
+
+@test "Test error program hook results in error and skipped steps" {
+    run .build/Host/cucumber_cpp/acceptance_test/Debug/cucumber_cpp.acceptance_test.custom run --feature cucumber_cpp/acceptance_test/features  --tag "@smoke and @result:OK" --report console --required --failprogramhook
+    assert_failure
+    assert_output --partial "skipped Given a given step"
+    refute_output --partial "should not be executed"
+    # assert_output --partial "tests   : 0/1 passed"
+}

cucumber_cpp/library/Application.cpp

@@ -224,7 +224,7 @@ namespace cucumber_cpp::library
 
     int Application::GetExitCode() const
     {
-        if (contextManager.ProgramContext().ExecutionStatus() == engine::Result::passed)
+        if (contextManager.ProgramContext().EffectiveExecutionStatus() == engine::Result::passed)
             return 0;
         else
             return 1;

cucumber_cpp/library/engine/ContextManager.cpp

@@ -5,6 +5,7 @@
 #include "cucumber_cpp/library/engine/RuleInfo.hpp"
 #include "cucumber_cpp/library/engine/ScenarioInfo.hpp"
 #include "cucumber_cpp/library/engine/StepInfo.hpp"
+#include <algorithm>
 #include <memory>
 #include <string>
 #include <string_view>
@@ -48,11 +49,29 @@ namespace cucumber_cpp::library::engine
         Start();
     }
 
+    [[nodiscard]] Result CurrentContext::InheritedExecutionStatus() const
+    {
+        if (parent == nullptr)
+            return executionStatus;
+        else
+            return std::max(executionStatus, parent->InheritedExecutionStatus());
+    }
+
+    [[nodiscard]] Result CurrentContext::EffectiveExecutionStatus() const
+    {
+        return std::max(executionStatus, nestedExecutionStatus);
+    }
+
     [[nodiscard]] Result CurrentContext::ExecutionStatus() const
     {
         return executionStatus;
     }
 
+    [[nodiscard]] Result CurrentContext::NestedExecutionStatus() const
+    {
+        return nestedExecutionStatus;
+    }
+
     void CurrentContext::Start()
     {
         traceTime.Start();
@@ -69,7 +88,16 @@ namespace cucumber_cpp::library::engine
             executionStatus = result;
 
         if (parent != nullptr)
-            parent->ExecutionStatus(result);
+            parent->NestedExecutionStatus(result);
+    }
+
+    void CurrentContext::NestedExecutionStatus(Result result)
+    {
+        if (result > nestedExecutionStatus)
+            nestedExecutionStatus = result;
+
+        if (parent != nullptr)
+            parent->NestedExecutionStatus(result);
     }
 
     ProgramContext::ProgramContext(std::shared_ptr<ContextStorageFactory> contextStorageFactory)
@@ -189,14 +217,11 @@ namespace cucumber_cpp::library::engine
     ContextManager::ScopedStepContext ContextManager::CreateStepContext(const StepInfo& stepInfo)
     {
         stepContext.push(std::make_shared<cucumber_cpp::library::engine::StepContext>(*scenarioContext, stepInfo));
-        runnerContext.push(stepContext.top());
-
         return ScopedStepContext{ *this };
     }
 
     void ContextManager::DisposeStepContext()
     {
-        runnerContext.pop();
         stepContext.pop();
     }
 
@@ -212,4 +237,12 @@ namespace cucumber_cpp::library::engine
     {
         return *runnerContext.top();
     }
+
+    cucumber_cpp::library::engine::RunnerContext* ContextManager::CurrentStepContext()
+    {
+        if (stepContext.empty())
+            return nullptr;
+
+        return stepContext.top().get();
+    }
 }

cucumber_cpp/library/engine/ContextManager.hpp

@@ -19,7 +19,10 @@ namespace cucumber_cpp::library::engine
         explicit CurrentContext(std::shared_ptr<ContextStorageFactory> contextStorageFactory);
         explicit CurrentContext(CurrentContext* parent);
 
+        [[nodiscard]] Result InheritedExecutionStatus() const;
+        [[nodiscard]] Result EffectiveExecutionStatus() const;
         [[nodiscard]] Result ExecutionStatus() const;
+        [[nodiscard]] Result NestedExecutionStatus() const;
 
         void Start();
         [[nodiscard]] TraceTime::Duration Duration() const;
@@ -28,9 +31,12 @@ namespace cucumber_cpp::library::engine
         void ExecutionStatus(Result result);
 
     private:
+        void NestedExecutionStatus(Result result);
+
         CurrentContext* parent{ nullptr };
 
         Result executionStatus{ Result::passed };
+        Result nestedExecutionStatus{ Result::passed };
         TraceTime traceTime;
     };
 
@@ -93,6 +99,7 @@ namespace cucumber_cpp::library::engine
         cucumber_cpp::library::engine::StepContext& StepContext();
 
         cucumber_cpp::library::engine::RunnerContext& CurrentContext();
+        cucumber_cpp::library::engine::RunnerContext* CurrentStepContext();
 
     private:
         void DisposeFeatureContext();

cucumber_cpp/library/engine/FailureHandler.cpp

@@ -53,6 +53,9 @@ namespace cucumber_cpp::library::engine
 
         contextManager.CurrentContext().ExecutionStatus(cucumber_cpp::library::engine::Result::failed);
 
+        if (auto* stepContext = contextManager.CurrentStepContext(); stepContext != nullptr)
+            stepContext->ExecutionStatus(cucumber_cpp::library::engine::Result::failed);
+
         reportHandler.Failure(message, relativeFilePath, line);
     }
 

cucumber_cpp/library/engine/HookExecutor.cpp

@@ -56,13 +56,14 @@ namespace cucumber_cpp::library::engine
 
         void ExecuteHook(cucumber_cpp::library::engine::RunnerContext& runnerContext, HookType hook, const std::set<std::string, std::less<>>& tags, const ThrowPolicy& throwPolicy)
         {
-            for (const auto& match : HookRegistry::Instance().Query(hook, tags))
-            {
-                match.factory(runnerContext)->Execute();
-
-                if (runnerContext.ExecutionStatus() != cucumber_cpp::library::engine::Result::passed)
-                    throwPolicy.Throw();
-            }
+            if (runnerContext.InheritedExecutionStatus() == Result::passed)
+                for (const auto& match : HookRegistry::Instance().Query(hook, tags))
+                {
+                    match.factory(runnerContext)->Execute();
+
+                    if (runnerContext.ExecutionStatus() != cucumber_cpp::library::engine::Result::passed)
+                        return;
+                }
         }
     }
 

cucumber_cpp/library/engine/TestExecution.cpp

@@ -84,7 +84,8 @@ namespace cucumber_cpp::library::engine
     void TestExecutionImpl::RunStep(const cucumber_cpp::library::engine::StepInfo& stepInfo)
     {
         auto scopedContext = contextManager.CreateStepContext(stepInfo);
-        if (contextManager.ScenarioContext().ExecutionStatus() == cucumber_cpp::library::engine::Result::passed)
+        if (contextManager.ScenarioContext().InheritedExecutionStatus() == Result::passed &&
+            contextManager.ScenarioContext().EffectiveExecutionStatus() == Result::passed)
         {
             const auto scopedStepReport = reportHandler.StepStart();
             const auto scopedStepHook = hookExecution.StepStart();

cucumber_cpp/library/engine/TestRunner.cpp

@@ -1,6 +1,5 @@
 
 #include "cucumber_cpp/library/engine/TestRunner.hpp"
-#include "cucumber_cpp/library/BodyMacro.hpp"
 #include "cucumber_cpp/library/engine/FeatureFactory.hpp"
 #include "cucumber_cpp/library/engine/FeatureInfo.hpp"
 #include "cucumber_cpp/library/engine/RuleInfo.hpp"
@@ -38,15 +37,10 @@ namespace cucumber_cpp::library::engine
 
     void TestRunnerImpl::Run(const std::vector<std::unique_ptr<FeatureInfo>>& features)
     {
-        auto runFeatures = [this, &features]
-        {
-            auto scope = testExecution.StartRun();
+        auto scope = testExecution.StartRun();
 
-            for (const auto& featurePtr : features)
-                RunFeature(*featurePtr);
-        };
-
-        ASSERT_NO_THROW(runFeatures());
+        for (const auto& featurePtr : features)
+            RunFeature(*featurePtr);
     }
 
     void TestRunnerImpl::NestedStep(StepType type, std::string step)
@@ -57,28 +51,20 @@ namespace cucumber_cpp::library::engine
 
     void TestRunnerImpl::RunFeature(const FeatureInfo& feature)
     {
-        auto runFeature = [this, &feature]
-        {
-            if (feature.Rules().empty() && feature.Scenarios().empty())
-                return;
+        if (feature.Rules().empty() && feature.Scenarios().empty())
+            return;
 
-            const auto featureScope = testExecution.StartFeature(feature);
+        const auto featureScope = testExecution.StartFeature(feature);
 
-            RunRules(feature.Rules());
-            RunScenarios(feature.Scenarios());
-        };
-        ASSERT_NO_THROW(runFeature());
+        RunRules(feature.Rules());
+        RunScenarios(feature.Scenarios());
     }
 
     void TestRunnerImpl::RunRule(const RuleInfo& rule)
     {
-        auto runRule = [this, &rule]
-        {
-            const auto ruleScope = testExecution.StartRule(rule);
+        const auto ruleScope = testExecution.StartRule(rule);
 
-            RunScenarios(rule.Scenarios());
-        };
-        ASSERT_NO_THROW(runRule());
+        RunScenarios(rule.Scenarios());
     }
 
     void TestRunnerImpl::RunRules(const std::vector<std::unique_ptr<RuleInfo>>& rules)
@@ -89,15 +75,11 @@ namespace cucumber_cpp::library::engine
 
     void TestRunnerImpl::RunScenario(const ScenarioInfo& scenario)
     {
-        auto runScenario = [this, &scenario]
-        {
-            const auto scenarioScope = testExecution.StartScenario(scenario);
+        const auto scenarioScope = testExecution.StartScenario(scenario);
 
-            currentScenario = &scenario;
+        currentScenario = &scenario;
 
-            ExecuteSteps(scenario);
-        };
-        ASSERT_NO_THROW(runScenario());
+        ExecuteSteps(scenario);
     }
 
     void TestRunnerImpl::RunScenarios(const std::vector<std::unique_ptr<ScenarioInfo>>& scenarios)