feat: catch exceptions while constructing a step fixture#364
Open
Daan Timmer (daantimmer) wants to merge 4 commits into
Open
Conversation
✅
|
| Descriptor | Linter | Files | Fixed | Errors | Warnings | Elapsed time |
|---|---|---|---|---|---|---|
| ✅ ACTION | actionlint | 6 | 0 | 0 | 0.33s | |
| ✅ ACTION | zizmor | 6 | 0 | 0 | 0 | 3.33s |
| ✅ CPP | clang-format | 239 | 1 | 0 | 0 | 1.66s |
| ✅ DOCKERFILE | hadolint | 1 | 0 | 0 | 0.03s | |
| markdownlint | 7 | 4 | 19 | 0 | 1.01s | |
| ✅ MARKDOWN | markdown-table-formatter | 7 | 4 | 0 | 0 | 0.25s |
| ✅ REPOSITORY | git_diff | yes | no | no | 0.04s | |
| ✅ REPOSITORY | grype | yes | no | no | 44.36s | |
| ✅ REPOSITORY | ls-lint | yes | no | no | 0.01s | |
| ✅ REPOSITORY | secretlint | yes | no | no | 3.74s | |
| ✅ REPOSITORY | syft | yes | no | no | 2.2s | |
| ✅ REPOSITORY | trivy | yes | no | no | 11.76s | |
| ✅ REPOSITORY | trivy-sbom | yes | no | no | 0.25s | |
| ✅ REPOSITORY | trufflehog | yes | no | no | 4.13s | |
| lychee | 86 | 1 | 0 | 13.53s | ||
| ✅ YAML | prettier | 10 | 0 | 0 | 0 | 0.7s |
| ✅ YAML | v8r | 10 | 0 | 0 | 7.3s | |
| ✅ YAML | yamllint | 10 | 0 | 0 | 0.51s |
Detailed Issues
⚠️ SPELL / lychee - 1 error
📝 Summary
---------------------
🔍 Total..........175
🔗 Unique.........169
✅ Successful.....174
⏳ Timeouts.........0
🔀 Redirected......80
👻 Excluded.........0
❓ Unknown..........0
🚫 Errors...........1
⛔ Unsupported......1
Errors in CONTRIBUTING.md
[404] https://github.com/yourname/amp-cucumber-cpp-runner.git (at 41:36) | Rejected status code: 404 Not Found | Followed 1 redirect. Redirects: https://github.com/yourname/amp-cucumber-cpp-runner.git --[301]--> https://github.com/yourname/amp-cucumber-cpp-runner
Hint: Followed 80 redirects. You might want to consider replacing redirecting URLs with the resolved URLs. Use verbose mode (`-v`/`-vv`) to see redirection details.
Hint: You can configure accepted/rejected response codes with `-a` or `--accept`
⚠️ MARKDOWN / markdownlint - 19 errors
CHANGELOG.md:31 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Bug Fixes"]
CHANGELOG.md:37 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Chores"]
CHANGELOG.md:44 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:51 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Chores"]
CHANGELOG.md:59 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "⚠ BREAKING CHANGES"]
CHANGELOG.md:63 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:72 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:78 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Chores"]
CHANGELOG.md:86 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "⚠ BREAKING CHANGES"]
CHANGELOG.md:90 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:95 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Bug Fixes"]
CHANGELOG.md:104 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:115 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Chores"]
CHANGELOG.md:123 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "⚠ BREAKING CHANGES"]
CHANGELOG.md:127 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:152 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Bug Fixes"]
CHANGELOG.md:159 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Chores"]
cmake/WindowsToolchainFilesProvidedBy.md:1 error MD041/first-line-heading/first-line-h1 First line in a file should be a top-level heading [Context: "<https://github.com/MarkSchofi..."]
README.md:132 error MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]
Notices
📣 MegaLinter 9.5.0 is out! Discover the new features and security recommendations in the release announcement. (Skip this info by defining SECURITY_SUGGESTIONS: false)
See detailed reports in MegaLinter artifacts
Your project could benefit from a custom flavor, which would allow you to run only the linters you need, and thus improve runtime performances. (Skip this info by defining FLAVOR_SUGGESTIONS: false)
- Documentation: Custom Flavors
- Command:
npx mega-linter-runner@9.5.0 --custom-flavor-setup --custom-flavor-linters ACTION_ACTIONLINT,ACTION_ZIZMOR,CPP_CLANG_FORMAT,DOCKERFILE_HADOLINT,MARKDOWN_MARKDOWNLINT,MARKDOWN_MARKDOWN_TABLE_FORMATTER,REPOSITORY_GIT_DIFF,REPOSITORY_GRYPE,REPOSITORY_LS_LINT,REPOSITORY_SECRETLINT,REPOSITORY_SYFT,REPOSITORY_TRIVY,REPOSITORY_TRIVY_SBOM,REPOSITORY_TRUFFLEHOG,SPELL_LYCHEE,YAML_PRETTIER,YAML_YAMLLINT,YAML_V8R
Show us your support by starring ⭐ the repository
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the existing step exception handling so that exceptions thrown during step fixture construction (not just during step/hook execution) are also caught and converted into a proper TestStepResult.
Changes:
- Introduced a
BodyFactory+ConstructAndExecute(...)flow to ensure both construction and execution are inside the same try/catch. - Extracted the common exception-to-
TestStepResultmapping into a shared utility (HandleErrors). - Added an acceptance test that verifies a failing step fixture is reported as a test failure (with the thrown message).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| cucumber_cpp/Steps.hpp | Adds an explicit include for engine/Hook.hpp needed for re-exported types. |
| cucumber_cpp/library/util/ExecuteAndCatchExceptions.hpp | New helper for mapping caught exceptions into TestStepResult. |
| cucumber_cpp/library/util/Body.hpp | Refactors the execution API to use BodyFactory and introduces ConstructAndExecute. |
| cucumber_cpp/library/util/Body.cpp | Implements ConstructAndExecute and routes exception handling through HandleErrors. |
| cucumber_cpp/library/runtime/Worker.cpp | Switches hook execution to the new factory-based construction+execution path. |
| cucumber_cpp/library/runtime/TestCaseRunner.cpp | Switches step/hook invocation to BodyFactory + ConstructAndExecute. |
| cucumber_cpp/library/runtime/NestedTestCaseRunner.cpp | Switches nested step invocation to BodyFactory + ConstructAndExecute. |
| cucumber_cpp/example/steps/Steps.cpp | Updates example includes to use the new umbrella header. |
| cucumber_cpp/acceptance_test/test.bats | Adds an acceptance test asserting fixture-construction failures are reported. |
| cucumber_cpp/acceptance_test/steps/UsedUnused.cpp | Updates include to cucumber_cpp/Steps.hpp. |
| cucumber_cpp/acceptance_test/steps/Steps.cpp | Adds a failing step fixture and updates includes. |
| cucumber_cpp/acceptance_test/features/test_step_fixture_failure.feature | New feature file covering step fixture construction failure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| namespace cucumber_cpp::library::util | ||
| { | ||
| inline void HandleErrors(TestStepResult& testStepResult) noexcept |
Comment on lines
4
to
+10
| #include "cucumber_cpp/library/cucumber_expression/ParameterRegistry.hpp" | ||
| #include "cucumber_cpp/library/util/TestStepResult.hpp" | ||
| #include "gtest/gtest.h" | ||
| #include <exception> | ||
| #include <functional> | ||
| #include <gtest/gtest-spi.h> | ||
| #include <memory> |
Comment on lines
+69
to
80
| using BodyFactory = std::function<std::unique_ptr<Body>()>; | ||
| TestStepResult ConstructAndExecute(const BodyFactory& bodyFactory, const ExecuteArgs& args = {}); | ||
|
|
||
| struct Body | ||
| { | ||
| virtual ~Body() = default; | ||
|
|
||
| TestStepResult ExecuteAndCatchExceptions(const ExecuteArgs& args = {}); | ||
|
|
||
| protected: | ||
| private: | ||
| virtual void Execute(const ExecuteArgs& args) = 0; | ||
|
|
||
| friend TestStepResult ConstructAndExecute(const BodyFactory& bodyFactory, const ExecuteArgs& args); | ||
| }; |
Comment on lines
+20
to
+21
| inline void HandleErrors(TestStepResult& testStepResult) noexcept | ||
| { |
Comment on lines
+6
to
+10
| #include "gtest/gtest.h" | ||
| #include <exception> | ||
| #include <functional> | ||
| #include <gtest/gtest-spi.h> | ||
| #include <memory> |
Comment on lines
+67
to
80
| struct Body; | ||
|
|
||
| using BodyFactory = std::function<std::unique_ptr<Body>()>; | ||
| TestStepResult ConstructAndExecute(const BodyFactory& bodyFactory, const ExecuteArgs& args = {}); | ||
|
|
||
| struct Body | ||
| { | ||
| virtual ~Body() = default; | ||
|
|
||
| TestStepResult ExecuteAndCatchExceptions(const ExecuteArgs& args = {}); | ||
|
|
||
| protected: | ||
| private: | ||
| virtual void Execute(const ExecuteArgs& args) = 0; | ||
|
|
||
| friend TestStepResult ConstructAndExecute(const BodyFactory& bodyFactory, const ExecuteArgs& args); | ||
| }; |
Comment on lines
+1
to
2
| #include "cucumber_cpp/Steps.hpp" | ||
| #include "fmt/format.h" |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Exceptions were already caught when executing a step (hook implementation or step implementation).
But custom step test fixtures could also throw (missing context values, ASSERT_'s etc). These were not caught.
This PR ensures both the construction of a step and the execution of a step are properly inside a try/catch block.