Fix control_filters tests (#261)

* Make tests fail fast

* Use common test_fixture and don't start a separate thread

* Remove the executor

Author: christophfroehlich

CMakeLists.txt

@@ -165,16 +165,19 @@ if(BUILD_TESTING)
   )
   target_link_libraries(test_low_pass_filter low_pass_filter low_pass_filter_parameters)
   ament_target_dependencies(test_low_pass_filter ${CONTROL_FILTERS_INCLUDE_DEPENDS})
+  set_tests_properties(test_low_pass_filter PROPERTIES TIMEOUT 2)
 
   ament_add_gmock(test_load_low_pass_filter test/control_filters/test_load_low_pass_filter.cpp)
   target_link_libraries(test_load_low_pass_filter low_pass_filter low_pass_filter_parameters)
   ament_target_dependencies(test_load_low_pass_filter ${CONTROL_FILTERS_INCLUDE_DEPENDS})
 
+  # rate_limiter
   add_rostest_with_parameters_gmock(test_rate_limiter test/control_filters/test_rate_limiter.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/test/control_filters/test_rate_limiter_parameters.yaml
   )
   target_link_libraries(test_rate_limiter rate_limiter rate_limiter_parameters)
   ament_target_dependencies(test_rate_limiter ${CONTROL_FILTERS_INCLUDE_DEPENDS})
+  set_tests_properties(test_rate_limiter PROPERTIES TIMEOUT 2)
 
   ament_add_gmock(test_load_rate_limiter test/control_filters/test_load_rate_limiter.cpp)
   target_link_libraries(test_load_rate_limiter rate_limiter rate_limiter_parameters)

test/control_filters/test_exponential_filter.cpp

@@ -12,9 +12,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "test_exponential_filter.hpp"
+#include "test_filter_util.hpp"
 
-TEST_F(ExponentialFilterTest, TestExponentialFilterThrowsUnconfigured)
+#include <memory>
+#include "gmock/gmock.h"
+
+#include "control_filters/exponential_filter.hpp"
+
+TEST_F(FilterTest, TestExponentialFilterThrowsUnconfigured)
 {
     std::shared_ptr<filters::FilterBase<double>> filter_ =
         std::make_shared<control_filters::ExponentialFilter<double>>();
@@ -23,7 +28,7 @@ TEST_F(ExponentialFilterTest, TestExponentialFilterThrowsUnconfigured)
 }
 
 
-TEST_F(ExponentialFilterTest, TestExponentialFilterComputation)
+TEST_F(FilterTest, TestExponentialFilterComputation)
 {
     // parameters should match the test yaml file
     double alpha = 0.7;

test/control_filters/test_exponential_filter.hpp

@@ -12,52 +12,35 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifndef CONTROL_FILTERS__TEST_LOW_PASS_FILTER_HPP_
-#define CONTROL_FILTERS__TEST_LOW_PASS_FILTER_HPP_
+#ifndef CONTROL_FILTERS__TEST_FILTER_UTIL_HPP_
+#define CONTROL_FILTERS__TEST_FILTER_UTIL_HPP_
 
 #include <memory>
-#include <thread>
 #include "gmock/gmock.h"
 
-#include "control_filters/low_pass_filter.hpp"
-#include "geometry_msgs/msg/wrench_stamped.hpp"
-#include "rclcpp/rclcpp.hpp"
+#include "rclcpp/node.hpp"
+#include "rclcpp/logger.hpp"
 
-namespace
-{
-static const rclcpp::Logger LOGGER = rclcpp::get_logger("test_low_pass_filter");
-}  // namespace
-
-class LowPassFilterTest : public ::testing::Test
+class FilterTest : public ::testing::Test
 {
 public:
   void SetUp() override
   {
     auto testname = ::testing::UnitTest::GetInstance()->current_test_info()->name();
     node_ = std::make_shared<rclcpp::Node>(testname);
-    executor_->add_node(node_);
-    executor_thread_ = std::thread([this]() { executor_->spin(); });
   }
 
-  LowPassFilterTest()
+  FilterTest()
   {
-    executor_ = std::make_shared<rclcpp::executors::SingleThreadedExecutor>();
   }
 
   void TearDown() override
   {
-    executor_->cancel();
-    if (executor_thread_.joinable())
-    {
-      executor_thread_.join();
-    }
     node_.reset();
   }
 
 protected:
   rclcpp::Node::SharedPtr node_;
-  rclcpp::Executor::SharedPtr executor_;
-  std::thread executor_thread_;
 };
 
-#endif  // CONTROL_FILTERS__TEST_LOW_PASS_FILTER_HPP_
+#endif  // CONTROL_FILTERS__TEST_FILTER_UTIL_HPP_

test/control_filters/test_low_pass_filter.hpp test/control_filters/test_filter_util.hpp

@@ -12,9 +12,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "test_low_pass_filter.hpp"
+#include "test_filter_util.hpp"
 
-TEST_F(LowPassFilterTest, TestLowPassWrenchFilterAllParameters)
+#include <memory>
+#include "gmock/gmock.h"
+
+#include "geometry_msgs/msg/wrench_stamped.hpp"
+
+#include "control_filters/low_pass_filter.hpp"
+
+TEST_F(FilterTest, TestLowPassWrenchFilterAllParameters)
 {
     std::shared_ptr<filters::FilterBase<geometry_msgs::msg::WrenchStamped>> filter_ =
         std::make_shared<control_filters::LowPassFilter<geometry_msgs::msg::WrenchStamped>>();
@@ -26,12 +33,13 @@ TEST_F(LowPassFilterTest, TestLowPassWrenchFilterAllParameters)
     // change a parameter
     node_->set_parameter(rclcpp::Parameter("sampling_frequency", 500.0));
     // accept second call to configure with valid parameters to already configured filter
+    // will give a warning "Filter %s already being reconfigured"
     ASSERT_TRUE(filter_->configure("", "TestLowPassFilter",
         node_->get_node_logging_interface(), node_->get_node_parameters_interface()));
 }
 
 
-TEST_F(LowPassFilterTest, TestLowPassWrenchFilterMissingParameter)
+TEST_F(FilterTest, TestLowPassWrenchFilterMissingParameter)
 {
     std::shared_ptr<filters::FilterBase<geometry_msgs::msg::WrenchStamped>> filter_ =
         std::make_shared<control_filters::LowPassFilter<geometry_msgs::msg::WrenchStamped>>();
@@ -41,7 +49,7 @@ TEST_F(LowPassFilterTest, TestLowPassWrenchFilterMissingParameter)
         node_->get_node_logging_interface(), node_->get_node_parameters_interface()));
 }
 
