From ab4b647fc13f3e8073ec80af6ed6863da8424905 Mon Sep 17 00:00:00 2001 From: Jenn Nguyen Date: Thu, 21 Jul 2022 18:30:07 -0700 Subject: [PATCH 1/4] increase utility coverage Signed-off-by: Jenn Nguyen --- src/Utility.cc | 33 +++++++++---- src/Utility_TEST.cc | 113 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 9 deletions(-) diff --git a/src/Utility.cc b/src/Utility.cc index 39072318..a1f5d44c 100644 --- a/src/Utility.cc +++ b/src/Utility.cc @@ -780,10 +780,12 @@ namespace ignition case msgs::PointCloudPacked::Field::FLOAT64: offset += 8; break; + // LCOV_EXCL_START default: std::cerr << "PointCloudPacked field datatype of [" << _type << "] is invalid.\n"; break; + // LCOV_EXCL_STOP } }; @@ -1005,6 +1007,14 @@ namespace ignition } meta.set_name(trimmed(elem->GetText())); + // Read the version, if present. + elem = modelElement->FirstChildElement("version"); + if (elem && elem->GetText()) + { + auto version = std::stoi(trimmed(elem->GetText())); + meta.set_version(version); + } + // Read the description, if present. elem = modelElement->FirstChildElement("description"); if (elem && elem->GetText()) @@ -1101,7 +1111,11 @@ namespace ignition } out << "\n" - << " \n"; + << " \n" + << " " + << _meta.model().file() << "\n"; } else { @@ -1112,15 +1126,16 @@ namespace ignition } out << "\n" - << " \n"; + << " \n" + << " " + << _meta.world().file() << "\n"; } out << " " << _meta.name() << "\n" - << " " << _meta.version() << "\n" - << " " - << _meta.model().file() << "\n" - << " " << _meta.description() << "\n"; + << " " << _meta.version() << "\n" + << " " << _meta.description() << "\n"; // Output author information. for (int i = 0; i < _meta.authors_size(); ++i) @@ -1135,9 +1150,9 @@ namespace ignition for (int i = 0; i < _meta.dependencies_size(); ++i) { out << " \n" - << " " + << " \n" << " " << _meta.dependencies(i).uri() << "\n" - << " " + << " \n" << " \n"; } diff --git a/src/Utility_TEST.cc b/src/Utility_TEST.cc index decc9a6f..78b00db3 100644 --- a/src/Utility_TEST.cc +++ b/src/Utility_TEST.cc @@ -316,6 +316,119 @@ TEST(UtilityTest, ConvertFloat) EXPECT_DOUBLE_EQ(s, 0.999f); } + +///////////////////////////////////////////////// +TEST(UtilityTest, ConvertFuelMetadata) +{ + msgs::FuelMetadata metaMsg; + std::string modelConfigStr; + + // test ConvertFuelMetadata(string, msgs::FuelMetadata) + { + EXPECT_FALSE(msgs::ConvertFuelMetadata(modelConfigStr, metaMsg)); + + metaMsg.Clear(); + modelConfigStr = ""; + EXPECT_FALSE(msgs::ConvertFuelMetadata(modelConfigStr, metaMsg)); + + metaMsg.Clear(); + modelConfigStr = "test"; + EXPECT_FALSE(msgs::ConvertFuelMetadata(modelConfigStr, metaMsg)); + + metaMsg.Clear(); + modelConfigStr = R"( + + test_model + + )"; + EXPECT_FALSE(msgs::ConvertFuelMetadata(modelConfigStr, metaMsg)); + + metaMsg.Clear(); + modelConfigStr = R"( + + model.sdf + test_model + 3 + A model for testing + + Foo Bar + foo@bar.org + + + + model://some_model + + + +)"; + + EXPECT_TRUE(msgs::ConvertFuelMetadata(modelConfigStr, metaMsg)); + EXPECT_EQ("test_model", metaMsg.name()); + EXPECT_EQ(3, metaMsg.version()); + EXPECT_EQ("A model for testing", metaMsg.description()); + EXPECT_EQ("model.sdf", metaMsg.model().file()); + EXPECT_EQ("sdf", metaMsg.model().file_format().name()); + EXPECT_EQ(1, metaMsg.model().file_format().version().major()); + EXPECT_EQ(7, metaMsg.model().file_format().version().minor()); + EXPECT_EQ(1, metaMsg.authors().size()); + EXPECT_EQ("Foo Bar", metaMsg.authors(0).name()); + EXPECT_EQ("foo@bar.org", metaMsg.authors(0).email()); + EXPECT_EQ(1, metaMsg.dependencies().size()); + EXPECT_EQ("model://some_model", metaMsg.dependencies(0).uri()); + } + + // test ConvertFuelMetadata(msgs::FuelMetadata, string) + { + std::string modelConfigOutput; + + // Test + metaMsg.Clear(); + metaMsg.mutable_world()->set_file("world.sdf"); + EXPECT_FALSE(msgs::ConvertFuelMetadata(metaMsg, modelConfigOutput)); + + metaMsg.set_name("test_world"); + metaMsg.set_description("A world for testing"); + metaMsg.set_version(2); + metaMsg.mutable_world()->mutable_file_format()->set_name("sdf"); + metaMsg.mutable_world()->mutable_file_format() + ->mutable_version()->set_major(1); + metaMsg.mutable_world()->mutable_file_format() + ->mutable_version()->set_minor(7); + EXPECT_TRUE(msgs::ConvertFuelMetadata(metaMsg, modelConfigOutput)); + + std::string expectedOutput = R"( + + world.sdf + test_world + 2 + A world for testing + +)"; + EXPECT_EQ(expectedOutput, modelConfigOutput); + + // Test + metaMsg.Clear(); + metaMsg.mutable_model()->set_file("model.sdf"); + EXPECT_FALSE(msgs::ConvertFuelMetadata(metaMsg, modelConfigOutput)); + + metaMsg.set_name("test_model"); + metaMsg.set_description("A model for testing"); + metaMsg.set_version(3); + metaMsg.mutable_model()->mutable_file_format()->set_name("sdf"); + metaMsg.mutable_model()->mutable_file_format() + ->mutable_version()->set_major(1); + metaMsg.mutable_model()->mutable_file_format() + ->mutable_version()->set_minor(7); + EXPECT_TRUE(msgs::ConvertFuelMetadata(metaMsg, modelConfigOutput)); + + metaMsg.add_authors()->set_name("Foo Bar"); + metaMsg.mutable_authors(0)->set_email("foo@bar.org"); + metaMsg.add_dependencies()->set_uri("model://some_model"); + EXPECT_TRUE(msgs::ConvertFuelMetadata(metaMsg, modelConfigOutput)); + EXPECT_EQ(modelConfigStr, modelConfigOutput); + } +} + ///////////////////////////////////////////////// TEST(UtilityTest, SetVector3) { From 8ff8356a98ba47c1206e466f074e68e46e059110 Mon Sep 17 00:00:00 2001 From: Jenn Nguyen Date: Thu, 21 Jul 2022 18:34:24 -0700 Subject: [PATCH 2/4] remove unused function Signed-off-by: Jenn Nguyen --- src/Utility.cc | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/src/Utility.cc b/src/Utility.cc index a1f5d44c..b65a3ef4 100644 --- a/src/Utility.cc +++ b/src/Utility.cc @@ -52,31 +52,6 @@ namespace ignition return _s; } - /// \brief Splits a string into tokens. This was copied from ignition - /// common, ign-common/Util.hh, to avoid adding another dependency. - /// Remove this function if ign-common every becomes a dependency. - /// \param[in] _str Input string. - /// \param[in] _delim Token delimiter. - /// \return Vector of tokens. - std::vector split(const std::string &_str, - const std::string &_delim) - { - std::vector tokens; - char *saveptr; - char *str = strdup(_str.c_str()); - - auto token = ignstrtok(str, _delim.c_str(), &saveptr); - - while (token) - { - tokens.push_back(token); - token = ignstrtok(NULL, _delim.c_str(), &saveptr); - } - - free(str); - return tokens; - } - ///////////////////////////////////////////// ignition::math::Vector3d Convert(const msgs::Vector3d &_v) { From a1d294d585853553e5d89e32c61d3716950f73f2 Mon Sep 17 00:00:00 2001 From: Jenn Nguyen Date: Thu, 21 Jul 2022 19:07:46 -0700 Subject: [PATCH 3/4] updated msg -> string conversion Signed-off-by: Jenn Nguyen --- src/Utility.cc | 34 +++++++++++++++++------ src/Utility_TEST.cc | 66 +++++++++++++++++++++++++++------------------ 2 files changed, 66 insertions(+), 34 deletions(-) diff --git a/src/Utility.cc b/src/Utility.cc index b65a3ef4..8195cf23 100644 --- a/src/Utility.cc +++ b/src/Utility.cc @@ -967,10 +967,17 @@ namespace ignition // Get the top level element. tinyxml2::XMLElement *modelElement = modelConfigDoc.FirstChildElement( "model"); + bool isModel = true; if (!modelElement) { - std::cerr << "Model config string does not contain a element\n"; - return false; + modelElement = modelConfigDoc.FirstChildElement("world"); + if (!modelElement) + { + std::cerr << "Model config string does not contain a " + << " or element\n"; + return false; + } + isModel = false; } // Read the name, which is a mandatory element. @@ -1044,23 +1051,34 @@ namespace ignition math::SemanticVersion ver(trimmed(verStr)); if (ver > maxVer) { - meta.mutable_model()->mutable_file_format()->set_name("sdf"); - ignition::msgs::Version *verMsg = - meta.mutable_model()->mutable_file_format()->mutable_version(); + ignition::msgs::Version *verMsg; + + if (isModel) + { + meta.mutable_model()->mutable_file_format()->set_name("sdf"); + verMsg = + meta.mutable_model()->mutable_file_format()->mutable_version(); + meta.mutable_model()->set_file(trimmed(elem->GetText())); + } + else + { + meta.mutable_world()->mutable_file_format()->set_name("sdf"); + verMsg = + meta.mutable_world()->mutable_file_format()->mutable_version(); + meta.mutable_world()->set_file(trimmed(elem->GetText())); + } verMsg->set_major(ver.Major()); verMsg->set_minor(ver.Minor()); verMsg->set_patch(ver.Patch()); verMsg->set_prerelease(ver.Prerelease()); verMsg->set_build(ver.Build()); - - meta.mutable_model()->set_file(trimmed(elem->GetText())); } } elem = elem->NextSiblingElement("sdf"); } - if (meta.model().file().empty()) + if (meta.model().file().empty() && meta.world().file().empty()) { std::cerr << "Model config string does not contain an element\n"; return false; diff --git a/src/Utility_TEST.cc b/src/Utility_TEST.cc index 78b00db3..9dd7a29f 100644 --- a/src/Utility_TEST.cc +++ b/src/Utility_TEST.cc @@ -321,30 +321,31 @@ TEST(UtilityTest, ConvertFloat) TEST(UtilityTest, ConvertFuelMetadata) { msgs::FuelMetadata metaMsg; - std::string modelConfigStr; + std::string modelConfigInput, worldConfigInput; // test ConvertFuelMetadata(string, msgs::FuelMetadata) { - EXPECT_FALSE(msgs::ConvertFuelMetadata(modelConfigStr, metaMsg)); + EXPECT_FALSE(msgs::ConvertFuelMetadata(modelConfigInput, metaMsg)); metaMsg.Clear(); - modelConfigStr = ""; - EXPECT_FALSE(msgs::ConvertFuelMetadata(modelConfigStr, metaMsg)); + modelConfigInput = ""; + EXPECT_FALSE(msgs::ConvertFuelMetadata(modelConfigInput, metaMsg)); metaMsg.Clear(); - modelConfigStr = "test"; - EXPECT_FALSE(msgs::ConvertFuelMetadata(modelConfigStr, metaMsg)); + modelConfigInput = "test"; + EXPECT_FALSE(msgs::ConvertFuelMetadata(modelConfigInput, metaMsg)); + // Test metaMsg.Clear(); - modelConfigStr = R"( + modelConfigInput = R"( test_model )"; - EXPECT_FALSE(msgs::ConvertFuelMetadata(modelConfigStr, metaMsg)); + EXPECT_FALSE(msgs::ConvertFuelMetadata(modelConfigInput, metaMsg)); metaMsg.Clear(); - modelConfigStr = R"( + modelConfigInput = R"( model.sdf test_model @@ -362,7 +363,7 @@ TEST(UtilityTest, ConvertFuelMetadata) )"; - EXPECT_TRUE(msgs::ConvertFuelMetadata(modelConfigStr, metaMsg)); + EXPECT_TRUE(msgs::ConvertFuelMetadata(modelConfigInput, metaMsg)); EXPECT_EQ("test_model", metaMsg.name()); EXPECT_EQ(3, metaMsg.version()); EXPECT_EQ("A model for testing", metaMsg.description()); @@ -375,16 +376,37 @@ TEST(UtilityTest, ConvertFuelMetadata) EXPECT_EQ("foo@bar.org", metaMsg.authors(0).email()); EXPECT_EQ(1, metaMsg.dependencies().size()); EXPECT_EQ("model://some_model", metaMsg.dependencies(0).uri()); + + // Test + metaMsg.Clear(); + worldConfigInput = R"( + + world.sdf + test_world + 2 + A world for testing + +)"; + EXPECT_TRUE(msgs::ConvertFuelMetadata(worldConfigInput, metaMsg)); + EXPECT_EQ("test_world", metaMsg.name()); + EXPECT_EQ(2, metaMsg.version()); + EXPECT_EQ("A world for testing", metaMsg.description()); + EXPECT_EQ("world.sdf", metaMsg.world().file()); + EXPECT_EQ("sdf", metaMsg.world().file_format().name()); + EXPECT_EQ(1, metaMsg.world().file_format().version().major()); + EXPECT_EQ(7, metaMsg.world().file_format().version().minor()); + EXPECT_EQ(0, metaMsg.authors().size()); + EXPECT_EQ(0, metaMsg.dependencies().size()); } // test ConvertFuelMetadata(msgs::FuelMetadata, string) { - std::string modelConfigOutput; + std::string modelConfig; // Test metaMsg.Clear(); metaMsg.mutable_world()->set_file("world.sdf"); - EXPECT_FALSE(msgs::ConvertFuelMetadata(metaMsg, modelConfigOutput)); + EXPECT_FALSE(msgs::ConvertFuelMetadata(metaMsg, modelConfig)); metaMsg.set_name("test_world"); metaMsg.set_description("A world for testing"); @@ -394,22 +416,14 @@ TEST(UtilityTest, ConvertFuelMetadata) ->mutable_version()->set_major(1); metaMsg.mutable_world()->mutable_file_format() ->mutable_version()->set_minor(7); - EXPECT_TRUE(msgs::ConvertFuelMetadata(metaMsg, modelConfigOutput)); + EXPECT_TRUE(msgs::ConvertFuelMetadata(metaMsg, modelConfig)); - std::string expectedOutput = R"( - - world.sdf - test_world - 2 - A world for testing - -)"; - EXPECT_EQ(expectedOutput, modelConfigOutput); + EXPECT_EQ(worldConfigInput, modelConfig); // Test metaMsg.Clear(); metaMsg.mutable_model()->set_file("model.sdf"); - EXPECT_FALSE(msgs::ConvertFuelMetadata(metaMsg, modelConfigOutput)); + EXPECT_FALSE(msgs::ConvertFuelMetadata(metaMsg, modelConfig)); metaMsg.set_name("test_model"); metaMsg.set_description("A model for testing"); @@ -419,13 +433,13 @@ TEST(UtilityTest, ConvertFuelMetadata) ->mutable_version()->set_major(1); metaMsg.mutable_model()->mutable_file_format() ->mutable_version()->set_minor(7); - EXPECT_TRUE(msgs::ConvertFuelMetadata(metaMsg, modelConfigOutput)); + EXPECT_TRUE(msgs::ConvertFuelMetadata(metaMsg, modelConfig)); metaMsg.add_authors()->set_name("Foo Bar"); metaMsg.mutable_authors(0)->set_email("foo@bar.org"); metaMsg.add_dependencies()->set_uri("model://some_model"); - EXPECT_TRUE(msgs::ConvertFuelMetadata(metaMsg, modelConfigOutput)); - EXPECT_EQ(modelConfigStr, modelConfigOutput); + EXPECT_TRUE(msgs::ConvertFuelMetadata(metaMsg, modelConfig)); + EXPECT_EQ(modelConfigInput, modelConfig); } } From c86f6d101a65e5dd7ac18664fe5da3f78bd71a1a Mon Sep 17 00:00:00 2001 From: Jenn Nguyen Date: Fri, 22 Jul 2022 15:27:24 -0700 Subject: [PATCH 4/4] renamed variable Signed-off-by: Jenn Nguyen --- src/Utility.cc | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Utility.cc b/src/Utility.cc index 8195cf23..ec454b2b 100644 --- a/src/Utility.cc +++ b/src/Utility.cc @@ -964,14 +964,14 @@ namespace ignition return false; } - // Get the top level element. - tinyxml2::XMLElement *modelElement = modelConfigDoc.FirstChildElement( + // Get the top level or element. + tinyxml2::XMLElement *topElement = modelConfigDoc.FirstChildElement( "model"); bool isModel = true; - if (!modelElement) + if (!topElement) { - modelElement = modelConfigDoc.FirstChildElement("world"); - if (!modelElement) + topElement = modelConfigDoc.FirstChildElement("world"); + if (!topElement) { std::cerr << "Model config string does not contain a " << " or element\n"; @@ -981,7 +981,7 @@ namespace ignition } // Read the name, which is a mandatory element. - tinyxml2::XMLElement *elem = modelElement->FirstChildElement("name"); + tinyxml2::XMLElement *elem = topElement->FirstChildElement("name"); if (!elem || !elem->GetText()) { std::cerr << "Model config string does not contain a element\n"; @@ -990,7 +990,7 @@ namespace ignition meta.set_name(trimmed(elem->GetText())); // Read the version, if present. - elem = modelElement->FirstChildElement("version"); + elem = topElement->FirstChildElement("version"); if (elem && elem->GetText()) { auto version = std::stoi(trimmed(elem->GetText())); @@ -998,12 +998,12 @@ namespace ignition } // Read the description, if present. - elem = modelElement->FirstChildElement("description"); + elem = topElement->FirstChildElement("description"); if (elem && elem->GetText()) meta.set_description(trimmed(elem->GetText())); // Read the dependencies, if any. - elem = modelElement->FirstChildElement("depend"); + elem = topElement->FirstChildElement("depend"); while (elem) { auto modelElem = elem->FirstChildElement("model"); @@ -1020,7 +1020,7 @@ namespace ignition } // Read the authors, if any. - elem = modelElement->FirstChildElement("author"); + elem = topElement->FirstChildElement("author"); while (elem) { ignition::msgs::FuelMetadata::Contact *author = meta.add_authors(); @@ -1041,7 +1041,7 @@ namespace ignition } // Get the most recent SDF file - elem = modelElement->FirstChildElement("sdf"); + elem = topElement->FirstChildElement("sdf"); math::SemanticVersion maxVer; while (elem) {