From c49d5606203d6d0ddc3345a553edf33a4a812bcf Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Mon, 28 Apr 2014 15:25:07 +0200 Subject: [PATCH] [3409] Unit tests check that the error message contains line number etc. --- configure.ac | 1 + src/bin/dhcp4/tests/Makefile.am | 1 + src/bin/dhcp4/tests/config_parser_unittest.cc | 79 ++++----- src/bin/dhcp6/tests/Makefile.am | 1 + src/bin/dhcp6/tests/config_parser_unittest.cc | 152 ++++++------------ src/lib/dhcpsrv/Makefile.am | 2 +- src/lib/dhcpsrv/tests/Makefile.am | 1 + .../dhcpsrv/tests/dhcp_parsers_unittest.cc | 7 + src/lib/dhcpsrv/testutils/Makefile.am | 24 +++ .../dhcpsrv/testutils/config_result_check.cc | 94 +++++++++++ .../dhcpsrv/testutils/config_result_check.h | 56 +++++++ 11 files changed, 272 insertions(+), 146 deletions(-) create mode 100644 src/lib/dhcpsrv/testutils/Makefile.am create mode 100644 src/lib/dhcpsrv/testutils/config_result_check.cc create mode 100644 src/lib/dhcpsrv/testutils/config_result_check.h diff --git a/configure.ac b/configure.ac index 8ebc1f7a2d..49247ca43f 100644 --- a/configure.ac +++ b/configure.ac @@ -1466,6 +1466,7 @@ AC_CONFIG_FILES([compatcheck/Makefile src/lib/dhcp_ddns/tests/Makefile src/lib/dhcp/Makefile src/lib/dhcpsrv/Makefile + src/lib/dhcpsrv/testutils/Makefile src/lib/dhcpsrv/tests/Makefile src/lib/dhcpsrv/tests/test_libraries.h src/lib/dhcp/tests/Makefile diff --git a/src/bin/dhcp4/tests/Makefile.am b/src/bin/dhcp4/tests/Makefile.am index 24931ec379..1961883706 100644 --- a/src/bin/dhcp4/tests/Makefile.am +++ b/src/bin/dhcp4/tests/Makefile.am @@ -106,6 +106,7 @@ dhcp4_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libkea-exceptions.la dhcp4_unittests_LDADD += $(top_builddir)/src/lib/log/libkea-log.la dhcp4_unittests_LDADD += $(top_builddir)/src/lib/util/libkea-util.la dhcp4_unittests_LDADD += $(top_builddir)/src/lib/hooks/libkea-hooks.la +dhcp4_unittests_LDADD += $(top_builddir)/src/lib/dhcpsrv/testutils/libdhcpsrvtest.la endif noinst_PROGRAMS = $(TESTS) diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index e01c1dde96..e7d7181197 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -27,6 +27,7 @@ #include #include #include +#include #include #include "marker_file.h" @@ -286,9 +287,8 @@ public: std::string config = createConfigWithOption(param_value, parameter); ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(1, rcode_); + checkResult(x, 1); + EXPECT_TRUE(errorContainsPosition(x, "")); } /// @brief Test invalid option paramater value. @@ -304,9 +304,8 @@ public: std::string config = createConfigWithOption(params); ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(1, rcode_); + checkResult(x, 1); + EXPECT_TRUE(errorContainsPosition(x, "")); } /// @brief Test option against given code and data. @@ -580,9 +579,7 @@ TEST_F(Dhcp4ParserTest, multipleSubnets) { do { EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); const Subnet4Collection* subnets = CfgMgr::instance().getSubnets4(); ASSERT_TRUE(subnets); @@ -635,9 +632,7 @@ TEST_F(Dhcp4ParserTest, multipleSubnetsExplicitIDs) { int cnt = 0; // Number of reconfigurations do { EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); const Subnet4Collection* subnets = CfgMgr::instance().getSubnets4(); ASSERT_TRUE(subnets); @@ -686,9 +681,8 @@ TEST_F(Dhcp4ParserTest, multipleSubnetsOverlapingIDs) { ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - EXPECT_NE(rcode_, 0); + checkResult(x, 2); + EXPECT_TRUE(errorContainsPosition(x, "")); } // Goal of this test is to verify that a previously configured subnet can be @@ -769,9 +763,7 @@ TEST_F(Dhcp4ParserTest, reconfigureRemoveSubnet) { ElementPtr json = Element::fromJSON(config4); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); const Subnet4Collection* subnets = CfgMgr::instance().getSubnets4(); ASSERT_TRUE(subnets); @@ -780,9 +772,7 @@ TEST_F(Dhcp4ParserTest, reconfigureRemoveSubnet) { // Do the reconfiguration (the last subnet is removed) json = Element::fromJSON(config_first3); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); subnets = CfgMgr::instance().getSubnets4(); ASSERT_TRUE(subnets); @@ -799,16 +789,12 @@ TEST_F(Dhcp4ParserTest, reconfigureRemoveSubnet) { /// @todo: Uncomment subnet removal test as part of #3281. json = Element::fromJSON(config4); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); // Do reconfiguration json = Element::fromJSON(config_second_removed); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); subnets = CfgMgr::instance().getSubnets4(); ASSERT_TRUE(subnets); @@ -932,12 +918,15 @@ TEST_F(Dhcp4ParserTest, nextServerNegative) { // check if returned status is always a failure EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json1)); checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json2)); checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json3)); checkResult(status, 0); + EXPECT_FALSE(errorContainsPosition(status, "")); } // Checks if the next-server defined as global value is overridden by subnet @@ -1065,6 +1054,7 @@ TEST_F(Dhcp4ParserTest, poolOutOfSubnet) { // returned value must be 1 (values error) // as the pool does not belong to that subnet checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } // Goal of this test is to verify if pools can be defined @@ -1284,6 +1274,7 @@ TEST_F(Dhcp4ParserTest, optionDefDuplicate) { EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); ASSERT_TRUE(status); checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } // The goal of this test is to verify that the option definition @@ -1392,6 +1383,7 @@ TEST_F(Dhcp4ParserTest, optionDefInvalidName) { ASSERT_TRUE(status); // Expecting parsing error (error code 1). checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } /// The purpose of this test is to verify that the option definition @@ -1418,6 +1410,7 @@ TEST_F(Dhcp4ParserTest, optionDefInvalidType) { ASSERT_TRUE(status); // Expecting parsing error (error code 1). checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } /// The purpose of this test is to verify that the option definition @@ -1444,6 +1437,7 @@ TEST_F(Dhcp4ParserTest, optionDefInvalidRecordType) { ASSERT_TRUE(status); // Expecting parsing error (error code 1). checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } /// The goal of this test is to verify that the invalid encapsulated @@ -1470,6 +1464,7 @@ TEST_F(Dhcp4ParserTest, optionDefInvalidEncapsulatedSpace) { ASSERT_TRUE(status); // Expecting parsing error (error code 1). checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } /// The goal of this test is to verify that the encapsulated @@ -1498,6 +1493,7 @@ TEST_F(Dhcp4ParserTest, optionDefEncapsulatedSpaceAndArray) { ASSERT_TRUE(status); // Expecting parsing error (error code 1). checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } /// The goal of this test is to verify that the option may not @@ -1524,6 +1520,7 @@ TEST_F(Dhcp4ParserTest, optionDefEncapsulateOwnSpace) { ASSERT_TRUE(status); // Expecting parsing error (error code 1). checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } /// The purpose of this test is to verify that it is not allowed @@ -1588,6 +1585,7 @@ TEST_F(Dhcp4ParserTest, optionStandardDefOverride) { ASSERT_TRUE(status); // Expecting parsing error (error code 1). checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); /// @todo The option 65 is a standard DHCPv4 option. However, at this point /// there is no definition for this option in libdhcp++, so it should be @@ -1655,9 +1653,7 @@ TEST_F(Dhcp4ParserTest, optionDataDefaults) { ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.200"), classify_); @@ -1960,9 +1956,7 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) { ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.24"), classify_); @@ -2113,9 +2107,7 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) { ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); Subnet4Ptr subnet1 = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.100"), classify_); @@ -2218,9 +2210,7 @@ TEST_F(Dhcp4ParserTest, optionDataLowerCase) { ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.5"), classify_); @@ -2263,9 +2253,7 @@ TEST_F(Dhcp4ParserTest, stdOptionData) { ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.5"), classify_); @@ -2343,6 +2331,7 @@ TEST_F(Dhcp4ParserTest, DISABLED_Uint32Parser) { // returned value must be rejected (1 configuration error) checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); // CASE 4: -1 (UINT_MIN -1 ) should not work EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, @@ -2351,6 +2340,7 @@ TEST_F(Dhcp4ParserTest, DISABLED_Uint32Parser) { // returned value must be rejected (1 configuration error) checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } // The goal of this test is to verify that the standard option can @@ -2935,6 +2925,7 @@ TEST_F(Dhcp4ParserTest, invalidD2ClientConfig) { // check if returned status is failed. checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); // Verify that the D2 configuraiton can be fetched and is set to disabled. D2ClientConfigPtr d2_client_config = CfgMgr::instance().getD2ClientConfig(); @@ -3008,9 +2999,7 @@ TEST_F(Dhcp4ParserTest, classifySubnets) { ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); const Subnet4Collection* subnets = CfgMgr::instance().getSubnets4(); ASSERT_TRUE(subnets); diff --git a/src/bin/dhcp6/tests/Makefile.am b/src/bin/dhcp6/tests/Makefile.am index 8084a17894..424bd5b29b 100644 --- a/src/bin/dhcp6/tests/Makefile.am +++ b/src/bin/dhcp6/tests/Makefile.am @@ -101,6 +101,7 @@ dhcp6_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la dhcp6_unittests_LDADD += $(top_builddir)/src/lib/dhcp/tests/libdhcptest.la dhcp6_unittests_LDADD += $(top_builddir)/src/lib/dhcp_ddns/libkea-dhcp_ddns.la dhcp6_unittests_LDADD += $(top_builddir)/src/lib/dhcpsrv/libkea-dhcpsrv.la +dhcp6_unittests_LDADD += $(top_builddir)/src/lib/dhcpsrv/testutils/libdhcpsrvtest.la dhcp6_unittests_LDADD += $(top_builddir)/src/lib/hooks/libkea-hooks.la dhcp6_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libkea-exceptions.la dhcp6_unittests_LDADD += $(top_builddir)/src/lib/log/libkea-log.la diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 42ba1128e5..6d3e8a0d70 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -25,6 +25,7 @@ #include #include #include +#include #include #include "test_data_files_config.h" @@ -375,9 +376,8 @@ public: std::string config = createConfigWithOption(param_value, parameter); ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(1, rcode_); + checkResult(x, 1); + EXPECT_TRUE(errorContainsPosition(x, "")); } /// @brief Test invalid option paramater value. @@ -393,9 +393,8 @@ public: std::string config = createConfigWithOption(params); ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(1, rcode_); + checkResult(x, 1); + EXPECT_TRUE(errorContainsPosition(x, "")); } /// @brief Test option against given code and data. @@ -491,9 +490,7 @@ TEST_F(Dhcp6ParserTest, version) { Element::fromJSON("{\"version\": 0}"))); // returned value must be 0 (configuration accepted) - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - EXPECT_EQ(0, rcode_); + checkResult(x, 0); } /// The goal of this test is to verify that the code accepts only @@ -506,9 +503,7 @@ TEST_F(Dhcp6ParserTest, bogusCommand) { Element::fromJSON("{\"bogus\": 5}"))); // returned value must be 1 (configuration parse error) - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - EXPECT_EQ(1, rcode_); + checkResult(x, 1); } /// The goal of this test is to verify if configuration without any @@ -526,9 +521,7 @@ TEST_F(Dhcp6ParserTest, emptySubnet) { "\"valid-lifetime\": 4000 }"))); // returned value should be 0 (success) - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(0, rcode_); + checkResult(status, 0); } /// The goal of this test is to verify if defined subnet uses global @@ -551,9 +544,7 @@ TEST_F(Dhcp6ParserTest, subnetGlobalDefaults) { EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); // check if returned status is OK - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(0, rcode_); + checkResult(status, 0); // Now check if the configuration was indeed handled and we have // expected pool configured. @@ -602,9 +593,7 @@ TEST_F(Dhcp6ParserTest, multipleSubnets) { do { EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6(); ASSERT_TRUE(subnets); @@ -659,9 +648,7 @@ TEST_F(Dhcp6ParserTest, multipleSubnetsExplicitIDs) { do { EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6(); ASSERT_TRUE(subnets); @@ -711,9 +698,8 @@ TEST_F(Dhcp6ParserTest, multipleSubnetsOverlapingIDs) { ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_NE(rcode_, 0); + checkResult(x, 2); + EXPECT_TRUE(errorContainsPosition(x, "")); } @@ -798,9 +784,7 @@ TEST_F(Dhcp6ParserTest, reconfigureRemoveSubnet) { ElementPtr json = Element::fromJSON(config4); EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6(); ASSERT_TRUE(subnets); @@ -809,9 +793,7 @@ TEST_F(Dhcp6ParserTest, reconfigureRemoveSubnet) { // Do the reconfiguration (the last subnet is removed) json = Element::fromJSON(config_first3); EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); subnets = CfgMgr::instance().getSubnets6(); ASSERT_TRUE(subnets); @@ -826,16 +808,12 @@ TEST_F(Dhcp6ParserTest, reconfigureRemoveSubnet) { json = Element::fromJSON(config4); EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); // Do reconfiguration json = Element::fromJSON(config_second_removed); EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); subnets = CfgMgr::instance().getSubnets6(); ASSERT_TRUE(subnets); @@ -873,9 +851,7 @@ TEST_F(Dhcp6ParserTest, subnetLocal) { EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); // returned value should be 0 (configuration success) - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(0, rcode_); + checkResult(status, 0); Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"), classify_); @@ -910,9 +886,7 @@ TEST_F(Dhcp6ParserTest, subnetInterface) { EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); // returned value should be 0 (configuration success) - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(0, rcode_); + checkResult(status, 0); Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"), classify_); @@ -944,9 +918,8 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceBogus) { EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); // returned value should be 1 (configuration error) - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(1, rcode_); + checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"), classify_); @@ -976,9 +949,8 @@ TEST_F(Dhcp6ParserTest, interfaceGlobal) { EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); // returned value should be 1 (parse error) - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(1, rcode_); + checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } @@ -1007,9 +979,7 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceId) { EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); // Returned value should be 0 (configuration success) - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(0, rcode_); + checkResult(status, 0); // Try to get a subnet based on bogus interface-id option OptionBuffer tmp(bogus_interface_id.begin(), bogus_interface_id.end()); @@ -1046,9 +1016,8 @@ TEST_F(Dhcp6ParserTest, interfaceIdGlobal) { EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); // Returned value should be 1 (parse error) - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(1, rcode_); + checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } // This test checks if it is not possible to define a subnet with an @@ -1071,10 +1040,8 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceAndInterfaceId) { EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); // Returned value should be 1 (configuration error) - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(1, rcode_); - + checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } @@ -1101,9 +1068,8 @@ TEST_F(Dhcp6ParserTest, poolOutOfSubnet) { // returned value must be 1 (values error) // as the pool does not belong to that subnet - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(1, rcode_); + checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } // Goal of this test is to verify if pools can be defined @@ -1129,9 +1095,7 @@ TEST_F(Dhcp6ParserTest, poolPrefixLen) { EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); // returned value must be 1 (configuration parse error) - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - EXPECT_EQ(0, rcode_); + checkResult(x, 0); Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"), classify_); @@ -1172,9 +1136,7 @@ TEST_F(Dhcp6ParserTest, pdPoolBasics) { // Returned value must be non-empty ConstElementPtr to config result. // rcode should be 0 which indicates successful configuration processing. EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - EXPECT_EQ(0, rcode_); + checkResult(x, 0); // Test that we can retrieve the subnet. Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"), @@ -1246,9 +1208,7 @@ TEST_F(Dhcp6ParserTest, pdPoolList) { // Returned value must be non-empty ConstElementPtr to config result. // rcode should be 0 which indicates successful configuration processing. EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - EXPECT_EQ(0, rcode_); + checkResult(x, 0); // Test that we can retrieve the subnet. Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"), @@ -1304,9 +1264,7 @@ TEST_F(Dhcp6ParserTest, subnetAndPrefixDelegated) { // Returned value must be non-empty ConstElementPtr to config result. // rcode should be 0 which indicates successful configuration processing. EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - EXPECT_EQ(0, rcode_); + checkResult(x, 0); // Test that we can retrieve the subnet. Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"), @@ -1423,9 +1381,8 @@ TEST_F(Dhcp6ParserTest, invalidPdPools) { // Returned value must be non-empty ConstElementPtr to config result. // rcode should be 1 which indicates configuration error. - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - EXPECT_EQ(1, rcode_); + checkResult(x, 1); + EXPECT_TRUE(errorContainsPosition(x, "")); } } @@ -1611,6 +1568,7 @@ TEST_F(Dhcp6ParserTest, optionDefDuplicate) { EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); ASSERT_TRUE(status); checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } // The goal of this test is to verify that the option definition @@ -1718,6 +1676,7 @@ TEST_F(Dhcp6ParserTest, optionDefInvalidName) { ASSERT_TRUE(status); // Expecting parsing error (error code 1). checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } /// The purpose of this test is to verify that the option definition @@ -1744,6 +1703,7 @@ TEST_F(Dhcp6ParserTest, optionDefInvalidType) { ASSERT_TRUE(status); // Expecting parsing error (error code 1). checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } /// The purpose of this test is to verify that the option definition @@ -1770,6 +1730,7 @@ TEST_F(Dhcp6ParserTest, optionDefInvalidRecordType) { ASSERT_TRUE(status); // Expecting parsing error (error code 1). checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } /// The goal of this test is to verify that the invalid encapsulated @@ -1796,6 +1757,7 @@ TEST_F(Dhcp6ParserTest, optionDefInvalidEncapsulatedSpace) { ASSERT_TRUE(status); // Expecting parsing error (error code 1). checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } /// The goal of this test is to verify that the encapsulated @@ -1824,6 +1786,7 @@ TEST_F(Dhcp6ParserTest, optionDefEncapsulatedSpaceAndArray) { ASSERT_TRUE(status); // Expecting parsing error (error code 1). checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } /// The goal of this test is to verify that the option may not @@ -1850,6 +1813,7 @@ TEST_F(Dhcp6ParserTest, optionDefEncapsulateOwnSpace) { ASSERT_TRUE(status); // Expecting parsing error (error code 1). checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); } /// The purpose of this test is to verify that it is not allowed @@ -1915,6 +1879,7 @@ TEST_F(Dhcp6ParserTest, optionStandardDefOverride) { ASSERT_TRUE(status); // Expecting parsing error (error code 1). checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); /// @todo The option 59 is a standard DHCPv6 option. However, at this point /// there is no definition for this option in libdhcp++, so it should be @@ -1982,9 +1947,7 @@ TEST_F(Dhcp6ParserTest, optionDataDefaults) { ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"), classify_); @@ -2292,9 +2255,7 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) { ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); Subnet6Ptr subnet1 = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"), classify_); @@ -2489,9 +2450,7 @@ TEST_F(Dhcp6ParserTest, optionDataLowerCase) { ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"), classify_); @@ -2534,9 +2493,7 @@ TEST_F(Dhcp6ParserTest, stdOptionData) { ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"), classify_); @@ -3025,9 +2982,7 @@ TEST_F(Dhcp6ParserTest, selectedInterfaces) { // returned value must be 1 (values error) // as the pool does not belong to that subnet - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(0, rcode_); + checkResult(status, 0); // eth0 and eth1 were explicitly selected. eth2 was not. EXPECT_TRUE(CfgMgr::instance().isActiveIface("eth0")); @@ -3063,9 +3018,7 @@ TEST_F(Dhcp6ParserTest, allInterfaces) { // returned value must be 1 (values error) // as the pool does not belong to that subnet - ASSERT_TRUE(status); - comment_ = parseAnswer(rcode_, status); - EXPECT_EQ(0, rcode_); + checkResult(status, 0); // All interfaces should be now active. EXPECT_TRUE(CfgMgr::instance().isActiveIface("eth0")); @@ -3137,9 +3090,7 @@ TEST_F(Dhcp6ParserTest, classifySubnets) { ElementPtr json = Element::fromJSON(config); EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json)); - ASSERT_TRUE(x); - comment_ = parseAnswer(rcode_, x); - ASSERT_EQ(0, rcode_); + checkResult(x, 0); const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6(); ASSERT_TRUE(subnets); @@ -3294,6 +3245,7 @@ TEST_F(Dhcp6ParserTest, invalidD2ClientConfig) { // check if returned status is failed. checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); // Verify that the D2 configuraiton can be fetched and is set to disabled. D2ClientConfigPtr d2_client_config = CfgMgr::instance().getD2ClientConfig(); diff --git a/src/lib/dhcpsrv/Makefile.am b/src/lib/dhcpsrv/Makefile.am index d19afb01bf..2f4be2ed63 100644 --- a/src/lib/dhcpsrv/Makefile.am +++ b/src/lib/dhcpsrv/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = . tests +SUBDIRS = . testutils tests dhcp_data_dir = @localstatedir@/@PACKAGE@ diff --git a/src/lib/dhcpsrv/tests/Makefile.am b/src/lib/dhcpsrv/tests/Makefile.am index 8e2d9126ad..6ef9a7dfac 100644 --- a/src/lib/dhcpsrv/tests/Makefile.am +++ b/src/lib/dhcpsrv/tests/Makefile.am @@ -107,6 +107,7 @@ libdhcpsrv_unittests_CXXFLAGS += -Wno-unused-variable -Wno-unused-parameter endif libdhcpsrv_unittests_LDADD = $(top_builddir)/src/lib/dhcpsrv/libkea-dhcpsrv.la +libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/dhcpsrv/testutils/libdhcpsrvtest.la libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/dhcp/tests/libdhcptest.la libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/dhcp_ddns/libkea-dhcp_ddns.la diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index eb234142a2..ff78e35466 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -37,6 +38,7 @@ using namespace isc; using namespace isc::config; using namespace isc::data; using namespace isc::dhcp; +using namespace isc::dhcp::test; using namespace isc::hooks; namespace { @@ -419,6 +421,11 @@ public: ConstElementPtr status = parseElementSet(json); ConstElementPtr comment = parseAnswer(rcode_, status); error_text_ = comment->stringValue(); + // If error was reported, the error string should contain + // position of the data element which caused failure. + if (rcode_ != 0) { + EXPECT_TRUE(errorContainsPosition(status, "")); + } } return (rcode_); diff --git a/src/lib/dhcpsrv/testutils/Makefile.am b/src/lib/dhcpsrv/testutils/Makefile.am new file mode 100644 index 0000000000..22249a82d7 --- /dev/null +++ b/src/lib/dhcpsrv/testutils/Makefile.am @@ -0,0 +1,24 @@ +SUBDIRS = . + +AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib +AM_CPPFLAGS += $(BOOST_INCLUDES) + +AM_CXXFLAGS = $(B10_CXXFLAGS) + +if USE_STATIC_LINK +AM_LDFLAGS = -static +endif + +CLEANFILES = *.gcno *.gcda + +if HAVE_GTEST + +noinst_LTLIBRARIES = libdhcpsrvtest.la + +libdhcpsrvtest_la_SOURCES = config_result_check.cc config_result_check.h +libdhcpsrvtest_la_CXXFLAGS = $(AM_CXXFLAGS) +libdhcpsrvtest_la_CPPFLAGS = $(AM_CPPFLAGS) +libdhcpsrvtest_la_LDFLAGS = $(AM_LDFLAGS) +libdhcpsrvtest_la_LIBADD = $(top_builddir)/src/lib/cc/libkea-cc.la + +endif diff --git a/src/lib/dhcpsrv/testutils/config_result_check.cc b/src/lib/dhcpsrv/testutils/config_result_check.cc new file mode 100644 index 0000000000..61d0c3748d --- /dev/null +++ b/src/lib/dhcpsrv/testutils/config_result_check.cc @@ -0,0 +1,94 @@ +// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#include +#include +#include +#include +#include +#include + +namespace isc { +namespace dhcp { +namespace test { + +using namespace isc; +using namespace isc::data; + +bool errorContainsPosition(ConstElementPtr error_element, + const std::string& file_name) { + if (error_element->contains(isc::cc::CC_PAYLOAD_RESULT)) { + ConstElementPtr result = error_element->get(isc::cc::CC_PAYLOAD_RESULT); + if ((result->getType() != Element::list) || (result->size() < 2) || + (result->get(1)->getType() != Element::string)) { + return (false); + } + + // Get the error message in the textual format. + std::string error_string = result->get(1)->stringValue(); + + // The position of the data element causing an error has the following + // format: ::. The has been specified + // by a caller, so let's first check if this file name is present in the + // error message. + size_t pos = error_string.find(file_name); + + // If the file name is present, check that it is followed by the line + // number and position within the line. + if (pos != std::string::npos) { + // Split the string starting at the begining of the . It + // should return a vector of strings. + std::string sub = error_string.substr(pos); + std::vector split_pos; + boost::split(split_pos, sub, boost::is_any_of(":"), + boost::algorithm::token_compress_off); + + // There should be at least three elements: , + // and . There can be even more, because one error string may + // contain more positions of data elements for multiple + // configuration nesting levels. We want at least one position. + if ((split_pos.size() >= 3) && (split_pos[0] == file_name) && + (!split_pos[1].empty()) && !(split_pos[2].empty())) { + + // Make sure that the line number comprises only digits. + for (int i = 0; i < split_pos[1].size(); ++i) { + if (!isdigit(split_pos[1][i])) { + return (false); + } + } + + // Go over digits of the position within the line. + int i = 0; + while (isdigit(split_pos[2][i])) { + ++i; + } + + // Make sure that there has been at least one digit and that the + // position is followed by the paren. + if ((i == 0) || (split_pos[2][i] != ')')) { + return (false); + } + + // All checks passed. + return (true); + } + } + } + + return (false); +} + +} +} +} diff --git a/src/lib/dhcpsrv/testutils/config_result_check.h b/src/lib/dhcpsrv/testutils/config_result_check.h new file mode 100644 index 0000000000..e7c519c515 --- /dev/null +++ b/src/lib/dhcpsrv/testutils/config_result_check.h @@ -0,0 +1,56 @@ +// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + +#ifndef CONFIG_RESULT_CHECK_H +#define CONFIG_RESULT_CHECK_H + +#include + +namespace isc { +namespace dhcp { +namespace test { + +/// @brief Checks that the error string created by the configuration parsers +/// contains the location of the data element... +/// +/// This function checks that the error string returned by the configuration +/// parsers contains the position of the element which caused an error. The +/// error string is expected to contain at least one occurence of the following: +/// +/// @code +/// [filename]:[linenum]:[pos] +/// @endcode +/// +/// where: +/// - [filename] is a configuration file name (provided by a caller), +/// - [linenum] is a line number of the element, +/// - [pos] is a position of the element within the line. +/// +/// Both [linenum] and [pos] must contain decimal digits. The [filename] +/// must match the name provided by the caller. +/// +/// @param error_element A result returned by the configuration. +/// @param file_name A configuration file name. +/// +/// @return true if the provided configuration result comprises a string +/// which holds a position of the data element which caused the error; +/// false otherwise. +bool errorContainsPosition(isc::data::ConstElementPtr error_element, + const std::string& file_name); + +} +} +} + +#endif