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

[daantimmer]

No description provided.

[github-actions]

Test Results

25 tests  +1   25 ✅ +1   3s ⏱️ ±0s
 1 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 3dc0c50. ± Comparison against base commit a76cdd4.

♻️ This comment has been updated with latest results.

[github-actions]

🦙 MegaLinter status: ⚠️ WARNING

Descriptor	Linter	Files	Fixed	Errors	Warnings	Elapsed time
✅ ACTION	actionlint	6		0	0	0.02s
✅ CPP	clang-format	82	1	0	0	0.61s
✅ DOCKERFILE	hadolint	1		0	0	0.11s
✅ JSON	jsonlint	8		0	0	0.15s
✅ JSON	prettier	8	6	0	0	0.49s
⚠️ MARKDOWN	markdownlint	4	1	2	0	0.72s
⚠️ MARKDOWN	markdown-link-check	4		2	0	627.3s
✅ MARKDOWN	markdown-table-formatter	4	1	0	0	0.18s
✅ REPOSITORY	checkov	yes		no	no	14.0s
✅ REPOSITORY	git_diff	yes		no	no	0.01s
✅ REPOSITORY	grype	yes		no	no	21.14s
✅ REPOSITORY	ls-lint	yes		no	no	0.06s
✅ REPOSITORY	secretlint	yes		no	no	1.03s
✅ REPOSITORY	syft	yes		no	no	1.26s
✅ REPOSITORY	trivy	yes		no	no	5.04s
✅ REPOSITORY	trivy-sbom	yes		no	no	0.07s
✅ REPOSITORY	trufflehog	yes		no	no	4.27s
⚠️ SPELL	lychee	50		35	0	7.76s
✅ YAML	prettier	10	0	0	0	0.55s
✅ YAML	v8r	10		0	0	5.16s
✅ YAML	yamllint	10		0	0	0.48s

See detailed report in MegaLinter reports

[Copilot]

Pull Request Overview

This PR addresses an issue with hooks execution by preventing the current scope from executing when an error occurs during a before hook. The key changes include:

• Wrapping test execution steps in lambdas with ASSERT_NO_THROW to avoid unintended execution upon hook failure.
• Introducing a custom exception (HookFailed) to control flow in hook execution.
• Adjusting hook execution logic in both TestRunner.cpp and HookExecutor.cpp.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
cucumber_cpp/library/engine/TestRunner.cpp	Wraps scope execution in lambdas with ASSERT_NO_THROW.
cucumber_cpp/library/engine/HookExecutor.cpp	Implements HookFailed exception and updates hook flow.

Comments suppressed due to low confidence (2)

cucumber_cpp/library/engine/TestRunner.cpp:41

• [nitpick] Consider renaming the lambda variable 'run' to a more descriptive name (e.g., 'executeScope') to improve clarity.

auto run = [this, &feature]

cucumber_cpp/library/engine/HookExecutor.cpp:48

• [nitpick] When rethrowing the HookFailed exception with a generic message, consider preserving or appending the original error details to aid debugging.

catch (HookFailed&)

[Copilot]

Pull Request Overview

This PR fixes hook execution so that when an error occurs during a before hook the current scope is not executed. Key changes include:

• Adding a ReportForwarderMock to capture and report hook failures.
• Introducing new test cases to validate that errors in before hooks properly halt execution.
• Refactoring hook execution logic (using ThrowPolicy and lambda wrappers) in both TestRunner and HookExecutor.

Reviewed Changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
cucumber_cpp/library/report/test_helper/ReportForwarderMock.hpp	New mock implementation for reporting hook failures
cucumber_cpp/library/engine/test_helper/ContextManagerInstance.hpp	Adjusted context manager initialization to support tags
cucumber_cpp/library/engine/test/TestTestRunner.cpp	Added tests to ensure execution stops on hook errors
cucumber_cpp/library/engine/test/TestHookExecutorHooks.cpp	Added a failing before step hook for error simulation
cucumber_cpp/library/engine/test/TestHookExecutor.cpp	Added tests to check exception throwing on hook errors
cucumber_cpp/library/engine/test/TestFailureHandler.cpp	Updated to use the new ReportForwarderMock
cucumber_cpp/library/engine/TestRunner.cpp	Refactored runner execution using lambdas and ASSERT_NO_THROW
cucumber_cpp/library/engine/HookExecutor.cpp	Updated hook execution with ThrowPolicy for error handling

Files not reviewed (2)

• cucumber_cpp/library/report/CMakeLists.txt: Language not supported
• cucumber_cpp/library/report/test_helper/CMakeLists.txt: Language not supported

Comments suppressed due to low confidence (1)

cucumber_cpp/library/engine/HookExecutor.cpp:76

• [nitpick] Consider making the error message more descriptive (e.g., including the hook type) to aid in debugging when a before hook fails.

ExecuteHook(runnerContext, hookPair.before, tags, ThrowExceptionPolicy{ "Hook failed" });

[Copilot]

Pull Request Overview

This PR fixes hook execution behavior so that the current scope is not executed when an error occurs in a before hook. Key changes include the introduction of a ReportForwarderMock to support testing; adjustments to various context management functions to use the new EffectiveExecutionStatus and NestedExecutionStatus; and modifications in hook execution to use a ThrowPolicy pattern for error propagation.

Reviewed Changes

