Skip to content

Fix testcase for invalid value at node initialization#615

Merged
christophfroehlich merged 2 commits into
masterfrom
fix-test-gpl
Jun 21, 2026
Merged

Fix testcase for invalid value at node initialization#615
christophfroehlich merged 2 commits into
masterfrom
fix-test-gpl

Conversation

@christophfroehlich

@christophfroehlich christophfroehlich commented May 18, 2026

Copy link
Copy Markdown
Member

Since PickNikRobotics/generate_parameter_library#339 the behavior of generate_parameter_library changed.
The parameter in the testcase can't even be declared as zero now (earlier it was declared with 0 but the validator failed later).

2026-05-18T07:35:36.9553861Z     [ RUN      ] FilterTest.TestLowPassWrenchFilterInvalidThenFixedParameter
2026-05-18T07:35:36.9555026Z     [ERROR] [1779089735.022610158] [TestLowPassWrenchFilterInvalidThenFixedParameter.TestLowPassFilter]: LowPass filter cannot be configured: parameter 'sampling_frequency' could not be set: Parameter {sampling_frequency} doesn't comply with floating point range. (type : N6rclcpp10exceptions30InvalidParameterValueExceptionE)
2026-05-18T07:35:36.9556068Z     unknown file: Failure
2026-05-18T07:35:36.9556522Z     C++ exception with description "parameter 'sampling_frequency' cannot be set because it was not declared" thrown in the test body.
2026-05-18T07:35:36.9556934Z     
2026-05-18T07:35:36.9557232Z     [  FAILED  ] FilterTest.TestLowPassWrenchFilterInvalidThenFixedParameter (7 ms)

Is this user-facing behavior change?

no

Did you use Generative AI?

no

@saikishor saikishor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get why the parameter failed to declare

@christophfroehlich

Copy link
Copy Markdown
Member Author

The parameter description is updated with correct limits using the rclcpp API. This now fails early at the time of parameter declaration. The test wasn't designed properly here.

@christophfroehlich

Copy link
Copy Markdown
Member Author

But I need to release generate_parameter_library to make the new test succeed.

@codecov-commenter

codecov-commenter commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.60%. Comparing base (0bd9f25) to head (ee8feb0).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #615      +/-   ##
==========================================
- Coverage   83.61%   83.60%   -0.01%     
==========================================
  Files          31       31              
  Lines        2203     2202       -1     
  Branches      128      126       -2     
==========================================
- Hits         1842     1841       -1     
  Misses        277      277              
  Partials       84       84              
Flag Coverage Δ
unittests 83.60% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...lbox/test/control_filters/test_low_pass_filter.cpp 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@christophfroehlich christophfroehlich moved this to Needs review in Review triage Jun 10, 2026
@christophfroehlich christophfroehlich linked an issue Jun 18, 2026 that may be closed by this pull request
This was unlinked from issues Jun 19, 2026
@christophfroehlich christophfroehlich enabled auto-merge (squash) June 19, 2026 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-humble Triggers PR backport to ROS 2 humble. backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted.

Projects

Status: Done

3 participants