fix: catch exceptions while constructing a step fixture

cucumber_cpp/Steps.hpp

@@ -8,6 +8,7 @@
 #include "cucumber_cpp/library/cucumber_expression/MatchRange.hpp"
 #include "cucumber_cpp/library/cucumber_expression/ParameterRegistry.hpp"
 #include "cucumber_cpp/library/engine/ExecutionContext.hpp"
+#include "cucumber_cpp/library/engine/Hook.hpp"
 #include "cucumber_cpp/library/util/DocString.hpp"
 #include "cucumber_cpp/library/util/Table.hpp"
 

cucumber_cpp/acceptance_test/features/test_nested_steps.feature

@@ -1,5 +1,10 @@
-@nested_steps
 Feature: Nested Steps
+    @nested_steps
     Scenario: Call other steps from within a step
         Given a step calls another step with "cucumber"
         Then the stored string is "cucumber"
+
+    @nested_failing_steps
+    Scenario: Call other steps from within a step
+        When a step calls another step that will fail
+        Then this should be skipped

cucumber_cpp/acceptance_test/features/test_step_fixture_failure.feature

@@ -0,0 +1,7 @@
+@fail_step_fixture
+Feature: Simple feature file
+    Rule: Test rule
+        Scenario: Test scenario without failing step fixture
+            Given step fixture does not fail
+        Scenario: Test with failing step fixture
+            Given step fixture fails

cucumber_cpp/acceptance_test/steps/Steps.cpp

@@ -1,4 +1,4 @@
-#include "cucumber_cpp/CucumberCpp.hpp"
+#include "cucumber_cpp/Steps.hpp"
 #include "fmt/format.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -107,7 +107,7 @@ THEN("the exception is caught")
 
 THEN("the next scenario is executed")
 {
-    /* do nothing */
+    // empty
 }
 
 GIVEN("{int} and {int} are equal", (std::int32_t a, std::int32_t b))
@@ -140,3 +140,30 @@ GIVEN(R"(I attach a link to {string})", (const std::string& url))
 {
     Link(url, "title");
 }
+
+GIVEN(R"(step fixture does not fail)")
+{
+    // empty
+}
+
+struct FailingStepFixture : cucumber_cpp::StepBase
+{
+    using StepBase::StepBase;
+
+    bool nonExistentKey = context.Get<bool>("nonExistentKey");
+};
+
+GIVEN_F(FailingStepFixture, R"(step fixture fails)")
+{
+    // empty
+}
+
+GIVEN("a nested step that fails")
+{
+    ASSERT_THAT(false, testing::IsTrue());
+}
+
+GIVEN("a step calls another step that will fail")
+{
+    Step("a nested step that fails");
+}

cucumber_cpp/acceptance_test/steps/UsedUnused.cpp

