feat: add optional asynchronous/parallel support#312
Draft
Daan Timmer (daantimmer) wants to merge 2 commits intomainfrom
Draft
feat: add optional asynchronous/parallel support#312Daan Timmer (daantimmer) wants to merge 2 commits intomainfrom
Daan Timmer (daantimmer) wants to merge 2 commits intomainfrom
Conversation
Daan Timmer (daantimmer)
changed the title
✅
|
| Descriptor | Linter | Files | Fixed | Errors | Warnings | Elapsed time |
|---|---|---|---|---|---|---|
| ✅ ACTION | actionlint | 6 | 0 | 0 | 0.02s | |
| ✅ CPP | clang-format | 231 | 0 | 0 | 0 | 1.81s |
| ✅ DOCKERFILE | hadolint | 1 | 0 | 0 | 0.03s | |
| ✅ JSON | jsonlint | 8 | 0 | 0 | 0.38s | |
| ✅ JSON | prettier | 8 | 6 | 0 | 0 | 0.65s |
| markdownlint | 6 | 3 | 16 | 0 | 1.19s | |
| ✅ MARKDOWN | markdown-table-formatter | 6 | 3 | 0 | 0 | 0.33s |
| ✅ REPOSITORY | git_diff | yes | no | no | 0.03s | |
| ✅ REPOSITORY | grype | yes | no | no | 33.12s | |
| ✅ REPOSITORY | ls-lint | yes | no | no | 0.1s | |
| ✅ REPOSITORY | secretlint | yes | no | no | 4.17s | |
| ✅ REPOSITORY | syft | yes | no | no | 1.25s | |
| ✅ REPOSITORY | trivy | yes | no | no | 10.04s | |
| ✅ REPOSITORY | trivy-sbom | yes | no | no | 0.26s | |
| ✅ REPOSITORY | trufflehog | yes | no | no | 4.37s | |
| lychee | 85 | 1 | 0 | 5.22s | ||
| ✅ YAML | prettier | 10 | 0 | 0 | 0 | 0.82s |
| ✅ YAML | v8r | 10 | 0 | 0 | 7.41s | |
| ✅ YAML | yamllint | 10 | 0 | 0 | 0.65s |
Detailed Issues
⚠️ SPELL / lychee - 1 error
[404] https://github.com/yourname/amp-cucumber-cpp-runner.git | Network error: Not Found
📝 Summary
---------------------
🔍 Total..........158
✅ Successful.....157
⏳ Timeouts.........0
🔀 Redirected.......0
👻 Excluded.........0
❓ Unknown..........0
🚫 Errors...........1
Errors in CONTRIBUTING.md
[404] https://github.com/yourname/amp-cucumber-cpp-runner.git | Network error: Not Found
⚠️ MARKDOWN / markdownlint - 16 errors
CHANGELOG.md:30 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:37 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Chores"]
CHANGELOG.md:45 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "⚠ BREAKING CHANGES"]
CHANGELOG.md:49 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:58 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:64 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Chores"]
CHANGELOG.md:72 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "⚠ BREAKING CHANGES"]
CHANGELOG.md:76 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:81 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Bug Fixes"]
CHANGELOG.md:90 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:101 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Chores"]
CHANGELOG.md:109 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "⚠ BREAKING CHANGES"]
CHANGELOG.md:113 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Features"]
CHANGELOG.md:138 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Bug Fixes"]
CHANGELOG.md:145 error MD024/no-duplicate-heading Multiple headings with the same content [Context: "Chores"]
README.md:114 error MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]
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.4.0 --custom-flavor-setup --custom-flavor-linters ACTION_ACTIONLINT,CPP_CLANG_FORMAT,DOCKERFILE_HADOLINT,JSON_JSONLINT,JSON_PRETTIER,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
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
[MegaLinter] reported by reviewdog 🐶
amp-cucumber-cpp-runner/.vscode/launch.json
Lines 42 to 77 in de36995
There was a problem hiding this comment.
[MegaLinter] reported by reviewdog 🐶
amp-cucumber-cpp-runner/CMakePresets.json
Lines 2 to 171 in de36995
Contributor
There was a problem hiding this comment.
Pull request overview
Adds optional parallel scenario execution to the cucumber-cpp runner (guarded behind CCR_ENABLE_PARALLEL_SUPPORT / ENABLE_PARALLEL_SUPPORT) by introducing a libcoro-backed runtime adapter, along with supporting refactors to broadcasting and runtime wiring.
Changes:
- Add optional libcoro dependency plumbing and CMake presets for enabling parallel support.
- Refactor
util::Broadcasterinto an interface +BroadcasterImplimplementation to support parallelized event emission. - Introduce
ParallelRuntimeAdapterselected via new--parallelruntime option (when compiled with parallel support).
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| external/jbaldwin/libcoro/CMakeLists.txt | FetchContent integration for libcoro and warning suppression configuration. |
| external/jbaldwin/CMakeLists.txt | Adds jbaldwin subtree (libcoro) to the build. |
| external/CMakeLists.txt | Conditionally includes jbaldwin deps when CCR_ENABLE_PARALLEL_SUPPORT is on. |
| cucumber_cpp/library/util/Broadcaster.hpp | Converts broadcaster into an abstract interface; adds BroadcasterImpl. |
| cucumber_cpp/library/util/Broadcaster.cpp | Moves implementations onto BroadcasterImpl; minor const-correctness tweak. |
| cucumber_cpp/library/support/Types.hpp | Adds parallel option to runtime config; tweaks enum underlying type. |
| cucumber_cpp/library/runtime/Worker.hpp | Exposes hook/suite helpers needed by adapters; adds [[nodiscard]]. |
| cucumber_cpp/library/runtime/Worker.cpp | Removes suite-level runner; small logic simplification and envelope construction tweaks. |
| cucumber_cpp/library/runtime/SerialRuntimeAdapter.hpp | Adds RunTestSuite(...) helper and includes for moved logic. |
| cucumber_cpp/library/runtime/SerialRuntimeAdapter.cpp | Hosts suite-level execution logic previously in Worker. |
| cucumber_cpp/library/runtime/ParallelRuntimeAdapter.hpp | New parallel runtime adapter interface (libcoro-based). |
| cucumber_cpp/library/runtime/ParallelRuntimeAdapter.cpp | New parallel execution implementation with thread pool + queued broadcasting. |
| cucumber_cpp/library/runtime/MakeRuntime.hpp | Removes exported MakeAdapter declaration (now internal). |
| cucumber_cpp/library/runtime/MakeRuntime.cpp | Selects serial vs parallel adapter based on options.parallel + build flag. |
| cucumber_cpp/library/runtime/CMakeLists.txt | Conditionally builds/links parallel adapter + libcoro; defines ENABLE_PARALLEL_SUPPORT. |
| cucumber_cpp/library/query/Query.hpp | Switches Query to inherit BroadcasterImpl (since Broadcaster is now abstract). |
| cucumber_cpp/library/formatter/helper/Theme.cpp | Removes unused optional helper. |
| cucumber_cpp/library/formatter/UsageFormatter.cpp | Avoids structured binding copies in loop. |
| cucumber_cpp/library/formatter/SummaryFormatter.cpp | Avoids structured binding copies; removes unused locals. |
| cucumber_cpp/library/engine/test/TestStep.cpp | Updates test fixture broadcaster type to BroadcasterImpl. |
| cucumber_cpp/library/Application.hpp | Adds parallel CLI option storage; updates broadcaster type + integer widths. |
| cucumber_cpp/library/Application.cpp | Registers --parallel when compiled with parallel support; plumbs into RunOptions. |
| compatibility/BaseCompatibility.cpp | Updates broadcaster type to BroadcasterImpl. |
| CMakePresets.json | Adds Host/Windows presets enabling parallel support. |
| CMakeLists.txt | Introduces CCR_ENABLE_PARALLEL_SUPPORT option; finds libcoro when enabled; standard settings moved earlier. |
| .vscode/launch.json | Updates debug args to include parallel run flags. |
| .clang-tidy | Disables coroutine reference-parameter guideline warning. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+43
to
+55
| coro::task<void> RunTestCase(std::unique_ptr<coro::thread_pool>& tp, coro::latch& tasksLatch, runtime::Worker& worker, const cucumber::messages::gherkin_document& gherkinDocument, const assemble::AssembledTestCase& assembledTestCase, Context& testSuiteContext, bool& failing) | ||
| { | ||
| co_await tp->schedule(); | ||
|
|
||
| try | ||
| { | ||
| failing |= !worker.RunTestCase(gherkinDocument, assembledTestCase, testSuiteContext, failing); | ||
| } | ||
| catch (...) | ||
| { | ||
| failing = true; | ||
| } | ||
|
|
Comment on lines
+150
to
+159
| coro::latch taskLatch{ 10 * std::ranges::distance(assembledTestSuites | std::views::transform([](const auto& suite) | ||
| { | ||
| return suite.testCases; | ||
| }) | | ||
| std::views::join) }; | ||
|
|
||
| for (const auto& assembledTestSuite : assembledTestSuites) | ||
| for (const auto& assembledTestCase : assembledTestSuite.testCases) | ||
| for (auto i{ 0 }; i < 10; ++i) | ||
| tasks.emplace_back(RunTestCase(threadPool, taskLatch, parallelWorker, assembledTestSuite.gherkinDocument, assembledTestCase, programContext, failing)); |
Comment on lines
+150
to
+160
| coro::latch taskLatch{ 10 * std::ranges::distance(assembledTestSuites | std::views::transform([](const auto& suite) | ||
| { | ||
| return suite.testCases; | ||
| }) | | ||
| std::views::join) }; | ||
|
|
||
| for (const auto& assembledTestSuite : assembledTestSuites) | ||
| for (const auto& assembledTestCase : assembledTestSuite.testCases) | ||
| for (auto i{ 0 }; i < 10; ++i) | ||
| tasks.emplace_back(RunTestCase(threadPool, taskLatch, parallelWorker, assembledTestSuite.gherkinDocument, assembledTestCase, programContext, failing)); | ||
|
|
Comment on lines
+150
to
+155
| coro::latch taskLatch{ 10 * std::ranges::distance(assembledTestSuites | std::views::transform([](const auto& suite) | ||
| { | ||
| return suite.testCases; | ||
| }) | | ||
| std::views::join) }; | ||
|
|
Comment on lines
+1
to
+16
| add_compile_options(-Wno-missing-field-initializers) | ||
|
|
||
| FetchContent_Declare( | ||
| libcoro | ||
| GIT_REPOSITORY https://github.com/jbaldwin/libcoro.git | ||
| GIT_TAG 0161911f260a7507ac1dd4aa26da2e2c092fb550 # v0.16.0 | ||
| SYSTEM | ||
| ) | ||
|
|
||
| set(LIBCORO_FEATURE_TLS OFF CACHE STRING "") | ||
| set(LIBCORO_BUILD_TESTS OFF CACHE STRING "") | ||
| set(LIBCORO_BUILD_EXAMPLES OFF CACHE STRING "") | ||
|
|
||
| FetchContent_MakeAvailable(libcoro) | ||
|
|
||
| target_compile_options(libcoro INTERFACE $<$<COMPILE_LANGUAGE:CXX>:-Wno-extra>) |
| @@ -0,0 +1,170 @@ | |||
| #include "cucumber_cpp/library/runtime/ParallelRuntimeAdapter.hpp" | |||
| #include ".build/Host-Iwyu/_deps/libfmt-src/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.
No description provided.