refactor: update HookExecutor and TestExecution to use optional scopes for feature and scenario starts

Author: daantimmer

cucumber_cpp/acceptance_test/test.bats

@@ -183,7 +183,7 @@ teardown() {
     assert_failure
     assert_output --partial "skipped Given a given step"
     refute_output --partial "should not be executed"
-    # assert_output --partial "tests   : 0/1 passed"
+    assert_output --partial "tests   : 0/1 passed"
 }
 
 @test "Test unicode" {

cucumber_cpp/library/engine/ContextManager.hpp

@@ -57,7 +57,10 @@ namespace cucumber_cpp::library::engine
         NestedContext(RunnerContext& parent, const T& info)
             : RunnerContext{ &parent }
             , info{ info }
-        {}
+        {
+            if (parent.InheritedExecutionStatus() != Result::passed)
+                ExecutionStatus(Result::skipped);
+        }
 
         const T& info;
     };

cucumber_cpp/library/engine/HookExecutor.cpp

@@ -6,6 +6,7 @@
 #include "cucumber_cpp/library/engine/ScenarioInfo.hpp"
 #include "cucumber_cpp/library/engine/StepInfo.hpp"
 #include <functional>
+#include <optional>
 #include <set>
 #include <stdexcept>
 #include <string>
@@ -72,14 +73,18 @@ namespace cucumber_cpp::library::engine
         return ProgramScope{ contextManager };
     }
 
-    HookExecutor::FeatureScope HookExecutorImpl::FeatureStart()
+    std::optional<HookExecutor::FeatureScope> HookExecutorImpl::FeatureStart(cucumber_cpp::library::engine::ContextManager& contextManager)
     {
-        return FeatureScope{ contextManager };
+        if (contextManager.FeatureContext().InheritedExecutionStatus() != Result::passed)
+            return std::nullopt;
+        return std::make_optional<FeatureScope>(contextManager);
     }
 
-    HookExecutor::ScenarioScope HookExecutorImpl::ScenarioStart()
+    std::optional<HookExecutor::ScenarioScope> HookExecutorImpl::ScenarioStart(cucumber_cpp::library::engine::ContextManager& contextManager)
     {
-        return ScenarioScope{ contextManager };
+        if (contextManager.ScenarioContext().InheritedExecutionStatus() != Result::passed)
+            return std::nullopt;
+        return std::make_optional<ScenarioScope>(contextManager);
     }
 
     HookExecutor::StepScope HookExecutorImpl::StepStart()

cucumber_cpp/library/engine/HookExecutor.hpp

@@ -5,6 +5,7 @@
 #include "cucumber_cpp/library/engine/ContextManager.hpp"
 #include "cucumber_cpp/library/util/Immoveable.hpp"
 #include <functional>
+#include <optional>
 #include <set>
 #include <string>
 
@@ -23,8 +24,8 @@ namespace cucumber_cpp::library::engine
         struct StepScope;
 
         [[nodiscard]] virtual ProgramScope BeforeAll() = 0;
-        [[nodiscard]] virtual FeatureScope FeatureStart() = 0;
-        [[nodiscard]] virtual ScenarioScope ScenarioStart() = 0;
+        [[nodiscard]] virtual std::optional<FeatureScope> FeatureStart(cucumber_cpp::library::engine::ContextManager& contextManager) = 0;
+        [[nodiscard]] virtual std::optional<ScenarioScope> ScenarioStart(cucumber_cpp::library::engine::ContextManager& contextManager) = 0;
         [[nodiscard]] virtual StepScope StepStart() = 0;
 
     private:
@@ -75,8 +76,8 @@ namespace cucumber_cpp::library::engine
 
         [[nodiscard]] ProgramScope
         BeforeAll() override;
-        [[nodiscard]] FeatureScope FeatureStart() override;
-        [[nodiscard]] ScenarioScope ScenarioStart() override;
+        [[nodiscard]] std::optional<FeatureScope> FeatureStart(cucumber_cpp::library::engine::ContextManager& contextManager) override;
+        [[nodiscard]] std::optional<ScenarioScope> ScenarioStart(cucumber_cpp::library::engine::ContextManager& contextManager) override;
         [[nodiscard]] StepScope StepStart() override;
 
     private:

