Skip to content

Commit

Permalink
Test included model folders missing model.config (#422)
Browse files Browse the repository at this point in the history
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 <scpeters@openrobotics.org>
  • Loading branch information
scpeters committed Dec 15, 2020
1 parent 68bb12e commit 2f00ecf
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 1 deletion.
12 changes: 11 additions & 1 deletion src/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
{
Expand Down
37 changes: 37 additions & 0 deletions test/integration/includes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
22 changes: 22 additions & 0 deletions test/integration/model/box_missing_config/model.sdf
Original file line number Diff line number Diff line change
@@ -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>

0 comments on commit 2f00ecf

Please sign in to comment.