Use findProperty instead of childAt where possible

- when adding properties, tests will not suddenly fail - makes the test more explicit
ros2 · Jun 15, 2018 · cae549b · cae549b
1 parent 281e204
commit cae549b
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 25 deletions.
diff --git a/rviz_default_plugins/test/rviz_default_plugins/displays/marker/marker_common_test.cpp b/rviz_default_plugins/test/rviz_default_plugins/displays/marker/marker_common_test.cpp
@@ -359,14 +359,15 @@ TEST_F(MarkerCommonFixture, update_does_not_retransform_normal_messages) {
 TEST_F(MarkerCommonFixture, processMessage_adds_new_namespace_for_message) {
   mockValidTransform();
 
-  ASSERT_THAT(display_->childAt(0)->numChildren(), Eq(0));
+  ASSERT_THAT(display_->findProperty("Namespaces")->numChildren(), Eq(0));
 
   auto marker = createSharedPtrMessage(
     visualization_msgs::msg::Marker::ADD, visualization_msgs::msg::Marker::POINTS);
 
   common_->processMessage(marker);
 
-  ASSERT_THAT(display_->childAt(0)->childAt(0)->getName().toStdString(), StrEq("test_ns"));
+  ASSERT_THAT(display_->findProperty("Namespaces")->childAt(0)->getName().toStdString(),
+    StrEq("test_ns"));
 }
 
 TEST_F(MarkerCommonFixture, processMessage_does_not_add_new_namespace_if_already_present) {
@@ -375,14 +376,16 @@ TEST_F(MarkerCommonFixture, processMessage_does_not_add_new_namespace_if_already
   common_->processMessage(createSharedPtrMessage(
       visualization_msgs::msg::Marker::ADD, visualization_msgs::msg::Marker::POINTS));
 
-  ASSERT_THAT(display_->childAt(0)->numChildren(), Eq(1));
-  ASSERT_THAT(display_->childAt(0)->childAt(0)->getName().toStdString(), StrEq("test_ns"));
+  ASSERT_THAT(display_->findProperty("Namespaces")->numChildren(), Eq(1));
+  ASSERT_THAT(display_->findProperty("Namespaces")->childAt(0)->getName().toStdString(),
+    StrEq("test_ns"));
 
   common_->processMessage(createSharedPtrMessage(
       visualization_msgs::msg::Marker::ADD, visualization_msgs::msg::Marker::TEXT_VIEW_FACING));
 
-  ASSERT_THAT(display_->childAt(0)->numChildren(), Eq(1));
-  ASSERT_THAT(display_->childAt(0)->childAt(0)->getName().toStdString(), StrEq("test_ns"));
+  ASSERT_THAT(display_->findProperty("Namespaces")->numChildren(), Eq(1));
+  ASSERT_THAT(display_->findProperty("Namespaces")->childAt(0)->getName().toStdString(),
+    StrEq("test_ns"));
 }
 
 TEST_F(MarkerCommonFixture, onEnableChanged_in_namespace_removes_all_markers_in_that_namespace) {
@@ -400,7 +403,7 @@ TEST_F(MarkerCommonFixture, onEnableChanged_in_namespace_removes_all_markers_in_
   EXPECT_TRUE(rviz_rendering::findOneMovableText(scene_manager_->getRootSceneNode()));
 
   auto namespace_property = dynamic_cast<rviz_default_plugins::displays::MarkerNamespace *>(
-    display_->childAt(0)->childAt(0));
+    display_->findProperty("Namespaces")->childAt(0));
   namespace_property->setValue(false);
 
   EXPECT_FALSE(rviz_rendering::findOnePointCloud(scene_manager_->getRootSceneNode()));
@@ -419,7 +422,7 @@ TEST_F(MarkerCommonFixture, processMessage_does_not_add_message_with_disabled_na
   EXPECT_TRUE(rviz_rendering::findOnePointCloud(scene_manager_->getRootSceneNode()));
 
   auto namespace_property = dynamic_cast<rviz_default_plugins::displays::MarkerNamespace *>(
-    display_->childAt(0)->childAt(0));
+    display_->findProperty("Namespaces")->childAt(0));
   namespace_property->setValue(false);
 
   marker->type = visualization_msgs::msg::Marker::TEXT_VIEW_FACING;

diff --git a/rviz_default_plugins/test/rviz_default_plugins/displays/path/path_display_test.cpp b/rviz_default_plugins/test/rviz_default_plugins/displays/path/path_display_test.cpp
@@ -149,9 +149,7 @@ TEST_F(PathTestFixture, reset_is_idempotent) {
 }
 
 TEST_F(PathTestFixture, reset_removes_all_axes) {
-  auto pose_style_widget_index = 8;
-  ASSERT_THAT(path_display_->childAt(pose_style_widget_index)->getNameStd(), StrEq("Pose Style"));
-  path_display_->childAt(pose_style_widget_index)->setValue("Axes");
+  path_display_->findProperty("Pose Style")->setValue("Axes");
 
   auto position = Ogre::Vector3::ZERO;
   auto orientation = Ogre::Quaternion::IDENTITY;
@@ -165,9 +163,7 @@ TEST_F(PathTestFixture, reset_removes_all_axes) {
 }
 
 TEST_F(PathTestFixture, reset_removes_all_arrows) {
-  auto pose_style_widget_index = 8;
-  ASSERT_THAT(path_display_->childAt(pose_style_widget_index)->getNameStd(), StrEq("Pose Style"));
-  path_display_->childAt(pose_style_widget_index)->setValue("Arrows");
+  path_display_->findProperty("Pose Style")->setValue("Arrows");
 
   auto position = Ogre::Vector3::ZERO;
   auto orientation = Ogre::Quaternion::IDENTITY;
@@ -196,9 +192,7 @@ TEST_F(PathTestFixture, processMessage_transforms_the_vertices_correctly) {
 }
 
 TEST_F(PathTestFixture, processMessage_adds_billboard_line_to_scene) {
-  auto line_style_widget_index = 2;
-  ASSERT_THAT(path_display_->childAt(line_style_widget_index)->getNameStd(), StrEq("Line Style"));
-  path_display_->childAt(line_style_widget_index)->setValue("Billboards");
+  path_display_->findProperty("Line Style")->setValue("Billboards");
 
   auto position = Ogre::Vector3::ZERO;
   auto orientation = Ogre::Quaternion::IDENTITY;
@@ -215,9 +209,7 @@ TEST_F(PathTestFixture, processMessage_adds_billboard_line_to_scene) {
 }
 
 TEST_F(PathTestFixture, processMessage_adds_axes_to_scene) {
-  auto pose_style_widget_index = 8;
-  ASSERT_THAT(path_display_->childAt(pose_style_widget_index)->getNameStd(), StrEq("Pose Style"));
-  path_display_->childAt(pose_style_widget_index)->setValue("Axes");
+  path_display_->findProperty("Pose Style")->setValue("Axes");
 
   auto position = Ogre::Vector3::ZERO;
   auto orientation = Ogre::Quaternion::IDENTITY;
@@ -234,8 +226,7 @@ TEST_F(PathTestFixture, processMessage_adds_axes_to_scene) {
 }
 
 TEST_F(PathTestFixture, processMessage_adds_arrows_to_scene) {
-  ASSERT_EQ("Pose Style", path_display_->childAt(8)->getNameStd());
-  path_display_->childAt(8)->setValue("Arrows");
+  path_display_->findProperty("Pose Style")->setValue("Arrows");
 
   auto position = Ogre::Vector3::ZERO;
   auto orientation = Ogre::Quaternion::IDENTITY;

diff --git a/...default_plugins/test/rviz_default_plugins/displays/pointcloud/point_cloud_common_test.cpp b/...default_plugins/test/rviz_default_plugins/displays/pointcloud/point_cloud_common_test.cpp
@@ -167,9 +167,7 @@ TEST_F(PointCloudCommonTestFixture, update_colors_the_points_using_the_selected_
 
   mockValidTransform();
 
-  auto color_transformer_property = parent_display_->childAt(7);
-  ASSERT_THAT(color_transformer_property->getNameStd(), StrEq("Color Transformer"));
-
+  auto color_transformer_property = parent_display_->findProperty("Color Transformer");
   color_transformer_property->setValue("FlatColor");
 
   auto color_property = parent_display_->findProperty("Color");