From: Thomas Markwalder Date: Mon, 18 Mar 2019 18:31:42 +0000 (-0400) Subject: [#401,!254] Further review work. X-Git-Tag: Kea-1.6.0-beta~362^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3c8a5b9be22d7a552a9b22dfb1ce96dd86b937eb;p=thirdparty%2Fkea.git [#401,!254] Further review work. Removed use of Option::toText from tests: src/lib/dhcpsrv/tests/cfg_option_unittest.cc TEST_F(CfgOptionTest, replace) TEST_F(CfgOptionTest, mergeTo) TEST_F(CfgOptionTest, createDescriptorOptionValid) src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc TEST(CfgSubnets4Test, mergeSubnets) --- diff --git a/src/lib/dhcpsrv/tests/cfg_option_unittest.cc b/src/lib/dhcpsrv/tests/cfg_option_unittest.cc index b820305bc7..21c6a0b995 100644 --- a/src/lib/dhcpsrv/tests/cfg_option_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_option_unittest.cc @@ -23,6 +23,7 @@ #include using namespace isc; +using namespace isc::asiolink; using namespace isc::dhcp; namespace { @@ -233,15 +234,21 @@ TEST_F(CfgOptionTest, replace) { // Now let's make sure we can find them and they are as expected. OptionDescriptor desc = cfg.get("isc", 1); ASSERT_TRUE(desc.option_); - ASSERT_EQ(desc.option_->toText(),"type=00001, len=00003: \"one\" (string)"); + OptionStringPtr opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("one", opstr->getValue()); desc = cfg.get("isc", 2); ASSERT_TRUE(desc.option_); - ASSERT_EQ(desc.option_->toText(),"type=00002, len=00003: \"two\" (string)"); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("two", opstr->getValue()); desc = cfg.get("isc", 3); ASSERT_TRUE(desc.option_); - ASSERT_EQ(desc.option_->toText(),"type=00003, len=00005: \"three\" (string)"); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("three", opstr->getValue()); // Now let's replace one and three. desc.option_.reset(new OptionString(Option::V6, 1, "new one")); @@ -253,18 +260,23 @@ TEST_F(CfgOptionTest, replace) { // Now let's make sure we can find them again and they are as expected. desc = cfg.get("isc", 1); ASSERT_TRUE(desc.option_); - ASSERT_EQ(desc.option_->toText(),"type=00001, len=00007: \"new one\" (string)"); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("new one", opstr->getValue()); desc = cfg.get("isc", 2); ASSERT_TRUE(desc.option_); - ASSERT_EQ(desc.option_->toText(),"type=00002, len=00003: \"two\" (string)"); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("two", opstr->getValue()); desc = cfg.get("isc", 3); ASSERT_TRUE(desc.option_); - ASSERT_EQ(desc.option_->toText(),"type=00003, len=00009: \"new three\" (string)"); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("new three", opstr->getValue()); } - // This test verifies that one configuration can be merged into another. TEST_F(CfgOptionTest, mergeTo) { CfgOption cfg_src; @@ -399,33 +411,27 @@ TEST_F(CfgOptionTest, validMerge) { // Create our existing config, that gets merged into. OptionPtr option(new Option(Option::V4, 1, OptionBuffer(1, 0x01))); - EXPECT_EQ("type=001, len=001: 01", option->toText()); ASSERT_NO_THROW(this_cfg.add(option, false, "isc")); // Add option 3 to "fluff" option.reset(new Option(Option::V4, 3, OptionBuffer(1, 0x03))); - EXPECT_EQ("type=003, len=001: 03", option->toText()); ASSERT_NO_THROW(this_cfg.add(option, false, "fluff")); // Add option 4 to "fluff". option.reset(new Option(Option::V4, 4, OptionBuffer(1, 0x04))); - EXPECT_EQ("type=004, len=001: 04", option->toText()); ASSERT_NO_THROW(this_cfg.add(option, false, "fluff")); // Create our other config that will be merged from. // Add Option 1 to "isc", this should "overwrite" the original. option.reset(new Option(Option::V4, 1, OptionBuffer(1, 0x10))); - EXPECT_EQ("type=001, len=001: 10", option->toText()); ASSERT_NO_THROW(other_cfg.add(option, false, "isc")); // Add option 2 to "isc". option.reset(new Option(Option::V4, 2, OptionBuffer(1, 0x20))); - EXPECT_EQ("type=002, len=001: 20", option->toText()); ASSERT_NO_THROW(other_cfg.add(option, false, "isc")); // Add option 4 to "isc". option.reset(new Option(Option::V4, 4, OptionBuffer(1, 0x40))); - EXPECT_EQ("type=004, len=001: 40", option->toText()); ASSERT_NO_THROW(other_cfg.add(option, false, "isc")); // Merge source configuration to the destination configuration. The options @@ -436,27 +442,37 @@ TEST_F(CfgOptionTest, validMerge) { // isc:1 should come from "other" config. OptionDescriptor desc = this_cfg.get("isc", 1); ASSERT_TRUE(desc.option_); - EXPECT_EQ("type=001, len=001: 16 (uint8)", desc.option_->toText()); + OptionUint8Ptr opint = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opint); + EXPECT_EQ(16, opint->getValue()); // isc:2 should come from "other" config. desc = this_cfg.get("isc", 2); ASSERT_TRUE(desc.option_); - EXPECT_EQ("type=002, len=001: 32 (uint8)", desc.option_->toText()); + opint = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opint); + EXPECT_EQ(32, opint->getValue()); // isc:4 should come from "other" config. desc = this_cfg.get("isc", 4); ASSERT_TRUE(desc.option_); - EXPECT_EQ("type=004, len=001: 64 (uint8)", desc.option_->toText()); + opint = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opint); + EXPECT_EQ(64, opint->getValue()); // fluff:3 should come from "this" config. desc = this_cfg.get("fluff", 3); ASSERT_TRUE(desc.option_); - EXPECT_EQ("type=003, len=001: 3 (uint8)", desc.option_->toText()); + opint = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opint); + EXPECT_EQ(3, opint->getValue()); // fluff:4 should come from "this" config. desc = this_cfg.get("fluff", 4); ASSERT_TRUE(desc.option_); - EXPECT_EQ("type=004, len=001: 4 (uint8)", desc.option_->toText()); + opint = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opint); + EXPECT_EQ(4, opint->getValue()); } // This test verifies that attempting to merge options @@ -536,7 +552,7 @@ TEST_F(CfgOptionTest, createDescriptorOptionValid) { ASSERT_TRUE(updated); OptionStringPtr opstr = boost::dynamic_pointer_cast(desc->option_); ASSERT_TRUE(opstr); - EXPECT_EQ("type=012, len=011: \"example.org\" (string)", opstr->toText()); + EXPECT_EQ("example.org", opstr->getValue()); // Next we'll try a vendor option with a formatted value space = "vendor-4491"; @@ -548,7 +564,8 @@ TEST_F(CfgOptionTest, createDescriptorOptionValid) { ASSERT_TRUE(updated); Option4AddrLstPtr opaddrs = boost::dynamic_pointer_cast(desc->option_); ASSERT_TRUE(opaddrs); - EXPECT_EQ("type=002, len=008: 192.0.2.1 192.0.2.2", opaddrs->toText()); + Option4AddrLst::AddressContainer exp_addresses = { IOAddress("192.0.2.1"), IOAddress("192.0.2.2") }; + EXPECT_EQ(exp_addresses, opaddrs->getAddresses()); // Now, a user defined uint8 option space = "isc"; @@ -559,7 +576,7 @@ TEST_F(CfgOptionTest, createDescriptorOptionValid) { ASSERT_TRUE(updated); OptionUint8Ptr opint = boost::dynamic_pointer_cast(desc->option_); ASSERT_TRUE(opint); - EXPECT_EQ("type=001, len=001: 119 (uint8)", opint->toText()); + EXPECT_EQ(119, opint->getValue()); // Now, a user defined array of ints from a formatted value option.reset(new Option(Option::V4, 2)); @@ -569,7 +586,8 @@ TEST_F(CfgOptionTest, createDescriptorOptionValid) { ASSERT_TRUE(updated); OptionUint8ArrayPtr oparray = boost::dynamic_pointer_cast(desc->option_); ASSERT_TRUE(oparray); - EXPECT_EQ("type=002, len=003: 1(uint8) 2(uint8) 3(uint8)", oparray->toText()); + std::vector exp_ints = { 1, 2, 3 }; + EXPECT_EQ(exp_ints, oparray->getValues()); // Finally, a generic, undefined option option.reset(new Option(Option::V4, 199, OptionBuffer(1, 0x77))); @@ -577,7 +595,8 @@ TEST_F(CfgOptionTest, createDescriptorOptionValid) { ASSERT_NO_THROW(updated = CfgOption::createDescriptorOption(defs, space, *desc)); ASSERT_FALSE(updated); - EXPECT_EQ("type=199, len=001: 77", desc->option_->toText()); + ASSERT_EQ(1, desc->option_->getData().size()); + EXPECT_EQ(0x77, desc->option_->getData()[0]); } // This test verifies that encapsulated options are added as sub-options diff --git a/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc b/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc index 3921dd4851..f883e0a9a0 100644 --- a/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -204,8 +205,6 @@ TEST(CfgSubnets4Test, mergeSubnets) { OptionPtr option(new Option(Option::V4, 1)); option->setData(value.begin(), value.end()); ASSERT_NO_THROW(subnet1b->getCfgOption()->add(option, false, "isc")); - // Verify that our option is a generic option. - EXPECT_EQ("type=001, len=004: 59:61:79:21", option->toText()); // subnet 3b updates subnet 3 and removes it from network 2 Subnet4Ptr subnet3b(new Subnet4(IOAddress("192.0.3.0"), @@ -216,8 +215,6 @@ TEST(CfgSubnets4Test, mergeSubnets) { option.reset(new Option(Option::V4, 1)); option->setData(value.begin(), value.end()); ASSERT_NO_THROW(subnet3b->getCfgOption()->add(option, false, "isc")); - // Verify that our option is a generic option. - EXPECT_EQ("type=001, len=005: 54:65:61:6d:21", option->toText()); // subnet 4b updates subnet 4 and moves it from network2 to network 1 Subnet4Ptr subnet4b(new Subnet4(IOAddress("192.0.4.0"), @@ -236,7 +233,6 @@ TEST(CfgSubnets4Test, mergeSubnets) { option.reset(new Option(Option::V4, 1)); option->setData(value.begin(), value.end()); ASSERT_NO_THROW(pool->getCfgOption()->add(option, false, "isc")); - EXPECT_EQ(option->toText(), "type=001, len=005: 50:4f:4f:4c:53"); subnet5->addPool(pool); // Add pool 2 @@ -245,7 +241,6 @@ TEST(CfgSubnets4Test, mergeSubnets) { option.reset(new Option(Option::V4, 1)); option->setData(value.begin(), value.end()); ASSERT_NO_THROW(pool->getCfgOption()->add(option, false, "isc")); - EXPECT_EQ(option->toText(), "type=001, len=005: 52:55:4c:45:21"); subnet5->addPool(pool); // Add subnets to the merge from config. @@ -266,7 +261,9 @@ TEST(CfgSubnets4Test, mergeSubnets) { auto subnet = cfg_to.getByPrefix("192.0.1.0/26"); auto desc = subnet->getCfgOption()->get("isc", 1); ASSERT_TRUE(desc.option_); - EXPECT_EQ(desc.option_->toText(), "type=001, len=004: \"Yay!\" (string)"); + OptionStringPtr opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("Yay!", opstr->getValue()); // The subnet2 should not be affected because it was not present. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, SubnetID(2), @@ -279,7 +276,9 @@ TEST(CfgSubnets4Test, mergeSubnets) { subnet = cfg_to.getByPrefix("192.0.3.0/26"); desc = subnet->getCfgOption()->get("isc", 1); ASSERT_TRUE(desc.option_); - EXPECT_EQ(desc.option_->toText(), "type=001, len=005: \"Team!\" (string)"); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("Team!", opstr->getValue()); // subnet4 should be replaced by subnet4b and moved to network1. ASSERT_NO_FATAL_FAILURE(checkMergedSubnet(cfg_to, SubnetID(4), @@ -294,14 +293,16 @@ TEST(CfgSubnets4Test, mergeSubnets) { const PoolPtr merged_pool = subnet->getPool(Lease::TYPE_V4, IOAddress("192.0.5.10")); ASSERT_TRUE(merged_pool); desc = merged_pool->getCfgOption()->get("isc", 1); - ASSERT_TRUE(desc.option_); - EXPECT_EQ(desc.option_->toText(), "type=001, len=005: \"POOLS\" (string)"); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("POOLS", opstr->getValue()); const PoolPtr merged_pool2 = subnet->getPool(Lease::TYPE_V4, IOAddress("192.0.5.30")); ASSERT_TRUE(merged_pool2); desc = merged_pool2->getCfgOption()->get("isc", 1); - ASSERT_TRUE(desc.option_); - EXPECT_EQ(desc.option_->toText(), "type=001, len=005: \"RULE!\" (string)"); + opstr = boost::dynamic_pointer_cast(desc.option_); + ASSERT_TRUE(opstr); + EXPECT_EQ("RULE!", opstr->getValue()); } // This test verifies that it is possible to retrieve a subnet using an