Test included model folders missing model.config (gazebosim#422)

Currently there is a confusing error message if a file is loaded that finds a folder matching the name of a model to be included but the folder does not have a model.config file. This improves the first error message and stops further loading to prevent additional confusing messages. Signed-off-by: Steve Peters <[email protected]>
scpeters · Dec 14, 2020 · fc33aef · fc33aef
1 parent c5b6db1
commit fc33aef
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 1 deletion.
diff --git a/src/parser.cc b/src/parser.cc
@@ -774,7 +774,8 @@ std::string getModelFilePath(const std::string &_modelDirPath)
     if (!sdf::filesystem::exists(configFilePath))
     {
       // We didn't find manifest.xml either, output an error and get out.
-      sdferr << "Could not find model.config or manifest.xml for the model\n";
+      sdferr << "Could not find model.config or manifest.xml in ["
+             << _modelDirPath << "]\n";
       return std::string();
     }
     else
@@ -960,6 +961,15 @@ bool readXml(tinyxml2::XMLElement *_xml, ElementPtr _sdf, Errors &_errors)
 
           // Get the config.xml filename
           filename = getModelFilePath(modelPath);
+
+          if (filename.empty())
+          {
+            _errors.push_back({ErrorCode::URI_LOOKUP,
+                "Unable to resolve uri[" + uri + "] to model path [" +
+                modelPath + "] since it does not contain a model.config " +
+                "file."});
+            continue;
+          }
         }
         else
         {

diff --git a/test/integration/includes.cc b/test/integration/includes.cc
@@ -318,3 +318,40 @@ TEST(IncludesTest, Includes_15_convert)
   EXPECT_EQ("1.6", modelElem->OriginalVersion());
   EXPECT_EQ("1.6", linkElem->OriginalVersion());
 }
+
+//////////////////////////////////////////////////
+TEST(IncludesTest, IncludeModelMissingConfig)
+{
+  sdf::setFindCallback(findFileCb);
+
+  std::ostringstream stream;
+  stream
+    << "<sdf version='" << SDF_VERSION << "'>"
+    << "<include>"
+    << "  <uri>box_missing_config</uri>"
+    << "</include>"
+    << "</sdf>";
+
+  sdf::SDFPtr sdfParsed(new sdf::SDF());
+  sdf::init(sdfParsed);
+  sdf::Errors errors;
+  ASSERT_TRUE(sdf::readString(stream.str(), sdfParsed, errors));
+
+  ASSERT_GE(1u, errors.size());
+  EXPECT_EQ(1u, errors.size());
+  std::cout << errors[0] << std::endl;
+  EXPECT_EQ(errors[0].Code(), sdf::ErrorCode::URI_LOOKUP);
+  EXPECT_NE(std::string::npos, errors[0].Message().find(
+      "Unable to resolve uri[box_missing_config] to model path")) << errors[0];
+  EXPECT_NE(std::string::npos, errors[0].Message().find(
+      "box_missing_config] since it does not contain a model.config file"))
+    << errors[0];
+
+  sdf::Root root;
+  errors = root.Load(sdfParsed);
+  for (auto e : errors)
+    std::cout << e.Message() << std::endl;
+  EXPECT_TRUE(errors.empty());
+
+  EXPECT_EQ(0u, root.ModelCount());
+}
diff --git a/test/integration/model/box_missing_config/model.sdf b/test/integration/model/box_missing_config/model.sdf
@@ -0,0 +1,22 @@
+<?xml version="1.0" ?>
+<sdf version="1.5">
+  <model name="box">
+    <pose>0 0 0.5 0 0 0</pose>
+    <link name="link">
+      <collision name="collision">
+        <geometry>
+          <box>
+            <size>1 1 1</size>
+          </box>
+        </geometry>
+      </collision>
+      <visual name="visual">
+        <geometry>
+          <box>
+            <size>1 1 1</size>
+          </box>
+        </geometry>
+      </visual>
+    </link>
+  </model>
+</sdf>