Copilot reviewed 15 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
cucumber_cpp/library/report/test_helper/ReportForwarderMock.hpp	Added a new mock for report forwarding
cucumber_cpp/library/engine/test_helper/ContextManagerInstance.hpp	Updated context instance initialization with tag-based parameters
cucumber_cpp/library/engine/test/TestTestRunner.cpp	Updated test expectations to use EffectiveExecutionStatus
cucumber_cpp/library/engine/test/TestHookExecutorHooks.cpp	Added a failing before-step hook for error testing
cucumber_cpp/library/engine/test/TestHookExecutor.cpp	Refactored context access to use optional pointers and updated tests
cucumber_cpp/library/engine/test/TestFailureHandler.cpp	Updated failure tests to use the ReportForwarderMock from test helper
cucumber_cpp/library/engine/TestRunner.cpp	Renamed a parameter from “feature” to “features” for clarity
cucumber_cpp/library/engine/TestExecution.cpp	Adjusted step execution to check both Inherited and Effective statuses
cucumber_cpp/library/engine/HookExecutor.cpp	Introduced ThrowPolicy patterns for hook execution error propagation
cucumber_cpp/library/engine/FailureHandler.cpp	Extended failure handling to update step context status
cucumber_cpp/library/engine/ContextManager.hpp & ContextManager.cpp	Added new context status functions and updated nested status management
cucumber_cpp/library/Application.cpp	Updated exit code calculation based on EffectiveExecutionStatus
cucumber_cpp/acceptance_test/hooks/Hooks.cpp	Added hooks to test program hook failure scenarios
cucumber_cpp/acceptance_test/MainCustom.cpp	Added a new flag to trigger program hook failure in acceptance tests

Files not reviewed (3)

• cucumber_cpp/acceptance_test/test.bats: Language not supported
• cucumber_cpp/library/report/CMakeLists.txt: Language not supported
• cucumber_cpp/library/report/test_helper/CMakeLists.txt: Language not supported

[Copilot]

The ExecuteHook function now accepts a ThrowPolicy parameter but does not invoke throwPolicy.Throw() anywhere in its implementation. Consider adding logic to call throwPolicy.Throw() when a hook error is detected to ensure errors are properly propagated.

[Copilot]

[nitpick] For consistency with other tests updating context status checks, consider using EffectiveExecutionStatus() instead of ExecutionStatus() when verifying a failed hook state.

Suggested change

	EXPECT_THAT(contextManagerInstance->CurrentStepContext()->ExecutionStatus(), testing::Eq(Result::failed));
	EXPECT_THAT(contextManagerInstance->CurrentStepContext()->EffectiveExecutionStatus(), testing::Eq(Result::failed));

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
91.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

Pull Request Overview

This PR addresses the issue where the current scope should not execute when an error occurs during a before hook. The changes update context status checks to use EffectiveExecutionStatus, modify hook execution to prevent unintended scope execution, and improve failure reporting and context propagation.

Reviewed Changes

Copilot reviewed 15 out of 18 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
cucumber_cpp/library/report/test_helper/ReportForwarderMock.hpp	Introduces a new mock implementation for report forwarding in tests
cucumber_cpp/library/engine/test_helper/ContextManagerInstance.hpp	Updates the ContextManagerInstance constructor to pass tags and reinitializes members
cucumber_cpp/library/engine/test/TestTestRunner.cpp	Updates tests to use EffectiveExecutionStatus and stricter mock expectations
cucumber_cpp/library/engine/test/TestHookExecutorHooks.cpp	Adds a new HOOK_BEFORE_STEP hook to simulate an error condition
cucumber_cpp/library/engine/test/TestHookExecutor.cpp	Refactors context access using std::optional and updates hook-related tests
cucumber_cpp/library/engine/test/TestFailureHandler.cpp	Switches to EffectiveExecutionStatus for validating context failure
cucumber_cpp/library/engine/TestRunner.cpp	Renames parameter from "feature" to "features" to improve clarity
cucumber_cpp/library/engine/TestExecution.cpp	Alters the step execution condition to check both inherited and effective statuses
cucumber_cpp/library/engine/HookExecutor.cpp	Removes the try-catch block in hook execution, relying on a condition check
cucumber_cpp/library/engine/FailureHandler.cpp	Ensures that the current step context is updated on failure
cucumber_cpp/library/engine/ContextManager.hpp & ContextManager.cpp	Adds support for inherited and nested execution statuses and updates context propagation
cucumber_cpp/library/Application.cpp	Updates exit code logic to use EffectiveExecutionStatus
cucumber_cpp/acceptance_test/hooks/Hooks.cpp	Adds conditions in hooks to simulate failure scenarios
cucumber_cpp/acceptance_test/MainCustom.cpp	Introduces a CLI flag to trigger intended hook failure behavior

Files not reviewed (3)

• cucumber_cpp/acceptance_test/test.bats: Language not supported
• cucumber_cpp/library/report/CMakeLists.txt: Language not supported
• cucumber_cpp/library/report/test_helper/CMakeLists.txt: Language not supported

Comments suppressed due to low confidence (2)

cucumber_cpp/library/engine/HookExecutor.cpp:25

• Removing the try-catch block means that exceptions thrown in hooks will now propagate; confirm that this behavior is intentional and that it won't cause unexpected test failures.

if (runnerContext.InheritedExecutionStatus() == Result::passed)

cucumber_cpp/library/engine/TestExecution.cpp:87

• Confirm that requiring both InheritedExecutionStatus and EffectiveExecutionStatus to be passed for executing a step is the intended logic, ensuring that erroneous scopes are correctly skipped.

if (contextManager.ScenarioContext().InheritedExecutionStatus() == Result::passed &&