@@ -1,4 +1,4 @@
-#include "cucumber_cpp/CucumberCpp.hpp"
+#include "cucumber_cpp/Steps.hpp"
 
 STEP("This step is unused")
 {

cucumber_cpp/acceptance_test/test.bats

@@ -222,3 +222,21 @@ teardown() {
     assert_output --partial "|   this step is used | -        |"
     assert_output --partial "| This step is unused | UNUSED   |"
 }
+
+@test "Test failure in step fixture results in error" {
+    run $acceptance_test --format summary --format-options "{ \"summary\": {\"theme\":\"plain\"} }"  --tags "@fail_step_fixture" -- cucumber_cpp/acceptance_test/features
+    assert_failure
+    assert_output --partial "key not found: \"nonExistentKey\""
+    assert_output --partial "2 scenarios 1 passed, 1 failed"
+    assert_output --partial "2 steps 1 passed, 1 failed"
+}
+
+@test "Test nested failures propagate properly" {
+    run $acceptance_test --format summary --format-options "{ \"summary\": {\"theme\":\"plain\"} }" --tags "@nested_failing_steps" -- cucumber_cpp/acceptance_test/features
+    assert_failure
+    assert_output --partial "FAILED nested step: \"* a nested step that fails\""
+    assert_output --partial "Value of: false"
+    assert_output --partial "Expected: is true"
+    assert_output --partial "Actual: false (of type bool)"
+    assert_output --partial "↷ Then this should be skipped"
+}

cucumber_cpp/example/steps/Steps.cpp

@@ -1,5 +1,5 @@
 #include "cucumber_cpp/library/Steps.hpp"
-#include "cucumber_cpp/CucumberCpp.hpp"
+#include "cucumber_cpp/Steps.hpp"
 #include "cucumber_cpp/library/Context.hpp"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"

cucumber_cpp/library/CMakeLists.txt

@@ -48,5 +48,5 @@ add_subdirectory(tag_expression)
 add_subdirectory(util)
 
 if (CCR_BUILD_TESTS)
-    # add_subdirectory(test)
+    add_subdirectory(test)
 endif()

cucumber_cpp/library/engine/CMakeLists.txt

@@ -24,5 +24,5 @@ target_link_libraries(cucumber_cpp.library.engine PUBLIC
 
 if (CCR_BUILD_TESTS)
     add_subdirectory(test)
-    # add_subdirectory(test_helper)
+    add_subdirectory(test_helper)
 endif()

cucumber_cpp/library/engine/test_helper/StepImplementations.cpp

@@ -62,7 +62,7 @@ WHEN("I call a nested step")
 {
     ASSERT_THAT(context.Contains("nested"), testing::IsFalse());
 
-    Given("I am a nested step");
+    Step("I am a nested step");
 }
 
 THEN("the nested step was called")

cucumber_cpp/library/runtime/NestedTestCaseRunner.cpp

@@ -70,9 +70,10 @@ namespace cucumber_cpp::library::runtime
             return testStep;
         }
 
-        void Invoke(std::size_t nesting, const std::string& step, std::unique_ptr<util::Body> body, const cucumber::messages::step_match_arguments_list& args)
+        void Invoke(std::size_t nesting, const std::string& step, const util::BodyFactory& bodyFactory, const cucumber::messages::step_match_arguments_list& args)
         {
-            const auto status = body->ExecuteAndCatchExceptions(util::StepMatchArgumentsListToExecuteArgs(args));
+            const auto status = util::ConstructAndExecute(bodyFactory, util::StepMatchArgumentsListToExecuteArgs(args));
+
             if (status.status != util::TestStepResultStatus::PASSED)
                 throw util::NestedTestCaseRunnerError{
                     .nesting = nesting,
@@ -88,7 +89,7 @@ namespace cucumber_cpp::library::runtime
                                                                              return supportCodeLibrary.stepRegistry.GetDefinitionById(id);
                                                                          });
 
-            if (testStep.step_definition_ids->size() == 0)
+            if (testStep.step_definition_ids->empty())
                 throw util::NestedTestCaseRunnerError{ .nesting = nesting, .status = {
                                                                                .duration = cucumber::messages::duration{},
                                                                                .status = cucumber::messages::test_step_result_status::UNDEFINED,
@@ -105,7 +106,12 @@ namespace cucumber_cpp::library::runtime
             else
             {
                 const auto& definition = stepDefinitions.front();
-                Invoke(nesting, step, definition.factory(NestedTestCaseRunner{ nesting, supportCodeLibrary, broadcaster, testCaseContext, testStepStarted }, broadcaster, testCaseContext, testStepStarted, util::TransformTable(dataTable), util::TransformDocString(docString)), testStep.step_match_arguments_lists->front());
+                NestedTestCaseRunner nestedTestCaseRunner{ nesting, supportCodeLibrary, broadcaster, testCaseContext, testStepStarted };
+                const util::BodyFactory bodyFactory = [&nestedTestCaseRunner, &definition, &broadcaster, &testCaseContext, &testStepStarted, &dataTable, &docString]
+                {
+                    return definition.factory(nestedTestCaseRunner, broadcaster, testCaseContext, testStepStarted, util::TransformTable(dataTable), util::TransformDocString(docString));
+                };
+                Invoke(nesting, step, bodyFactory, testStep.step_match_arguments_lists->front());
             }
         }
     }

cucumber_cpp/library/runtime/TestCaseRunner.cpp

@@ -47,9 +47,9 @@ namespace cucumber_cpp::library::runtime
 {
     namespace
     {
-        cucumber::messages::test_step_result InvokeStep(std::unique_ptr<util::Body> body, const cucumber::messages::step_match_arguments_list& args = {})
+        cucumber::messages::test_step_result InvokeStep(const util::BodyFactory& bodyFactory, const cucumber::messages::step_match_arguments_list& args)
         {
-            return util::TransformTestStepResult(body->ExecuteAndCatchExceptions(util::StepMatchArgumentsListToExecuteArgs(args)));
+            return util::TransformTestStepResult(util::ConstructAndExecute(bodyFactory, util::StepMatchArgumentsListToExecuteArgs(args)));
         }
     }
 
@@ -159,7 +159,12 @@ namespace cucumber_cpp::library::runtime
                 .status = cucumber::messages::test_step_result_status::SKIPPED,
             };
 
-        return InvokeStep(hookDefinition.factory(broadcaster, testCaseContext, util::TransformTestStepStarted(testStepStarted), hasError));
+        const util::BodyFactory bodyFactory = [&hookDefinition, this, &testCaseContext, &testStepStarted, hasError]
+        {
+            return hookDefinition.factory(broadcaster, testCaseContext, util::TransformTestStepStarted(testStepStarted), hasError);
+        };
+
+        return InvokeStep(bodyFactory, {});
     }
 
     std::vector<cucumber::messages::test_step_result> TestCaseRunner::RunStepHooks(const cucumber::messages::pickle_step& /*pickleStep*/, util::HookType hookType, Context& testCaseContext, const cucumber::messages::test_step_started& testStepStarted)
