]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#153,!94] Addressed review comments.
authorMarcin Siodelski <marcin@isc.org>
Tue, 23 Oct 2018 14:46:06 +0000 (16:46 +0200)
committerMarcin Siodelski <marcin@isc.org>
Tue, 23 Oct 2018 16:41:26 +0000 (12:41 -0400)
Moved ThreadedTest to testutils. Also, added commentary to the test.

src/bin/netconf/tests/control_socket_unittests.cc
src/lib/testutils/Makefile.am
src/lib/testutils/threaded_test.cc [new file with mode: 0644]
src/lib/testutils/threaded_test.h [new file with mode: 0644]

index 0ac9016e17fbe5c556accb7e2d6c4d9f50b10ed8..ce2cf033993db8148cc481d4273fa7cce1255d47 100644 (file)
@@ -18,6 +18,7 @@
 #include <http/response_creator_factory.h>
 #include <http/response_json.h>
 #include <http/tests/response_test.h>
+#include <testutils/threaded_test.h>
 #include <util/threads/thread.h>
 #include <util/threads/sync.h>
 #include <gtest/gtest.h>
@@ -30,6 +31,7 @@ using namespace isc::asiolink;
 using namespace isc::data;
 using namespace isc::http;
 using namespace isc::http::test;
+using namespace isc::test;
 using namespace isc::util::thread;
 
 namespace {
@@ -37,100 +39,6 @@ namespace {
 /// @brief Type definition for the pointer to Thread objects.
 typedef boost::shared_ptr<Thread> ThreadPtr;
 
-/// @brief Base class for tests requiring threads.
-///
-/// This class contains 3 flags to signal when the thread has
-/// started, is stopping and when it is stopped. The flags
-/// are accessed in thread safe manner.
-class ThreadedTest : public ::testing::Test {
-protected:
-
-    /// @brief Constructor.
-    ThreadedTest()
-        : thread_(), condvar_(), ready_(false), stopping_(false),
-          stopped_(false) {
-    }
-
-    /// @brief Sets selected flag to true and signals condition
-    /// variable.
-    ///
-    /// @param flag Reference to flag which should be set to true.
-    void doSignal(bool& flag) {
-        {
-            Mutex::Locker lock(mutex_);
-            flag = true;
-        }
-        condvar_.signal();
-    }
-
-    /// @brief Signal that thread is ready.
-    void signalReady() {
-        doSignal(ready_);
-    }
-
-    /// @brief Signal that thread is stopping.
-    void signalStopping() {
-        doSignal(stopping_);
-    }
-
-    /// @brief Signal that thread is stopped.
-    void signalStopped() {
-        doSignal(stopped_);
-    }
-
-    /// @brief Wait for a selected flag to be set.
-    ///
-    /// @param flag Reference to a flag on which the thread is
-    /// waiting.
-    void doWait(bool& flag) {
-        Mutex::Locker lock(mutex_);
-        while (!flag) {
-            condvar_.wait(mutex_);
-        }
-    }
-
-    /// @brief Wait for the thread to be ready.
-    void waitReady() {
-        doWait(ready_);
-    }
-
-    /// @brief Wait for the thread to be stopping.
-    void waitStopping() {
-        doWait(stopping_);
-    }
-
-    /// @brief Wait for the thread to stop.
-    void waitStopped() {
-        doWait(stopped_);
-    }
-
-    /// @brief Checks if the thread is stopping.
-    ///
-    /// @return true if the thread is stopping, false otherwise.
-    bool isStopping() {
-        Mutex::Locker lock(mutex_);
-        return (stopping_);
-    }
-
-    /// @brief Pointer to server thread.
-    ThreadPtr thread_;
-
-    /// @brief Mutex used to synchronize threads.
-    Mutex mutex_;
-
-    /// Condtional variable for thread waits.
-    CondVar condvar_;
-
-    /// Flag indicating that the thread is ready.
-    bool ready_;
-
-    /// Flag indicating that the thread is stopping.
-    bool stopping_;
-
-    /// Flag indicating that the thread is stopped.
-    bool stopped_;
-};
-
 //////////////////////////////// STDOUT ////////////////////////////////
 
 /// @brief Test derived StdoutControlSocket class.
@@ -640,15 +548,27 @@ public:
     /// Run IO in a thread.
     void start() {
         thread_.reset(new Thread([this]() {
+            // The thread is ready to go. Signal it to the main
+            // thread so it can start the actual test.
             signalReady();
+            // Until stop() is called run IO service.
             while (!isStopping()) {
                 io_service_.run_one();
             }
+            // Main thread signalled that the thread should
+            // terminate. Launch any outstanding IO service
+            // handlers.
             io_service_.poll();
+            // Nothing more to do. Signal that the thread is
+            // done so as the main thread can close HTTP
+            // listener and clean up after the test.
             signalStopped();
         }));
+
+        // Main thread waits here for the thread to start.
         waitReady();
 
+        // If the thread is ready to go, start the listener.
         if (listener_) {
             ASSERT_NO_THROW(listener_->start());
         }
@@ -658,10 +578,18 @@ public:
     ///
     /// Post an empty action to finish current run_one.
     void stop() {
+        // Notify the thread that it should terminate.
         signalStopping();
+        // If the thread is blocked on running the IO
+        // service, post the empty handler to cause
+        // run_one to return.
         io_service_.post([]() { return; });
+        // We asked that the thread stops. Let's wait
+        // for it to signal that it has stopped.
         waitStopped();
 
+        // Thread has terminated. We can stop the HTTP
+        // listener safely.
         if (listener_) {
             ASSERT_NO_THROW(listener_->stop());
         }
index fc7665ec2265f71058acf68f4c151460f7c6a382..bc3c4a23a0f350249f34bab0e49eb6e7e56395b6 100644 (file)
@@ -10,6 +10,7 @@ noinst_LTLIBRARIES = libkea-testutils.la
 libkea_testutils_la_SOURCES  = io_utils.cc io_utils.h
 libkea_testutils_la_SOURCES += log_utils.cc log_utils.h
 libkea_testutils_la_SOURCES += test_to_element.cc test_to_element.h
+libkea_testutils_la_SOURCES += threaded_test.cc threaded_test.h
 libkea_testutils_la_SOURCES += unix_control_client.cc unix_control_client.h
 libkea_testutils_la_SOURCES += user_context_utils.cc user_context_utils.h
 libkea_testutils_la_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
diff --git a/src/lib/testutils/threaded_test.cc b/src/lib/testutils/threaded_test.cc
new file mode 100644 (file)
index 0000000..2302771
--- /dev/null
@@ -0,0 +1,73 @@
+// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC")
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#include <testutils/threaded_test.h>
+
+using namespace isc::util::thread;
+
+namespace isc {
+namespace test {
+
+ThreadedTest::ThreadedTest()
+    : thread_(), condvar_(), ready_(false), stopping_(false),
+      stopped_(false) {
+}
+
+void
+ThreadedTest::doSignal(bool& flag) {
+    {
+        Mutex::Locker lock(mutex_);
+        flag = true;
+    }
+    condvar_.signal();
+}
+
+void
+ThreadedTest::signalReady() {
+    doSignal(ready_);
+}
+
+void
+ThreadedTest::signalStopping() {
+    doSignal(stopping_);
+}
+
+void
+ThreadedTest::signalStopped() {
+    doSignal(stopped_);
+}
+
+void
+ThreadedTest::doWait(bool& flag) {
+    Mutex::Locker lock(mutex_);
+    while (!flag) {
+        condvar_.wait(mutex_);
+    }
+}
+
+void
+ThreadedTest::waitReady() {
+    doWait(ready_);
+}
+
+void
+ThreadedTest::waitStopping() {
+    doWait(stopping_);
+}
+
+void
+ThreadedTest::waitStopped() {
+    doWait(stopped_);
+}
+
+bool
+ThreadedTest::isStopping() {
+    Mutex::Locker lock(mutex_);
+    return (stopping_);
+}
+
+} // end of namespace isc::test
+} // end of namespace isc
diff --git a/src/lib/testutils/threaded_test.h b/src/lib/testutils/threaded_test.h
new file mode 100644 (file)
index 0000000..537aec8
--- /dev/null
@@ -0,0 +1,87 @@
+// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC")
+//
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#ifndef THREADED_TEST_H
+#define THREADED_TEST_H
+
+#include <util/threads/thread.h>
+#include <util/threads/sync.h>
+#include <boost/shared_ptr.hpp>
+#include <gtest/gtest.h>
+
+namespace isc {
+namespace test {
+
+/// @brief Base class for tests requiring threads.
+///
+/// This class contains 3 flags to signal when the thread has
+/// started, is stopping and when it is stopped. The flags
+/// are accessed in thread safe manner.
+class ThreadedTest : public ::testing::Test {
+protected:
+
+    /// @brief Constructor.
+    ThreadedTest();
+
+    /// @brief Sets selected flag to true and signals condition
+    /// variable.
+    ///
+    /// @param flag Reference to flag which should be set to true.
+    void doSignal(bool& flag);
+
+    /// @brief Signal that thread is ready.
+    void signalReady();
+
+    /// @brief Signal that thread is stopping.
+    void signalStopping();
+
+    /// @brief Signal that thread is stopped.
+    void signalStopped();
+
+    /// @brief Wait for a selected flag to be set.
+    ///
+    /// @param flag Reference to a flag on which the thread is
+    /// waiting.
+    void doWait(bool& flag);
+
+    /// @brief Wait for the thread to be ready.
+    void waitReady();
+
+    /// @brief Wait for the thread to be stopping.
+    void waitStopping();
+
+    /// @brief Wait for the thread to stop.
+    void waitStopped();
+
+    /// @brief Checks if the thread is stopping.
+    ///
+    /// @return true if the thread is stopping, false otherwise.
+    bool isStopping();
+
+    /// @brief Pointer to server thread.
+    boost::shared_ptr<util::thread::Thread> thread_;
+
+    /// @brief Mutex used to synchronize threads.
+    util::thread::Mutex mutex_;
+
+    /// Condtional variable for thread waits.
+    util::thread::CondVar condvar_;
+
+    /// Flag indicating that the thread is ready.
+    bool ready_;
+
+    /// Flag indicating that the thread is stopping.
+    bool stopping_;
+
+    /// Flag indicating that the thread is stopped.
+    bool stopped_;
+};
+
+
+} // end of namespace isc::test
+} // end of namespace isc
+
+#endif