Feature/test coverage #6
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a Codecov coverage GitHub Actions workflow and README badge, introduces Python toolchain and rules_python in Bazel, adds Bazel/CMake test targets and GoogleTest suite for FixedIDArray, broadens the IDMapExtractor concept to accept optional-like and plain types, and updates a workflow cache action. Changes
Sequence Diagram(s)(omitted — changes do not introduce a multi-component runtime control-flow that benefits from a sequence diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
motorium/motorium_model/include/motorium_model/FixedIDArray.h (1)
1-1: Pipeline failure: Abseil ABI/version mismatch during test linking.The test executable
test_fixed_id_arrayfailed to link due to missing Abseil log symbols (ABI/version mismatch). This is a build configuration issue, not a problem with the FixedIDArray.h header itself. The test target transitively depends on Abseil through the motorium_model library, and the mismatch likely stems from incompatible Abseil versions between dependencies.Recommended steps to resolve:
- Check Abseil version consistency: Ensure all dependencies, particularly motorium_core, use compatible Abseil versions.
- Review test build configuration: Verify that Abseil dependencies are properly linked for the test target in both
motorium/motorium_model/BUILD.bazelandmotorium/motorium_model/CMakeLists.txt.- Verify compiler flags and ABI settings: Ensure consistent compiler flags and ABI compatibility across all Abseil-dependent targets.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/coverage.ymlREADME.mdmotorium/motorium_model/BUILD.bazelmotorium/motorium_model/CMakeLists.txtmotorium/motorium_model/include/motorium_model/FixedIDArray.hmotorium/motorium_model/test/testFixedIDArray.cpp
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/coverage.yml
22-22: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
25-25: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
34-34: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
66-66: the runner of "codecov/codecov-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 Cppcheck (2.19.0)
motorium/motorium_model/test/testFixedIDArray.cpp
[information] 1-1: Include file
(missingIncludeSystem)
[information] 3-3: Include file
(missingIncludeSystem)
[information] 4-4: Include file
(missingInclude)
🪛 GitHub Actions: Colcon Build
motorium/motorium_model/BUILD.bazel
[error] 1-1: Command failed during linking: the test executable 'test_fixed_id_array' could not be produced due to missing Abseil log symbols (ABI/version mismatch).
motorium/motorium_model/CMakeLists.txt
[error] 1-1: Command failed during linking: the test executable 'test_fixed_id_array' could not be produced due to missing Abseil log symbols (ABI/version mismatch).
motorium/motorium_model/include/motorium_model/FixedIDArray.h
[error] 1-1: Command failed during linking: the test executable 'test_fixed_id_array' could not be produced due to missing Abseil log symbols (ABI/version mismatch).
motorium/motorium_model/test/testFixedIDArray.cpp
[error] 1-1: Linking failed: undefined references to absl::lts_20250814::log_internal::LogMessageFatal and related Abseil log symbols. Possible ABI mismatch or missing Abseil libraries when building test_fixed_id_array.
🔇 Additional comments (8)
README.md (1)
6-7: LGTM! Codecov badge integration is correct.The codecov badge properly references the main branch and links to the Codecov dashboard, aligning with the new Coverage workflow introduced in
.github/workflows/coverage.yml.motorium/motorium_model/test/testFixedIDArray.cpp (6)
1-16: LGTM! Well-structured test setup.The test includes proper headers and defines a helper struct with appropriate equality operator for testing non-trivial types.
20-31: LGTM! Comprehensive initialization tests.The tests properly cover:
- Valid size initialization
- Zero size failure (using EXPECT_DEATH)
- Too large size failure (using EXPECT_DEATH)
The death tests ensure proper error handling for invalid initialization scenarios.
33-46: LGTM! Thorough access validation.The tests verify both
operator[]andat()methods for valid indices and properly use EXPECT_DEATH to check out-of-bounds access failures.
48-55: LGTM! Const-correctness validation.The test properly verifies that const references to FixedIDArray provide read-only access through both accessor methods.
57-75: LGTM! Assignment behavior properly tested.The tests validate successful copy assignment and properly check for size mismatch failures using EXPECT_DEATH.
90-150: LGTM! Comprehensive Eigen integration tests.Excellent coverage of
toEigenVectorfunctionality across multiple scenarios:
- Simple types (double)
- Optional-wrapped types with default values for nullopt
- Custom structs with member extraction
- Optional-wrapped structs
The tests validate both the extraction logic and proper handling of missing values (nullopt) with defaults.
motorium/motorium_model/BUILD.bazel (1)
29-36: Remove proposed Abseil dependencies—they are already available transitively.The test target
test_fixed_id_arraydepends on:motorium_model, which depends on//motorium/motorium_core. Themotorium_corelibrary explicitly declares@com_google_absl//absl/log:checkand@com_google_absl//absl/log:absl_login its deps. These transitive dependencies are automatically available to any target that depends onmotorium_model, making the proposed additions redundant. FixedIDArray.h includesmotorium_core/Check.h, which uses these Abseil modules, and the dependency chain is already complete.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
MODULE.bazelREADME.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: bazel-build
- GitHub Check: coverage
🔇 Additional comments (1)
README.md (1)
6-7: LGTM! Coverage badge addition is well-formatted.The Codecov badge follows the standard pattern and is correctly positioned with the other CI status badges. This will provide visibility into test coverage metrics for the project.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Summary by CodeRabbit
Tests
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.