@@ -171,7 +176,12 @@ namespace cucumber_cpp::library::runtime
         for (const auto& id : ids)
         {
             const auto& definition = supportCodeLibrary.hookRegistry.GetDefinitionById(id);
-            results.emplace_back(InvokeStep(definition.factory(broadcaster, testCaseContext, util::TransformTestStepStarted(testStepStarted), false)));
+            const auto bodyFactory = [&definition, this, &testCaseContext, &testStepStarted]
+            {
+                return definition.factory(broadcaster, testCaseContext, util::TransformTestStepStarted(testStepStarted), false);
+            };
+
+            results.emplace_back(InvokeStep(bodyFactory, {}));
         }
 
         return results;
@@ -221,11 +231,15 @@ namespace cucumber_cpp::library::runtime
             const auto& docString = pickleStep.argument ? pickleStep.argument->doc_string : std::nullopt;
 
             const auto& definition = stepDefinitions.front();
-            const auto result = InvokeStep(definition.factory(
-                                               NestedTestCaseRunner{ 0, supportCodeLibrary, broadcaster, testCaseContext, util::TransformTestStepStarted(testStepStarted) },
-                                               broadcaster, testCaseContext, util::TransformTestStepStarted(testStepStarted), util::TransformTable(dataTable), util::TransformDocString(docString)),
-                testStep.step_match_arguments_lists->front());
-            stepResults.push_back(result);
+
+            NestedTestCaseRunner nestedTestCaseRunner{ 0, supportCodeLibrary, broadcaster, testCaseContext, util::TransformTestStepStarted(testStepStarted) };
+
+            const util::BodyFactory bodyFactory = [this, &definition, &nestedTestCaseRunner, &testCaseContext, &testStepStarted, &dataTable, &docString]
+            {
+                return definition.factory(nestedTestCaseRunner, broadcaster, testCaseContext, util::TransformTestStepStarted(testStepStarted), util::TransformTable(dataTable), util::TransformDocString(docString));
+            };
+
+            stepResults.push_back(InvokeStep(bodyFactory, testStep.step_match_arguments_lists->front()));
         }
 
         const auto afterStepHookResults = RunStepHooks(pickleStep, util::HookType::afterStep, testCaseContext, testStepStarted);

