feat(taskfiles): Add tasks to install boost; Add test for boost install tasks. #42
Conversation
## Walkthrough
This update introduces a comprehensive Boost C++ library integration and testing workflow. It adds a set of YAML taskfiles to automate downloading, building, installing, and testing Boost, including a dedicated C++ test program that exercises multiple Boost components. The test orchestration is modularized, with reusable tasks for dependency management and test execution. The main test runner now delegates to these modular taskfiles, consolidating and simplifying previous inline test logic. Supporting files for CMake configuration and code formatting are also included for the Boost test suite.
## Changes
| File(s) | Change Summary |
|------------------------------------------------------------|---------------|
| exports/taskfiles/utils/boost.yaml | Added a new taskfile automating Boost download, build, install, and CMake settings generation with parameterized internal tasks. |
| exports/taskfiles/utils/utils.yaml | Added `boost: boost.yaml` to the includes section. |
| taskfile.yaml | Modified the `test` task to invoke `tests:all` instead of `tests:download-and-extract-zip-basic`. |
| taskfiles/boost/.clang-format | Added a .clang-format file specifying Boost, C++, and C header grouping priorities. |
| taskfiles/boost/CMakeLists.txt | Added a CMake configuration for building and linking a Boost-based test executable. |
| taskfiles/boost/test_boost.cpp | Added a C++ test program exercising Boost.Program_options, Filesystem, Iostreams, Process, Regex, and URL libraries. |
| taskfiles/boost/tests.yaml | Added a Boost-specific test taskfile orchestrating dependency installation, build, execution, and cleanup. |
| taskfiles/remote/tests.yaml | Added a test taskfile for remote zip download/extract with basic, inclusion, and exclusion tests. |
| taskfiles/tests.yaml | Refactored to delegate all test logic to included `boost/tests.yaml` and `remote/tests.yaml`, consolidating tests into an `all` task. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant Taskfile
participant BoostTasks
participant RemoteTasks
participant CMake
participant BoostTestExe
User->>Taskfile: Run test
Taskfile->>tests.yaml: Invoke all
tests.yaml->>BoostTasks: boost:test
BoostTasks->>BoostTasks: init
BoostTasks->>BoostTasks: install-boost (download, build, install)
BoostTasks->>CMake: Generate build files
BoostTasks->>CMake: Build test executable
BoostTasks->>BoostTestExe: Run test_boost with args
BoostTestExe-->>BoostTasks: Return result
BoostTasks->>BoostTasks: clean
tests.yaml->>RemoteTasks: remote:default
RemoteTasks->>RemoteTasks: Run remote zip tests (basic, exclusions, inclusions)Possibly related PRs
Suggested reviewers
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
tests/boost/CMakeLists.txt (2)
15-16: Including generated CMake settings from tasks.
The pathbuild/deps/cmake-settings/all.cmakematches the taskfile’s output directory. Consider guarding it withif(EXISTS ...)if you want to fail more gracefully when settings aren’t generated.
27-32: Executable target and linkage are correctly configured.
Usingtarget_sourcesand linkingBoost::program_optionsandBoost::systemprivately is appropriate. Optionally, you could combine into a singleadd_executable(boost-test boost-test.cpp)for brevity, but the current form is clear.tests/boost/boost-test.yaml (2)
6-15: Consistent naming for directory variables.
Variables are inUPPER_SNAKE_CASE, which is fine, but consider aligning naming conventions with the rest of your Taskfiles (e.g., lower_snake_case). Also, you may collapse redundant levels if there’s a commonG_BUILD_DIR.
23-31: Reconfigure on source changes.
Theconfig-cmake-projecttask only watches the Taskfile andCMakeLists.txt. Ifboost-test.cppchanges, CMake cache won’t regenerate automatically. Consider adding:sources: - "{{.TASKFILE}}" - "CMakeLists.txt" - "boost-test.cpp"so that a code update triggers a reconfigure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
exports/taskfiles/utils/boost.yaml(1 hunks)exports/taskfiles/utils/utils.yaml(1 hunks)taskfile.yaml(2 hunks)tests/boost/.clang-format(1 hunks)tests/boost/CMakeLists.txt(1 hunks)tests/boost/boost-test.cpp(1 hunks)tests/boost/boost-test.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
tests/boost/boost-test.cpp
🔇 Additional comments (11)
tests/boost/.clang-format (1)
1-20: Format configuration looks good!The .clang-format file is well-structured with clear include categories and appropriate priorities. It correctly inherits from the parent configuration and establishes a proper hierarchy for Boost headers, C system headers, and C++ standard library headers.
exports/taskfiles/utils/boost.yaml (1)
1-167: Well-structured and thorough implementation of Boost tasksThe taskfile provides a comprehensive set of utilities for downloading, building, and installing Boost. The tasks are well-documented with clear parameters and dependencies. The implementation correctly handles the Boost build process, which doesn't use CMake, and provides proper parameter customization.
exports/taskfiles/utils/utils.yaml (1)
4-4: Integration looks goodThe boost.yaml taskfile is properly integrated into the utils.yaml file, maintaining alphabetical order of includes.
taskfile.yaml (2)
5-5: Boost test integration looks goodThe boost-test taskfile is properly included in the main taskfile.
21-21: Test task integration looks goodThe boost-test task is properly integrated into the main test task.
tests/boost/boost-test.cpp (1)
11-62: Overall implementation is solidThe Boost test program is well-structured with proper error handling, exception catching, and validation. It correctly demonstrates the use of Boost.Program_options for argument parsing.
tests/boost/CMakeLists.txt (2)
1-7: Project and CMake minimum version are well-defined.
Settingcmake_minimum_required(VERSION 3.22.1)along withproject(boost-test LANGUAGES CXX VERSION 0.1.0)and enforcing C++20 is clear and correct.
8-13: Enabling export of compile commands is appropriate for tooling.
The cachedCMAKE_EXPORT_COMPILE_COMMANDSoption will generatecompile_commands.json, which is helpful for linters and IDE integration.tests/boost/boost-test.yaml (3)
3-5: Ensure referenced utils include contains Boost and CMake tasks.
You’re includingutils.yamlunder the aliasutils, but later invokeutils:boost:boost-download-and-installandutils:cmake:install-deps-and-generate-settings. Please verify thatexports/taskfiles/utils/utils.yamlbrings in both theboostandcmakenamespaces as expected.
65-71:run-boost-testtask invocation looks correct.
Calling the compiledboost-testbinary with flags--input 'testtest' --size 8aligns with the test code.
72-83: Compositeboost-testworkflow is well-sequenced.
You initialise, install Boost, configure, build, run tests, and clean up in the correct order. This integrates cleanly into the main test suite.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…dev-utils into boost-install-task
There was a problem hiding this comment.
Lets reorganize the test files a bit:
- a file to include all the tests (like utils.yaml):
./taskfiles/test.yaml - move the current tests.yaml to:
./taskfiles/tests/remote.yaml - move your
./tests/boost/directory to./taskfiles/tests/boost/ - move your
./tests/boost/boost-test.yamlto./taskfiles/tests/boost/tests.yaml
Co-authored-by: davidlion <[email protected]>
…dev-utils into boost-install-task
taskfiles/boost/boost-test.cpp
Outdated
| desc.add_options()("size", boost::program_options::value<int>(), "Size of the string."); | ||
| boost::program_options::variables_map variables; | ||
| boost::program_options::store( | ||
| // NOLINTNEXTLINE(misc-include-cleaner) |
There was a problem hiding this comment.
Is there really no way to properly include boost::program_options::parse_command_line?
Co-authored-by: davidlion <[email protected]>
This reverts commit 0ab8ca5.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
taskfiles/boost/test_boost.cpp (3)
27-40: Command-line argument handling implemented correctly, but missing help message support.The argument parsing is correctly implemented using Boost.Program_options, but the code doesn't provide a way to display the help message when the "--help" option is specified.
Consider adding a check for the help option and displaying the usage when it's specified:
auto parse_args(int argc, char** argv) -> boost::program_options::variables_map { boost::program_options::options_description desc; desc.add_options()("help", "Boost task test. Check the string against its size."); desc.add_options()("input", boost::program_options::value<std::string>(), "Input string."); desc.add_options()("size", boost::program_options::value<int>(), "Size of the string."); boost::program_options::variables_map variables; boost::program_options::store( // NOLINTNEXTLINE(misc-include-cleaner) boost::program_options::parse_command_line(argc, argv, desc), variables ); boost::program_options::notify(variables); + if (variables.count("help")) { + std::cout << desc << "\n"; + std::exit(0); + } return variables; }
59-59: Consider making cWaitTime more descriptive.The constant name could be more descriptive to indicate its units and purpose.
-constexpr int cWaitTime = 10; +constexpr int cWaitTimeMilliseconds = 10;
61-74: Process test may be brittle with fixed timeout values.The test uses fixed timeouts (10ms) which may not be sufficient on slower systems or under high load, potentially causing flaky tests.
Consider increasing the timeout or making it configurable:
-constexpr int cWaitTime = 10; +constexpr int cWaitTimeMilliseconds = 100; // Increased for more reliabilityAnd in the test:
- io_context.run_for(std::chrono::milliseconds(cWaitTime)); - if (std::future_status::ready != result.wait_for(std::chrono::milliseconds(cWaitTime))) { + io_context.run_for(std::chrono::milliseconds(cWaitTimeMilliseconds)); + if (std::future_status::ready != result.wait_for(std::chrono::milliseconds(cWaitTimeMilliseconds))) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
exports/taskfiles/utils/boost.yaml(1 hunks)taskfiles/boost/CMakeLists.txt(1 hunks)taskfiles/boost/test_boost.cpp(1 hunks)taskfiles/boost/tests.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- taskfiles/boost/CMakeLists.txt
🚧 Files skipped from review as they are similar to previous changes (2)
- taskfiles/boost/tests.yaml
- exports/taskfiles/utils/boost.yaml
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
taskfiles/boost/test_boost.cpp
🔇 Additional comments (10)
taskfiles/boost/test_boost.cpp (10)
8-24: Well-organized includes demonstrating good Boost component integration.The includes are well-organized with standard library headers first, followed by Boost component headers. The code demonstrates a good breadth of Boost components: program_options, filesystem, iostreams, process, regex, and URL. This seems appropriate for a comprehensive test suite.
42-45: Simple filesystem test looks good.This test properly checks that the current source file has a parent path, which is a good basic verification of the Boost.Filesystem library functionality.
47-57: Iostreams test is well-implemented with proper exception handling.The test correctly creates a filtering stream, writes to it, flushes it, and verifies the output. The try-catch block properly handles potential exceptions.
76-78: Regex constants look good.The regex pattern and test string are appropriately defined as constants.
79-88: Regex test is well-implemented.The test correctly creates a regex pattern and verifies that it matches the expected input string. The exception handling is appropriate.
90-101: URL test correctly validates the parsed URL component.The test properly parses a complex URL and verifies the host component. Error handling is done correctly through the result type.
103-110: Good use of error code constants.Using named constants for error codes improves readability and maintainability. Each test has its distinct error code.
112-136: Command-line argument validation is thorough with proper error handling.The code correctly validates the presence of both required arguments and handles type conversion errors appropriately. The error messages are clear and helpful.
137-160: Test execution flow is clear and follows the coding guidelines.The main function correctly verifies the input size and runs all tests sequentially. The code consistently uses
false == <expression>pattern as required by the coding guidelines.
1-163: Overall well-structured test program with comprehensive Boost component testing.This test program effectively validates multiple Boost components through focused test functions. It provides clear error reporting with distinct error codes for different failure scenarios. The code follows the project's coding guidelines and demonstrates good error handling practices.
Some minor improvements could be made to the command-line argument handling and timeout values, but these are not critical issues.
taskfiles/boost/test_boost.cpp
Outdated
There was a problem hiding this comment.
@coderabbitai Please make an issue to use Catch2 for testing proper installation and linking of C++ libraries installed through task.
There was a problem hiding this comment.
🧩 Analysis chain
@davidlion I've created an issue as requested to suggest using Catch2 for testing proper installation and linking of C++ libraries installed through task. The issue outlines the benefits of using Catch2 compared to the current custom testing approach and proposes implementation steps. I've assigned the issue to you for tracking.
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue to suggest using Catch2 for testing C++ libraries installed through task
gh issue create \
--title "Use Catch2 for testing proper installation and linking of C++ libraries installed through task" \
--body "## Background
The current approach for testing C++ libraries installed through task (e.g., Boost in PR #42) uses custom C++ programs with manual validation logic and error handling.
For example, in \`taskfiles/boost/test_boost.cpp\`, the current implementation:
- Uses a custom main function with manual command-line argument parsing
- Implements individual test functions that return boolean success/failure
- Manually tracks and reports different error codes for various test failures
## Proposal
Replace the current custom testing approach with [Catch2](https://github.com/catchorg/Catch2), a modern C++ test framework that would provide:
- Simpler test declarations with REQUIRE/CHECK macros
- Automatic test registration and discovery
- Better test output with clear failure messages
- Test sections and fixtures for better organization
- Built-in command-line parser with test filtering options
- Seamless integration with CI systems and test report generation
- Reduced boilerplate code
## Implementation Steps
1. Add a task to download and install Catch2 (similar to how Boost is installed)
2. Update the test C++ programs to use Catch2's test macros and framework
3. Modify the task running the tests to capture and report Catch2 test results
4. Consider adding this as a standard approach for all C++ library testing tasks
This would standardize testing across all C++ libraries and make it easier to add new tests in the future.
## References
- [Catch2 GitHub repository](https://github.com/catchorg/Catch2)
- [Catch2 documentation](https://github.com/catchorg/Catch2/blob/devel/docs/tutorial.md)" \
--assignee "davidlion"
echo "Exit status: $?"
Length of output: 1863
@davidlion I've created GitHub issue #43 titled "Use Catch2 for testing proper installation and linking of C++ libraries installed through task" and assigned it to you. You can view it here: #43
The issue includes details about the current testing approach, the benefits of switching to Catch2, and proposed implementation steps.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
Co-authored-by: davidlion <[email protected]>
Co-authored-by: davidlion <[email protected]>
…dev-utils into boost-install-task
Co-authored-by: davidlion <[email protected]>
Description
Boostdoes not support build withcmake, so we cannot use the tasks fromcmake.yamlto installBoost.This PR adds:
Boost.Boost.C++project that uses the installedBoostlibrary as dependency.Checklist
breaking change.
Validation performed
task tests:boostran successfully.Summary by CodeRabbit
New Features
Refactor
Chores