cucumber_cpp/library/engine/TestExecution.cpp

@@ -25,7 +25,7 @@ namespace cucumber_cpp::library::engine
     TestExecution::FeatureScope::FeatureScope(cucumber_cpp::library::engine::ContextManager& contextManager, report::ReportForwarder& reportHandler, HookExecutor& hookExecution, const cucumber_cpp::library::engine::FeatureInfo& featureInfo)
         : scopedFeatureContext{ contextManager.CreateFeatureContext(featureInfo) }
         , scopedFeatureReport{ reportHandler.FeatureStart() }
-        , scopedFeatureHook{ hookExecution.FeatureStart() }
+        , scopedFeatureHook{ hookExecution.FeatureStart(contextManager) }
     {
     }
 
@@ -38,7 +38,7 @@ namespace cucumber_cpp::library::engine
     TestExecution::ScenarioScope::ScenarioScope(cucumber_cpp::library::engine::ContextManager& contextManager, report::ReportForwarder& reportHandler, HookExecutor& hookExecution, const cucumber_cpp::library::engine::ScenarioInfo& scenarioInfo)
         : scopedScenarioContext{ contextManager.CreateScenarioContext(scenarioInfo) }
         , scopedScenarioReport{ reportHandler.ScenarioStart() }
-        , scopedScenarioHook{ hookExecution.ScenarioStart() }
+        , scopedScenarioHook{ hookExecution.ScenarioStart(contextManager) }
     {
     }
 

cucumber_cpp/library/engine/TestExecution.hpp

@@ -11,6 +11,7 @@
 #include "cucumber_cpp/library/engine/StepInfo.hpp"
 #include "cucumber_cpp/library/report/Report.hpp"
 #include "cucumber_cpp/library/util/Immoveable.hpp"
+#include <optional>
 #include <variant>
 #include <vector>
 
@@ -53,7 +54,7 @@ namespace cucumber_cpp::library::engine
     private:
         cucumber_cpp::library::engine::ContextManager::ScopedFeatureContext scopedFeatureContext;
         report::ReportForwarder::FeatureScope scopedFeatureReport;
-        HookExecutor::FeatureScope scopedFeatureHook;
+        std::optional<HookExecutor::FeatureScope> scopedFeatureHook;
     };
 
     struct TestExecution::RuleScope : library::util::Immoveable
@@ -72,7 +73,7 @@ namespace cucumber_cpp::library::engine
     private:
         cucumber_cpp::library::engine::ContextManager::ScopedScenarioContext scopedScenarioContext;
         report::ReportForwarder::ScenarioScope scopedScenarioReport;
-        HookExecutor::ScenarioScope scopedScenarioHook;
+        std::optional<HookExecutor::ScenarioScope> scopedScenarioHook;
     };
 
     struct TestExecution::Policy

cucumber_cpp/library/engine/test/TestHookExecutor.cpp

@@ -41,7 +41,7 @@ namespace cucumber_cpp::library::engine
         ASSERT_FALSE(contextManagerInstance->FeatureContext().Contains("hookAfterFeature"));
 
         {
-            auto scope = hookExecutor->FeatureStart();
+            auto scope = hookExecutor->FeatureStart(*contextManagerInstance);
             EXPECT_TRUE(contextManagerInstance->FeatureContext().Contains("hookBeforeFeature"));
         }
         EXPECT_TRUE(contextManagerInstance->FeatureContext().Contains("hookAfterFeature"));
@@ -53,7 +53,7 @@ namespace cucumber_cpp::library::engine
         ASSERT_FALSE(contextManagerInstance->ScenarioContext().Contains("hookAfterScenario"));
 
         {
-            auto scope = hookExecutor->ScenarioStart();
+            auto scope = hookExecutor->ScenarioStart(*contextManagerInstance);
             EXPECT_TRUE(contextManagerInstance->ScenarioContext().Contains("hookBeforeScenario"));
         }
         EXPECT_TRUE(contextManagerInstance->ScenarioContext().Contains("hookAfterScenario"));