Add tests that check sdf::Errors is propagated properly

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>

include/sdf/Element.hh

@@ -1016,7 +1016,7 @@ namespace sdf
     std::pair<T, bool> result = this->Get<T>(errors, _key, _defaultValue);
     for(auto& error : errors)
     {
-      error.ThrowOrPrintError(sdferr);
+      internal::throwOrPrintError(sdferr, error);
     }
     return result;
   }
@@ -1069,7 +1069,7 @@ namespace sdf
     bool result = this->Set<T>(errors, _value);
     for(auto& error : errors)
     {
-      error.ThrowOrPrintError(sdferr);
+      internal::throwOrPrintError(sdferr, error);
     }
     return result;
   }

src/Element.cc

@@ -1143,8 +1143,13 @@ std::map<std::string, std::size_t> Element::CountNamedElements(
       // Get("name") returns attribute value if it exists before checking
       // for the value of a child element <name>, so it's safe to use
       // here since we've checked HasAttribute("name").
+      std::size_t lastErrorIndex = _errors.size();
       std::string childNameAttributeValue = elem->Get<std::string>(
           _errors, "name");
+      if (hasFatalError(_errors, lastErrorIndex))
+      {
+        return result;
+      }
       if (result.find(childNameAttributeValue) == result.end())
       {
         result[childNameAttributeValue] = 1;
@@ -1235,8 +1240,13 @@ ElementPtr Element::AddElement(const std::string &_name, sdf::Errors &_errors)
   {
     for (unsigned int i = 0; i < parent->GetElementDescriptionCount(); ++i)
     {
+      std::size_t lastErrorIndex = _errors.size();
       this->dataPtr->elementDescriptions.push_back(
-        parent->GetElementDescription(i)->Clone(_errors));
+          parent->GetElementDescription(i)->Clone(_errors));
+      if (hasFatalError(_errors, lastErrorIndex))
+      {
+        return ElementPtr();
+      }
     }
   }
 
@@ -1246,7 +1256,13 @@ ElementPtr Element::AddElement(const std::string &_name, sdf::Errors &_errors)
   {
     if ((*iter)->dataPtr->name == _name)
     {
+      std::size_t lastErrorIndex = _errors.size();
       ElementPtr elem = (*iter)->Clone(_errors);
+      if (hasFatalError(_errors, lastErrorIndex))
+      {
+        return ElementPtr();
+      }
+
       elem->SetParent(shared_from_this());
       this->dataPtr->elements.push_back(elem);
 
@@ -1257,7 +1273,12 @@ ElementPtr Element::AddElement(const std::string &_name, sdf::Errors &_errors)
         // Add only required child element
         if ((*iter2)->GetRequired() == "1")
         {
+          lastErrorIndex = _errors.size();
           elem->AddElement((*iter2)->dataPtr->name, _errors);
+          if (hasFatalError(_errors, lastErrorIndex))
+          {
+            return ElementPtr();
+          }
         }
       }
       return this->dataPtr->elements.back();

src/Element_TEST.cc

@@ -1091,3 +1091,65 @@ TEST(Element, FindElement)
     EXPECT_EQ("first_child", childElemB->GetAttribute("name")->GetAsString());
   }
 }
+
+// [x] Get
+// [x] Set
+// [x] ToString
+// [x] PrintValues
+// [ ] HasUniqueChildNames
+// [ ] CountNamedElements
+// [ ] Update
+// [x] PrintAttributes
+/////////////////////////////////////////////////
+TEST(Element, ErrorPropagation)
+{
+  // This test only checks that the `sdf::Errors` parameter is propagated to
+  // each function that could emit an error. The functions that actually emit
+  // the errors are in `sdf::Param`, so just checking if the resulting `errors`
+  // object is non empty is sufficient.
+  sdf::ElementPtr elem = std::make_shared<sdf::Element>();
+  {
+    sdf::Errors errors;
+    elem->AddValue("pose", "non_pose_default", true, errors, "");
+    EXPECT_FALSE(errors.empty());
+  }
+  {
+    sdf::Errors errors;
+    elem->Set(errors, "non_pose_value");
+    EXPECT_FALSE(errors.empty());
+  }
+  {
+    sdf::Errors errors;
+    elem->Get<int>(errors);
+    EXPECT_FALSE(errors.empty());
+  }
+  {
+    sdf::Errors errors;
+    elem->ToString(errors, "");
+    EXPECT_FALSE(errors.empty());
+  }
+  {
+    sdf::Errors errors;
+    elem->PrintValues(errors, "");
+    EXPECT_FALSE(errors.empty());
+  }
+
+  {
+    sdf::Errors errors;
+    elem->AddAttribute("test_attr", "pose", "non_pose_default", true, errors);
+    EXPECT_FALSE(errors.empty());
+  }
+  {
+    sdf::Errors errors;
+    auto attr = elem->GetAttribute("test_attr");
+    ASSERT_NE(nullptr, attr);
+    attr->Set("non_pose_value", errors);
+    EXPECT_FALSE(errors.empty());
+  }
+  {
+    sdf::Errors errors;
+    elem->Update(errors);
+    // Should emit error because update function is not set.
+    EXPECT_FALSE(errors.empty());
+  }
+}

src/Utils.cc

@@ -167,14 +167,27 @@ void enforceConfigurablePolicyCondition(
 }
 
 /////////////////////////////////////////////////
-void throwOrPrintErrors(const sdf::Errors& _errors)
+void throwOrPrintErrors(const sdf::Errors &_errors)
 {
   for(auto& error : _errors)
   {
     internal::throwOrPrintError(sdferr, error);
   }
 }
 
+/////////////////////////////////////////////////
+bool hasFatalError(const sdf::Errors &_errors, std::size_t _startIndex)
+{
+  auto isFatalError = [](const sdf::Error &_err)
+  {
+    return _err.Code() == ErrorCode::FATAL_ERROR;
+  };
+
+  auto start = _errors.begin();
+  std::advance(start, _startIndex);
+  return _errors.end() != std::find_if(start, _errors.end(), isFatalError);
+}
+
 /////////////////////////////////////////////////
 /// \brief Compute the absolute name of an entity by walking up the element
 /// tree.

src/Utils.hh

@@ -88,7 +88,10 @@ namespace sdf
   /// \brief It will print the errors to sdferr or throw them using
   /// SDF_ASSERT depending on their ErrorCode.
   /// \param[in] _errors  The vector of errors.
-  void throwOrPrintErrors(const sdf::Errors& _errors);
+  void throwOrPrintErrors(const sdf::Errors &_errors);
+
+  /////////////////////////////////////////////////
+  bool hasFatalError(const sdf::Errors &_errors, std::size_t _startIndex = 0);
 
   /// \brief Load all objects of a specific sdf element type. No error
   /// is returned if an element is not present. This function assumes that