fix(hooks): don't execute the current scope when an error occurs during a before hook

Author: daantimmer

cucumber_cpp/library/engine/HookExecutor.cpp

@@ -7,6 +7,7 @@
 #include "cucumber_cpp/library/engine/StepInfo.hpp"
 #include <functional>
 #include <set>
+#include <stdexcept>
 #include <string>
 
 namespace cucumber_cpp::library::engine
@@ -18,21 +19,19 @@ namespace cucumber_cpp::library::engine
         constexpr HookPair scenarioHooks{ HookType::before, HookType::after };
         constexpr HookPair stepHooks{ HookType::beforeStep, HookType::afterStep };
 
+        struct HookFailed : std::runtime_error
+        {
+            using runtime_error::runtime_error;
+        };
+
         void ExecuteHook(cucumber_cpp::library::engine::RunnerContext& runnerContext, HookType hook, const std::set<std::string, std::less<>>& tags)
         {
-            try
+            for (const auto& match : HookRegistry::Instance().Query(hook, tags))
             {
-                for (const auto& match : HookRegistry::Instance().Query(hook, tags))
-                {
-                    match.factory(runnerContext)->Execute();
+                match.factory(runnerContext)->Execute();
 
-                    if (runnerContext.ExecutionStatus() != cucumber_cpp::library::engine::Result::passed)
-                        return;
-                }
-            }
-            catch (...)
-            {
-                runnerContext.ExecutionStatus(cucumber_cpp::library::engine::Result::failed);
+                if (runnerContext.ExecutionStatus() != cucumber_cpp::library::engine::Result::passed)
+                    throw HookFailed{ "hook failed" };
             }
         }
     }
@@ -42,12 +41,31 @@ namespace cucumber_cpp::library::engine
         , hookPair{ hookPair }
         , tags{ tags }
     {
-        ExecuteHook(runnerContext, hookPair.before, tags);
+        try
+        {
+            ExecuteHook(runnerContext, hookPair.before, tags);
+        }
+        catch (HookFailed&)
+        {
+            /* This throw ensures no tests are executed in and below the current scope. */
+            throw HookFailed{ "Error during BEFORE_x_HOOK" };
+        }
     }
 
     HookExecutor::ScopedHook::~ScopedHook()
     {
-        ExecuteHook(runnerContext, hookPair.after, tags);
+        try
+        {
+            ExecuteHook(runnerContext, hookPair.after, tags);
+        }
+        catch (HookFailed&)
+        {
+            /*
+            Can't throw from a destructor. Error state will be handled by calling functions.
+
+            Errors during AFTER_ALL hook is not a problem, because no other hooks will be executed.
+            */
+        }
     }
 
     HookExecutor::ProgramScope::ProgramScope(cucumber_cpp::library::engine::ContextManager& contextManager)

cucumber_cpp/library/engine/TestRunner.cpp

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