[1698-test] Additional changes as a result of review

src/lib/log/message_initializer.cc

@@ -20,24 +20,20 @@
 
 // As explained in the header file, initialization of the message dictionary
 // is a two-stage process:
-// 1) In the MessageInitializer constructor, the a pointer to the array of
+// 1) In the MessageInitializer constructor, a pointer to the array of
 //    messages is stored in a pre-defined array.  Since the MessageInitializers
-//    are declared external to a program unit, this takes place before main()
-//    is called.  As no heap storage is allocated in this process, we should
-//    avoid the static initialization fiasco in cases where initialization
-//    of system libraries is also carried out at the same time.
+//    are declared statically outside a program unit, this takes place before
+//    main() is called.  As no heap storage is allocated in this process, we
+//    should avoid the static initialization fiasco in cases where
+//    initialization of system libraries is also carried out at the same time.
 // 2) After main() starts executing, loadDictionary() is called.
 //
 //
 
 namespace {
 
-// Declare the array of pointers to value arrays.  At the time of writing
-// (end of BIND 10's third year of development), only 25 such arrays have
-// been defined, so a value of 64 should allow for significant expansion.
-// (Besides, an assertion will be triggered if the array overflows.)
-const int MAX_LOGGERS = 64;
-const char** logger_values[MAX_LOGGERS];
+// Declare the array of pointers to value arrays.
+const char** logger_values[isc::log::MessageInitializer::MAX_MESSAGE_ARRAYS];
 
 // Declare the index used to access the array.  As this needs to be initialized
 // at first used, it is accessed it via a function.
@@ -56,10 +52,17 @@ namespace log {
 // This method will trigger an assertion failure if the array overflows.
 
 MessageInitializer::MessageInitializer(const char* values[]) {
-    assert(getIndex() < MAX_LOGGERS);
+    assert(getIndex() < MAX_MESSAGE_ARRAYS);
     logger_values[getIndex()++] = values;
 }
 
+// Return the number of arrays registered but not yet loaded.
+
+size_t
+MessageInitializer::getPendingCount() {
+    return (getIndex());
+}
+
 // Load the messages in the arrays registered in the logger_values array
 // into the global dictionary.
 

src/lib/log/message_initializer.h

@@ -15,6 +15,7 @@
 #ifndef __MESSAGEINITIALIZER_H
 #define __MESSAGEINITIALIZER_H
 
+#include <cstdlib>
 #include <string>
 #include <vector>
 #include <log/message_dictionary.h>
@@ -44,13 +45,19 @@ namespace log {
 ///
 /// To avoid static initialization fiasco problems, the initialization is
 /// carried out in two stages:
-/// -# The constructor a pointer to the values array to a pre-defined array
-///    of pointers.
-/// -# During the run-time initialization of the logging system, the static
-///    method loadDictionary() is called to load the message dictionary.
+/// - The constructor adds a pointer to the values array to a pre-defined array
+///   of pointers.
+/// - During the run-time initialization of the logging system, the static
+///   method loadDictionary() is called to load the message dictionary.
 /// This way, no heap storage is allocated during the static initialization,
 /// something that may give problems on some operating systems.
 ///
+/// \note The maximum number of message arrays that can be added to the
+/// dictionary in this way is defined by the constant
+/// MessageInitializer::MAX_MESSAGE_ARRAYS.  This is set to 256 as a compromise
+/// between wasted space and allowing for future expansion, but can be
+/// changed (by editing the source file) to any value desired.
+///
 /// When messages are added to the dictionary, the are added via the
 /// MessageDictionary::add() method, so any duplicates are stored in the
 /// global dictionary's overflow vector whence they can be retrieved at
@@ -58,6 +65,8 @@ namespace log {
 
 class MessageInitializer {
 public:
+    /// Maximum number of message arrays that can be initialized in this way
+    static const size_t MAX_MESSAGE_ARRAYS = 256;
 
     /// \brief Constructor
     ///
@@ -65,9 +74,21 @@ public:
     /// pointers to message arrays.
     ///
     /// \param values NULL-terminated array of alternating identifier strings
-    /// and associated message text.
+    /// and associated message text. N.B. This object stores a pointer to the
+    /// passed array; the array MUST remain valid at least until
+    /// loadDictionary() has been called.
     MessageInitializer(const char* values[]);
 
+    /// \brief Obtain pending load count
+    ///
+    /// Returns a count of the message of message arrays that have been
+    /// registered with this class and will be loaded with the next call
+    /// to loadDictionary().
+    ///
+    /// \return Number of registered message arrays.  This is reset to zero
+    ///         when loadDictionary() is called.
+    static size_t getPendingCount();
+
     /// \brief Run-Time Initialization
     ///
     /// Loops through the internal array of pointers to message arrays

src/lib/log/tests/Makefile.am

@@ -12,6 +12,24 @@ CLEANFILES = *.gcno *.gcda
 
 TESTS =
 if HAVE_GTEST
+
+# Define the flags used in each set of tests
+COMMON_CXXFLAGS = $(AM_CXXFLAGS)
+if USE_CLANGPP
+# Workaround unused variables tcout and tcerr in log4cplus's streams.h.
+COMMON_CXXFLAGS += -Wno-unused-variable
+endif
+COMMON_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) $(LOG4CPLUS_INCLUDES)
+COMMON_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)
+
+COMMON_LDADD  = $(GTEST_LDADD)
+COMMON_LDADD += $(top_builddir)/src/lib/log/liblog.la
+COMMON_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
+COMMON_LDADD += $(top_builddir)/src/lib/util/libutil.la
+COMMON_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
+COMMON_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
+
+# Set of unit tests for the general logging classes
 TESTS += run_unittests
 run_unittests_SOURCES  = run_unittests.cc
 run_unittests_SOURCES += log_formatter_unittest.cc
@@ -23,26 +41,37 @@ run_unittests_SOURCES += logger_support_unittest.cc
 run_unittests_SOURCES += logger_unittest.cc
 run_unittests_SOURCES += logger_specification_unittest.cc
 run_unittests_SOURCES += message_dictionary_unittest.cc
-run_unittests_SOURCES += message_initializer_unittest_2.cc
-run_unittests_SOURCES += message_initializer_unittest.cc
 run_unittests_SOURCES += message_reader_unittest.cc
 run_unittests_SOURCES += output_option_unittest.cc
 
-run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) $(LOG4CPLUS_INCLUDES)
-run_unittests_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)
+run_unittests_CPPFLAGS = $(COMMON_CPPFLAGS)
+run_unittests_CXXFLAGS = $(COMMON_CXXFLAGS)
+run_unittests_LDADD    = $(COMMON_LDADD)
+run_unittests_LDFLAGS  = $(COMMON_LDFLAGS)
+
+# logging initialization tests.  These are put in separate programs to
+# ensure that the initialization status at the start of each test is known,
+# and to prevent circumstances where the execution of one test affects the
+# starting conditions of the next.
+TESTS += initializer_unittests_1
+initializer_unittests_1_SOURCES  = run_initializer_unittests.cc
+initializer_unittests_1_SOURCES += message_initializer_1_unittest.cc
+initializer_unittests_1_SOURCES += message_initializer_1a_unittest.cc
+
+initializer_unittests_1_CPPFLAGS = $(COMMON_CPPFLAGS)
+initializer_unittests_1_CXXFLAGS = $(COMMON_CXXFLAGS)
+initializer_unittests_1_LDADD    = $(COMMON_LDADD)
+initializer_unittests_1_LDFLAGS  = $(COMMON_LDFLAGS)
+
+TESTS += initializer_unittests_2
+initializer_unittests_2_SOURCES  = run_initializer_unittests.cc
+initializer_unittests_2_SOURCES += message_initializer_2_unittest.cc
+
+initializer_unittests_2_CPPFLAGS = $(COMMON_CPPFLAGS)
+initializer_unittests_2_CXXFLAGS = $(COMMON_CXXFLAGS)
+initializer_unittests_2_LDADD    = $(COMMON_LDADD)
+initializer_unittests_2_LDFLAGS  = $(COMMON_LDFLAGS)
 
-run_unittests_CXXFLAGS = $(AM_CXXFLAGS)
-if USE_CLANGPP
-# This is to workaround unused variables tcout and tcerr in
-# log4cplus's streams.h.
-run_unittests_CXXFLAGS += -Wno-unused-variable
-endif
-run_unittests_LDADD  = $(GTEST_LDADD)
-run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
-run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
-run_unittests_LDADD += $(top_builddir)/src/lib/util/libutil.la
-run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
-run_unittests_LDADD += $(top_builddir)/src/lib/util/unittests/libutil_unittests.la
 endif
 
 noinst_PROGRAMS = logger_example

src/lib/log/tests/message_initializer_unittest.cc

@@ -12,9 +12,9 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <cstddef>
 #include <string>
 #include <gtest/gtest.h>
+#include <boost/lexical_cast.hpp>
 #include <log/message_dictionary.h>
 #include <log/message_initializer.h>
 
@@ -40,27 +40,36 @@ const char* values2[] = {
 }
 
 // Statically initialize the global dictionary with those messages.  Three sets
-// are used to check that the declaration of separate initializer objects really// does combine the messages. (The third set is declared in the separately-
-// compiled file message_identifier_initializer_unittest_2.cc.)
+// are used to check that the declaration of separate initializer objects really
+// does combine the messages. (The third set - declaring message IDs GLOBAL5
+// and GLOBAL6) is declared in the separately-compiled file,
+// message_identifier_initializer_1a_unittest.cc.)
 
 MessageInitializer init_message_initializer_unittest_1(values1);
 MessageInitializer init_message_initializer_unittest_2(values2);
 
-
-class MessageInitializerTest : public ::testing::Test {
-protected:
-    MessageInitializerTest()
-    {
-    }
-};
-
-
 // Check that the global dictionary is initialized with the specified
 // messages.
 
-TEST_F(MessageInitializerTest, MessageTest) {
+TEST(MessageInitializerTest1, MessageTest) {
     MessageDictionary& global = MessageDictionary::globalDictionary();
 
+    // Pointers to the message arrays should have been stored, but none of the
+    // messages should yet be in the dictionary.
+    for (int i = 1; i <= 6; ++i) {
+        string symbol = string("GLOBAL") + boost::lexical_cast<std::string>(i);
+        EXPECT_EQ(string(""), global.getText(symbol));
+    }
+
+    // Load the dictionary - this should clear the message array pending count.
+    // (N.B. We do not check for a known value before the call, only that the
+    // value is not zero.  This is because libraries against which the test
+    // is linked may have registered ther own message arrays.)
+    EXPECT_NE(0, MessageInitializer::getPendingCount());
+    MessageInitializer::loadDictionary();
+    EXPECT_EQ(0, MessageInitializer::getPendingCount());
+
+    // ... and check the messages loaded.
     EXPECT_EQ(string("global message one"), global.getText("GLOBAL1"));
     EXPECT_EQ(string("global message two"), global.getText("GLOBAL2"));
     EXPECT_EQ(string("global message three"), global.getText("GLOBAL3"));

src/lib/log/tests/message_initializer_unittest_2.cc

@@ -33,7 +33,5 @@ const char* values3[] = {
 
 }
 
-// Statically initialize the global dictionary with those messages.
-// Three sets are used to check that the declaration of separate
-// initializer objects really does combine the messages.
+// Register the messages for loading into the global dictionary
 MessageInitializer init_message_initializer_unittest_3(values3);

src/lib/log/tests/message_initializer_2_unittest.cc

@@ -0,0 +1,44 @@
+// Copyright (C) 2011  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 <gtest/gtest.h>
+#include <log/message_initializer.h>
+
+using namespace isc::log;
+
+// Declare a set of messages to go into the global dictionary.
+
+namespace {
+const char* values[] = {
+    "GLOBAL1", "global message one",
+    "GLOBAL2", "global message two",
+    NULL
+};
+}
+
+TEST(MessageInitializerTest2, MessageLoadTest) {
+    // Load the maximum number of message arrays allowed.  Some arrays may
+    // already have been loaded because of static initialization from modules
+    // in libraries liked against the test program, hence the reason for the
+    // loop starting from the value returned by getPendingCount() instead of 0.
+    for (size_t i = MessageInitializer::getPendingCount();
+         i < MessageInitializer::MAX_MESSAGE_ARRAYS; ++i) {
+        MessageInitializer initializer1(values);
+    }
+
+    // Adding one more should take us over the limit.
+    ASSERT_DEATH({
+        MessageInitializer initializer2(values);
+        }, ".*");
+}

src/lib/log/tests/run_initializer_unittests.cc

@@ -0,0 +1,24 @@
+// Copyright (C) 2011  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 <gtest/gtest.h>
+#include <util/unittests/run_all.h>
+
+#include <log/logger_support.h>
+
+int
+main(int argc, char* argv[]) {
+    ::testing::InitGoogleTest(&argc, argv);
+    return (isc::util::unittests::run_all());
+}