cucumber_cpp/library/runtime/Worker.cpp

@@ -16,6 +16,7 @@
 #include "cucumber_cpp/library/support/HookRegistry.hpp"
 #include "cucumber_cpp/library/support/SupportCodeLibrary.hpp"
 #include "cucumber_cpp/library/support/Types.hpp"
+#include "cucumber_cpp/library/util/Body.hpp"
 #include "cucumber_cpp/library/util/Broadcaster.hpp"
 #include "cucumber_cpp/library/util/GetWorstTestStepResult.hpp"
 #include "cucumber_cpp/library/util/HookData.hpp"
@@ -189,9 +190,15 @@ namespace cucumber_cpp::library::runtime
         broadcaster.BroadcastEvent({ .test_run_hook_started = testRunHookStarted });
 
         cucumber::messages::test_step_result result{ .duration{ .seconds = 0, .nanos = 0 }, .status = cucumber::messages::test_step_result_status::SKIPPED };
+
         if (!options.dryRun)
         {
-            result = util::TransformTestStepResult(definition.factory(broadcaster, context, util::TransformTestRunHookStarted(testRunHookStarted), false)->ExecuteAndCatchExceptions());
+            const util::BodyFactory bodyFactory = [&definition, this, &context, &testRunHookStarted]
+            {
+                return definition.factory(broadcaster, context, util::TransformTestRunHookStarted(testRunHookStarted), false);
+            };
+
+            result = util::TransformTestStepResult(util::ConstructAndExecute(bodyFactory));
 
             if (result.status != cucumber::messages::test_step_result_status::PASSED && options.failGlobalHookFast)
                 throw GlobalHookError{ fmt::format("Global Hook Failed: {}\nresult:{}", util::TransformHookData(definition.data).to_string(), result.to_string()) };

cucumber_cpp/library/test/CMakeLists.txt

@@ -13,5 +13,4 @@ target_link_libraries(cucumber_cpp.library.test PUBLIC
 target_sources(cucumber_cpp.library.test PRIVATE
     TestApplication.cpp
     TestContext.cpp
-    TestSteps.cpp
 )

cucumber_cpp/library/test/TestApplication.cpp

@@ -2,6 +2,7 @@
 #include "cucumber_cpp/library/Application.hpp"
 #include "cucumber_cpp/library/engine/test_helper/TemporaryFile.hpp"
 #include "gmock/gmock.h"
+#include "gtest/gtest.h"
 #include <CLI/Error.hpp>
 #include <array>
 #include <cstddef>
@@ -34,41 +35,10 @@ namespace cucumber_cpp::library
         return capturedStdout;
     }
 
-    TEST_F(TestApplication, RunHelpWithoutArguments)
-    {
-
-        const std::array args{ "application" };
-
-        RunWithArgs(args, static_cast<std::underlying_type_t<CLI::ExitCodes>>(CLI::ExitCodes::RequiredError));
-    }
-
-    TEST_F(TestApplication, RunCommandWithoutArguments)
-    {
-
-        const std::array args{ "application", "run" };
-
-        RunWithArgs(args, static_cast<std::underlying_type_t<CLI::ExitCodes>>(CLI::ExitCodes::RequiredError));
-    }
-
-    TEST_F(TestApplication, RunCommand)
-    {
-
-        const std::array args{ "application", "run", "--feature", "./", "--report", "console" };
-
-        RunWithArgs(args, static_cast<std::underlying_type_t<CLI::ExitCodes>>(CLI::ExitCodes::Success));
-    }
-
-    TEST_F(TestApplication, DryRunCommand)
-    {
-        const std::array args{ "application", "run", "--feature", "./", "--report", "console", "--dry" };
-
-        RunWithArgs(args, static_cast<std::underlying_type_t<CLI::ExitCodes>>(CLI::ExitCodes::Success));
-    }
-
     TEST_F(TestApplication, InvalidArgument)
     {
 
-        const std::array args{ "application", "run", "--feature", "./", "--report", "console", "--doesntexist" };
+        const std::array args{ "application", "--doesntexist" };
 
         RunWithArgs(args, static_cast<std::underlying_type_t<CLI::ExitCodes>>(CLI::ExitCodes::ExtrasError));
     }
@@ -83,11 +53,12 @@ namespace cucumber_cpp::library
                "    Scenario: Test scenario1\n"
                "      Given 5 and 5 are equal\n";
 
-        const std::array args{ "application", "run", "--feature", path.c_str(), "--report", "console", "--dry" };
+        const std::array args{ "application", "--format-options", R"({ "summary": {"theme":"plain"} })", "--dry-run", path.c_str() };
 
         std::string stdoutString = RunWithArgs(args, static_cast<std::underlying_type_t<CLI::ExitCodes>>(CLI::ExitCodes::Success));
 
-        EXPECT_THAT(stdoutString, testing::HasSubstr("1/1 passed"));
+        EXPECT_THAT(stdoutString, testing::HasSubstr("1 scenarios 1 skipped"));
+        EXPECT_THAT(stdoutString, testing::HasSubstr("1 steps 1 skipped"));
     }
 
     TEST_F(TestApplication, RunFeatureFile)