-TEST_F(LowPassFilterTest, TestLowPassWrenchFilterInvalidThenFixedParameter)
+TEST_F(FilterTest, TestLowPassWrenchFilterInvalidThenFixedParameter)
 {
     std::shared_ptr<filters::FilterBase<geometry_msgs::msg::WrenchStamped>> filter_ =
         std::make_shared<control_filters::LowPassFilter<geometry_msgs::msg::WrenchStamped>>();
@@ -57,23 +65,23 @@ TEST_F(LowPassFilterTest, TestLowPassWrenchFilterInvalidThenFixedParameter)
         node_->get_node_logging_interface(), node_->get_node_parameters_interface()));
 }
 
-TEST_F(LowPassFilterTest, TestLowPassFilterThrowsUnconfigured)
+TEST_F(FilterTest, TestLowPassFilterThrowsUnconfigured)
 {
     std::shared_ptr<filters::FilterBase<double>> filter_ =
         std::make_shared<control_filters::LowPassFilter<double>>();
     double in, out;
     ASSERT_THROW(filter_->update(in, out), std::runtime_error);
 }
 
-TEST_F(LowPassFilterTest, TestLowPassWrenchFilterThrowsUnconfigured)
+TEST_F(FilterTest, TestLowPassWrenchFilterThrowsUnconfigured)
 {
     std::shared_ptr<filters::FilterBase<geometry_msgs::msg::WrenchStamped>> filter_ =
         std::make_shared<control_filters::LowPassFilter<geometry_msgs::msg::WrenchStamped>>();
     geometry_msgs::msg::WrenchStamped in, out;
     ASSERT_THROW(filter_->update(in, out), std::runtime_error);
 }
 
-TEST_F(LowPassFilterTest, TestLowPassWrenchFilterComputation)
+TEST_F(FilterTest, TestLowPassWrenchFilterComputation)
 {
     // parameters should match the test yaml file
     double sampling_freq = 1000.0;

test/control_filters/test_low_pass_filter.cpp

@@ -12,9 +12,15 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "test_rate_limiter.hpp"
+#include "test_filter_util.hpp"
 
-TEST_F(RateLimiterTest, TestRateLimiterAllParameters)
+#include <memory>
+#include <thread>
+#include "gmock/gmock.h"
+
+#include "control_filters/rate_limiter.hpp"
+
+TEST_F(FilterTest, TestRateLimiterAllParameters)
 {
     std::shared_ptr<filters::FilterBase<double>> filter_ =
         std::make_shared<control_filters::RateLimiter<double>>();
@@ -26,12 +32,13 @@ TEST_F(RateLimiterTest, TestRateLimiterAllParameters)
     // change a parameter
     node_->set_parameter(rclcpp::Parameter("sampling_interval", 0.5));
     // accept second call to configure with valid parameters to already configured filter
+    // will give a warning "Filter %s already being reconfigured"
     ASSERT_TRUE(filter_->configure("", "TestRateLimiter",
         node_->get_node_logging_interface(), node_->get_node_parameters_interface()));
 }
 
 
-TEST_F(RateLimiterTest, TestRateLimiterMissingParameter)
+TEST_F(FilterTest, TestRateLimiterMissingParameter)
 {
     std::shared_ptr<filters::FilterBase<double>> filter_ =
         std::make_shared<control_filters::RateLimiter<double>>();
@@ -41,7 +48,7 @@ TEST_F(RateLimiterTest, TestRateLimiterMissingParameter)
         node_->get_node_logging_interface(), node_->get_node_parameters_interface()));
 }
 
-TEST_F(RateLimiterTest, TestRateLimiterInvalidThenFixedParameter)
+TEST_F(FilterTest, TestRateLimiterInvalidThenFixedParameter)
 {
     std::shared_ptr<filters::FilterBase<double>> filter_ =
         std::make_shared<control_filters::RateLimiter<double>>();
@@ -57,15 +64,15 @@ TEST_F(RateLimiterTest, TestRateLimiterInvalidThenFixedParameter)
         node_->get_node_logging_interface(), node_->get_node_parameters_interface()));
 }
 
-TEST_F(RateLimiterTest, TestRateLimiterThrowsUnconfigured)
+TEST_F(FilterTest, TestRateLimiterThrowsUnconfigured)
 {
     std::shared_ptr<filters::FilterBase<double>> filter_ =
         std::make_shared<control_filters::RateLimiter<double>>();
     double in, out;
     ASSERT_THROW(filter_->update(in, out), std::runtime_error);
 }
 
-TEST_F(RateLimiterTest, TestRateLimiterCompute)
+TEST_F(FilterTest, TestRateLimiterCompute)
 {
     std::shared_ptr<filters::FilterBase<double>> filter_ =
         std::make_shared<control_filters::RateLimiter<double>>();