BIND 10 trac2981, updated. a6cd22451be6684f4bebbc34d5344371afdeaa4b [2981] Final modifications before review
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Jul 25 15:11:52 UTC 2013
The branch, trac2981 has been updated
via a6cd22451be6684f4bebbc34d5344371afdeaa4b (commit)
from aac023f8e10f6922400400d614d0a0b141e79c53 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit a6cd22451be6684f4bebbc34d5344371afdeaa4b
Author: Stephen Morris <stephen at isc.org>
Date: Thu Jul 25 16:11:34 2013 +0100
[2981] Final modifications before review
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/tests/config_parser_unittest.cc | 23 ++++++++++++-----------
src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc | 4 +++-
src/bin/dhcp4/tests/marker_file.cc | 15 ++-------------
src/bin/dhcp4/tests/marker_file.h.in | 4 +---
src/bin/dhcp6/config_parser.cc | 2 +-
src/bin/dhcp6/tests/config_parser_unittest.cc | 17 +++++++++--------
src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc | 4 +++-
src/bin/dhcp6/tests/marker_file.cc | 15 ++-------------
src/bin/dhcp6/tests/marker_file.h.in | 4 +---
9 files changed, 34 insertions(+), 54 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index 9b56501..e7eb3ab 100644
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -1870,12 +1870,13 @@ TEST_F(Dhcp4ParserTest, NoHooksLibraries) {
string config = buildHooksLibrariesConfig();
if (!executeConfiguration(config,
"set configuration with no hooks libraries")) {
- return;
- }
+ FAIL() << "Unable to execute configuration";
- // No libraries should be loaded at the end of the test.
- libraries = HooksManager::getLibraryNames();
- ASSERT_TRUE(libraries.empty());
+ } else {
+ // No libraries should be loaded at the end of the test.
+ libraries = HooksManager::getLibraryNames();
+ EXPECT_TRUE(libraries.empty());
+ }
}
// Verify parsing fails with one library that will fail validation.
@@ -1906,8 +1907,8 @@ TEST_F(Dhcp4ParserTest, LibrariesSpecified) {
ASSERT_TRUE(libraries.empty());
// Marker files should not be present.
- EXPECT_TRUE(checkMarkerFile(LOAD_MARKER_FILE, NULL));
- EXPECT_TRUE(checkMarkerFile(UNLOAD_MARKER_FILE, NULL));
+ EXPECT_FALSE(checkMarkerFileExists(LOAD_MARKER_FILE));
+ EXPECT_FALSE(checkMarkerFileExists(UNLOAD_MARKER_FILE));
// Set up the configuration with two libraries and load them.
string config = buildHooksLibrariesConfig(CALLOUT_LIBRARY_1,
@@ -1917,10 +1918,10 @@ TEST_F(Dhcp4ParserTest, LibrariesSpecified) {
// Expect two libraries to be loaded in the correct order (load marker file
// is present, no unload marker file).
- libraries = HooksManager::getLibraryNames();
- ASSERT_EQ(2, libraries.size());
- EXPECT_TRUE(checkMarkerFile(LOAD_MARKER_FILE, "12"));
- EXPECT_TRUE(checkMarkerFile(UNLOAD_MARKER_FILE, NULL));
+ libraries = HooksManager::getLibraryNames();
+ ASSERT_EQ(2, libraries.size());
+ EXPECT_TRUE(checkMarkerFile(LOAD_MARKER_FILE, "12"));
+ EXPECT_FALSE(checkMarkerFileExists(UNLOAD_MARKER_FILE));
// Unload the libraries. The load file should not have changed, but
// the unload one should indicate the unload() functions have been run.
diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc
index 201322e..e6b500b 100644
--- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc
@@ -123,7 +123,9 @@ TEST_F(CtrlDhcpv4SrvTest, libreload) {
ASSERT_TRUE(libraries == loaded_libraries);
// ... which also included checking that the marker file created by the
- // load functions exists.
+ // load functions exists and holds the correct value (of "12" - the
+ // first library appends "1" to the file, the second appends "2"). Also
+ // check that the unload marker file does not yet exist.
EXPECT_TRUE(checkMarkerFile(LOAD_MARKER_FILE, "12"));
EXPECT_FALSE(checkMarkerFileExists(UNLOAD_MARKER_FILE));
diff --git a/src/bin/dhcp4/tests/marker_file.cc b/src/bin/dhcp4/tests/marker_file.cc
index d05ff60..d1c4aba 100644
--- a/src/bin/dhcp4/tests/marker_file.cc
+++ b/src/bin/dhcp4/tests/marker_file.cc
@@ -34,22 +34,10 @@ checkMarkerFile(const char* name, const char* expected) {
// Is it open?
if (!file.is_open()) {
-
- // No. This is OK if we don't expected is to be present but is
- // a failure otherwise.
- if (expected == NULL) {
- return (true);
- }
ADD_FAILURE() << "Unable to open " << name << ". It was expected "
<< "to be present and to contain the string '"
<< expected << "'";
return (false);
- } else if (expected == NULL) {
-
- // File is open but we don't expect it to be present.
- ADD_FAILURE() << "Opened " << name << " but it is not expected to "
- << "be present.";
- return (false);
}
// OK, is open, so read the data and see what we have. Compare it
@@ -58,7 +46,8 @@ checkMarkerFile(const char* name, const char* expected) {
getline(file, content);
string expected_str(expected);
- EXPECT_EQ(expected_str, content) << "Data was read from " << name;
+ EXPECT_EQ(expected_str, content) << "Marker file " << name
+ << "did not contain the expected data";
file.close();
return (expected_str == content);
diff --git a/src/bin/dhcp4/tests/marker_file.h.in b/src/bin/dhcp4/tests/marker_file.h.in
index 45ccbdd..52fc006 100644
--- a/src/bin/dhcp4/tests/marker_file.h.in
+++ b/src/bin/dhcp4/tests/marker_file.h.in
@@ -42,9 +42,7 @@ namespace test {
///
/// @param name Name of the marker file.
/// @param expected Characters expected. If a marker file is present,
-/// it is expected to contain characters. Therefore a value of NULL
-/// is used to signify that the marker file is not expected to be
-/// present.
+/// it is expected to contain characters.
///
/// @return true if all tests pass, false if not (in which case a failure
/// will have been logged).
diff --git a/src/bin/dhcp6/config_parser.cc b/src/bin/dhcp6/config_parser.cc
index 5117d52..41c7d31 100644
--- a/src/bin/dhcp6/config_parser.cc
+++ b/src/bin/dhcp6/config_parser.cc
@@ -596,7 +596,7 @@ configureDhcp6Server(Dhcpv6Srv&, isc::data::ConstElementPtr config_set) {
}
// Now commit any changes that have been validated but not yet committed,
- // but which can't be rolled back.
+ // and which can't be rolled back.
if (hooks_parser) {
hooks_parser->commit();
}
diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc
index 958aa6c..a7a60c6 100644
--- a/src/bin/dhcp6/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp6/tests/config_parser_unittest.cc
@@ -1985,12 +1985,13 @@ TEST_F(Dhcp6ParserTest, NoHooksLibraries) {
string config = buildHooksLibrariesConfig();
if (!executeConfiguration(config,
"set configuration with no hooks libraries")) {
- return;
- }
+ FAIL() << "Unable to execute configuration";
- // No libraries should be loaded at the end of the test.
- libraries = HooksManager::getLibraryNames();
- ASSERT_TRUE(libraries.empty());
+ } else {
+ // No libraries should be loaded at the end of the test.
+ libraries = HooksManager::getLibraryNames();
+ EXPECT_TRUE(libraries.empty());
+ }
}
// Verify parsing fails with one library that will fail validation.
@@ -2021,8 +2022,8 @@ TEST_F(Dhcp6ParserTest, LibrariesSpecified) {
ASSERT_TRUE(libraries.empty());
// Marker files should not be present.
- EXPECT_TRUE(checkMarkerFile(LOAD_MARKER_FILE, NULL));
- EXPECT_TRUE(checkMarkerFile(UNLOAD_MARKER_FILE, NULL));
+ EXPECT_FALSE(checkMarkerFileExists(LOAD_MARKER_FILE));
+ EXPECT_FALSE(checkMarkerFileExists(UNLOAD_MARKER_FILE));
// Set up the configuration with two libraries and load them.
string config = buildHooksLibrariesConfig(CALLOUT_LIBRARY_1,
@@ -2035,7 +2036,7 @@ TEST_F(Dhcp6ParserTest, LibrariesSpecified) {
libraries = HooksManager::getLibraryNames();
ASSERT_EQ(2, libraries.size());
EXPECT_TRUE(checkMarkerFile(LOAD_MARKER_FILE, "12"));
- EXPECT_TRUE(checkMarkerFile(UNLOAD_MARKER_FILE, NULL));
+ EXPECT_FALSE(checkMarkerFileExists(UNLOAD_MARKER_FILE));
// Unload the libraries. The load file should not have changed, but
// the unload one should indicate the unload() functions have been run.
diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
index 5111e54..6dfb981 100644
--- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
@@ -123,7 +123,9 @@ TEST_F(CtrlDhcpv6SrvTest, libreload) {
ASSERT_TRUE(libraries == loaded_libraries);
// ... which also included checking that the marker file created by the
- // load functions exists.
+ // load functions exists and holds the correct value (of "12" - the
+ // first library appends "1" to the file, the second appends "2"). Also
+ // check that the unload marker file does not yet exist.
EXPECT_TRUE(checkMarkerFile(LOAD_MARKER_FILE, "12"));
EXPECT_FALSE(checkMarkerFileExists(UNLOAD_MARKER_FILE));
diff --git a/src/bin/dhcp6/tests/marker_file.cc b/src/bin/dhcp6/tests/marker_file.cc
index d05ff60..d1c4aba 100644
--- a/src/bin/dhcp6/tests/marker_file.cc
+++ b/src/bin/dhcp6/tests/marker_file.cc
@@ -34,22 +34,10 @@ checkMarkerFile(const char* name, const char* expected) {
// Is it open?
if (!file.is_open()) {
-
- // No. This is OK if we don't expected is to be present but is
- // a failure otherwise.
- if (expected == NULL) {
- return (true);
- }
ADD_FAILURE() << "Unable to open " << name << ". It was expected "
<< "to be present and to contain the string '"
<< expected << "'";
return (false);
- } else if (expected == NULL) {
-
- // File is open but we don't expect it to be present.
- ADD_FAILURE() << "Opened " << name << " but it is not expected to "
- << "be present.";
- return (false);
}
// OK, is open, so read the data and see what we have. Compare it
@@ -58,7 +46,8 @@ checkMarkerFile(const char* name, const char* expected) {
getline(file, content);
string expected_str(expected);
- EXPECT_EQ(expected_str, content) << "Data was read from " << name;
+ EXPECT_EQ(expected_str, content) << "Marker file " << name
+ << "did not contain the expected data";
file.close();
return (expected_str == content);
diff --git a/src/bin/dhcp6/tests/marker_file.h.in b/src/bin/dhcp6/tests/marker_file.h.in
index 45ccbdd..52fc006 100644
--- a/src/bin/dhcp6/tests/marker_file.h.in
+++ b/src/bin/dhcp6/tests/marker_file.h.in
@@ -42,9 +42,7 @@ namespace test {
///
/// @param name Name of the marker file.
/// @param expected Characters expected. If a marker file is present,
-/// it is expected to contain characters. Therefore a value of NULL
-/// is used to signify that the marker file is not expected to be
-/// present.
+/// it is expected to contain characters.
///
/// @return true if all tests pass, false if not (in which case a failure
/// will have been logged).
More information about the bind10-changes
mailing list