@@ -100,44 +71,16 @@ namespace cucumber_cpp::library
                "    Scenario: Test scenario1\n"
                "      Given 5 and 5 are equal\n";
 
-        const std::array args{ "application", "run", "--feature", path.c_str(), "--report", "console" };
+        const std::array args{ "application", "--format-options", R"({ "summary": {"theme":"plain"} })", path.c_str() };
 
         std::string stdoutString = RunWithArgs(args, static_cast<std::underlying_type_t<CLI::ExitCodes>>(CLI::ExitCodes::Success));
 
-        EXPECT_THAT(stdoutString, testing::HasSubstr("1/1 passed"));
+        EXPECT_THAT(stdoutString, testing::HasSubstr("1 scenarios 1 passed"));
+        EXPECT_THAT(stdoutString, testing::HasSubstr("1 steps 1 passed"));
     }
 
     TEST_F(TestApplication, ExposeParameterRegistration)
     {
         EXPECT_THAT(&Application{}.ParameterRegistration(), testing::NotNull());
     }
-
-    TEST_F(TestApplication, UnusedParameters)
-    {
-        auto tmp = engine::test_helper::TemporaryFile{ "tmpfile.feature" };
-        const auto path = tmp.Path().string();
-
-        tmp << "Feature: Test feature\n"
-               "  Rule: Test rule\n"
-               "    Scenario: Test scenario1\n"
-               "      Given 5 and 5 are equal\n"
-               "      And This is a GIVEN step\n"
-               "      And This is a WHEN step\n"
-               "      And This is a THEN step\n"
-               "      And This is a STEP step\n";
-
-        const std::array args{ "application", "run", "--feature", path.c_str(), "--report", "console", "--unused" };
-
-        std::string stdoutString = RunWithArgs(args, static_cast<std::underlying_type_t<CLI::ExitCodes>>(CLI::ExitCodes::Success));
-
-        EXPECT_THAT(stdoutString, testing::HasSubstr("The following steps have not been used:"));
-        EXPECT_THAT(stdoutString, testing::HasSubstr("^This is a step with a ([0-9]+)s delay$"));
-        EXPECT_THAT(stdoutString, testing::HasSubstr("Step with cucumber expression syntax {float} {string} {int}"));
-
-        EXPECT_THAT(stdoutString, testing::Not(testing::HasSubstr("{int} and {int} are equal")));
-        EXPECT_THAT(stdoutString, testing::Not(testing::HasSubstr("And This is a GIVEN step")));
-        EXPECT_THAT(stdoutString, testing::Not(testing::HasSubstr("And This is a WHEN step")));
-        EXPECT_THAT(stdoutString, testing::Not(testing::HasSubstr("And This is a THEN step")));
-        EXPECT_THAT(stdoutString, testing::Not(testing::HasSubstr("And This is a STEP step")));
-    }
 }