Browse Source

[trac3687] Add unittests

Some cleanup and add unittests
Shawn Routhier 10 years ago
parent
commit
5ba6d26513

+ 1 - 1
src/lib/util/Makefile.am

@@ -18,6 +18,7 @@ libkea_util_la_SOURCES += time_utilities.h time_utilities.cc
 libkea_util_la_SOURCES += memory_segment.h
 libkea_util_la_SOURCES += memory_segment_local.h memory_segment_local.cc
 libkea_util_la_SOURCES += optional_value.h
+libkea_util_la_SOURCES += pid_file.h pid_file.cc
 libkea_util_la_SOURCES += range_utilities.h
 libkea_util_la_SOURCES += signal_set.cc signal_set.h
 libkea_util_la_SOURCES += encode/base16_from_binary.h
@@ -28,7 +29,6 @@ libkea_util_la_SOURCES += encode/binary_from_base32hex.h
 libkea_util_la_SOURCES += encode/binary_from_base16.h
 libkea_util_la_SOURCES += random/qid_gen.h random/qid_gen.cc
 libkea_util_la_SOURCES += random/random_number_generator.h
-libkea_util_la_SOURCES += pid.h pid.cc
 
 libkea_util_la_LIBADD = $(top_builddir)/src/lib/exceptions/libkea-exceptions.la
 CLEANFILES = *.gcno *.gcda

+ 25 - 29
src/lib/util/pid.cc

@@ -12,42 +12,42 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <util/pid.h>
+#include <util/pid_file.h>
 #include <cstdio>
 #include <signal.h>
+#include <unistd.h>
 
 namespace isc {
 namespace util {
 
-pidFile::pidFile(const std::string& filename)
+PIDFile::PIDFile(const std::string& filename)
     : filename_(filename) {
 }
 
+PIDFile::~PIDFile() {
+}
+
 bool
-pidFile::check() {
-    boost::shared_ptr<std::ifstream> fs;
+PIDFile::check() {
+    std::ifstream fs(filename_.c_str());
     int pid;
     bool good;
 
-    fs.reset(new std::ifstream(filename_.c_str()));
-
     // If we weren't able to open the file treat
     // it as if the process wasn't running
-    if (fs->is_open() == false) {
-	fs.reset();
-	return (false);
+    if (fs.is_open() == false) {
+        return (false);
     }
 
     // Try to get the pid, get the status and get rid of the file
-    *fs >> pid;
-    good = fs->good();
-    fs->close();
-    fs.reset();
+    fs >> pid;
+    good = fs.good();
+    fs.close();
 
     // Treat being unable to read a pid the same as if we
     // got a pid and the process is still running.
     if ((good == false) || (kill(pid, 0) == 0)) {
-	return (true);
+        return (true);
     }
 
     // No process
@@ -55,37 +55,33 @@ pidFile::check() {
 }
 
 void
-pidFile::write() {
+PIDFile::write() {
     write(getpid());
 }
 
 void
-pidFile::write(int pid) {
-    boost::shared_ptr<std::ofstream> fs;
-
-    fs.reset(new std::ofstream(filename_.c_str()));
-    if (fs->is_open() == false) {
-	fs.reset();
-	isc_throw(pidFileError, "Unable to open PID file '"
-		  << filename_ << "' for write");
+PIDFile::write(int pid) {
+    std::ofstream fs(filename_.c_str());
+
+    if (fs.is_open() == false) {
+        isc_throw(PIDFileError, "Unable to open PID file '"
+                  << filename_ << "' for write");
     }
 
     // File is open, write the pid.
-    *fs << pid << std::endl;
+    fs << pid << std::endl;
 
     // That's it
-    fs->close();
-    fs.reset();
+    fs.close();
 }
 
 void
-pidFile::deleteFile() {
+PIDFile::deleteFile() {
     if (remove(filename_.c_str()) != 0) {
-        isc_throw(pidFileError, "Unable to delete PID file '"
+	isc_throw(PIDFileError, "Unable to delete PID file '"
 		  << filename_ << "'");
     }
 }
 
 } // namespace isc::util
 } // namespace isc
-

+ 10 - 14
src/lib/util/pid.h

@@ -12,12 +12,10 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#ifndef PID_H
-#define PID_H
+#ifndef PID_FILE_H
+#define PID_FILE_H
 
 #include <exceptions/exceptions.h>
-#include <boost/lexical_cast.hpp>
-#include <boost/shared_ptr.hpp>
 #include <fstream>
 #include <ostream>
 #include <string>
@@ -26,35 +24,35 @@ namespace isc {
 namespace util {
 
 /// @brief Exception thrown when an error occurs during PID file processing.
-class pidFileError : public Exception {
+class PIDFileError : public Exception {
 public:
-    pidFileError(const char* file, size_t line, const char* what) :
+    PIDFileError(const char* file, size_t line, const char* what) :
     isc::Exception(file, line, what) { };
 };
 
 /// @brief Class to help with processing PID files
 ///
 /// This is a utility class to manipulate PID files.  It provides
-/// functiosn for writing and deleting a file containing a PID as
+/// functions for writing and deleting a file containing a PID as
 /// well as for extracting a PID from a file and checking if the
 /// process is still running.
 
-class pidFile {
+class PIDFile {
 public:
     /// @brief Constructor
     ///
     /// @param filename PID filename.
-    pidFile(const std::string& filename);
+    PIDFile(const std::string& filename);
 
     /// @brief Destructor
-    ~pidFile();
+    ~PIDFile();
 
     /// @brief Read the PID in from the file and check it.
     ///
     /// Read the PID in from the file then use it to see if
     /// a process with that PID exists.  If the file doesn't
     /// exist treat it as the process not being there.
-    /// If the file exists but can't be read or it doesn't
+    /// If the file exists but can't be read or it doesn't have
     /// the proper format treat it as the process existing.
     ///
     /// @return true if the PID is in use, false otherwise
@@ -82,6 +80,4 @@ private:
 } // namespace isc::util
 } // namespace isc
 
-#endif // PID_H
-
-
+#endif // PID_FILE_H

+ 1 - 1
src/lib/util/tests/Makefile.am

@@ -38,6 +38,7 @@ run_unittests_SOURCES += memory_segment_local_unittest.cc
 run_unittests_SOURCES += memory_segment_common_unittest.h
 run_unittests_SOURCES += memory_segment_common_unittest.cc
 run_unittests_SOURCES += optional_value_unittest.cc
+run_unittests_SOURCES += pid_file_unittest.cc
 run_unittests_SOURCES += qid_gen_unittest.cc
 run_unittests_SOURCES += random_number_generator_unittest.cc
 run_unittests_SOURCES += socketsession_unittest.cc
@@ -45,7 +46,6 @@ run_unittests_SOURCES += strutil_unittest.cc
 run_unittests_SOURCES += time_utilities_unittest.cc
 run_unittests_SOURCES += range_utilities_unittest.cc
 run_unittests_SOURCES += signal_set_unittest.cc
-run_unittests_SOURCES += pid_unittest.cc
 
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)

+ 150 - 0
src/lib/util/tests/pid_file_unittest.cc

@@ -0,0 +1,150 @@
+// Copyright (C) 2015 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 <util/pid_file.h>
+#include <gtest/gtest.h>
+#include <signal.h>
+
+namespace {
+using namespace isc::util;
+
+#define TESTNAME "./pid_file.test"
+
+/// @brief Test file writing and deleteion. Start by removing
+/// any leftover file. Then write a known PID to the file and
+/// attempt to read the file and verify the PID. Next write
+/// a second and verify a second PID to verify that an existing
+/// file is properly overwritten.
+
+TEST(PIDFileTest, writeAndDelete) {
+    PIDFile pid_file(TESTNAME);
+    std::ifstream fs;
+    int pid(0);
+
+    // Remove any leftovers
+    remove(TESTNAME);
+
+    // Write a known process id
+    pid_file.write(10);
+
+    // Read the file and comapre the pid
+    fs.open(TESTNAME, std::ifstream::in);
+    fs >> pid;
+    EXPECT_TRUE(fs.good());
+    EXPECT_EQ(pid, 10);
+    fs.close();
+
+    // Write a second known process id
+    pid_file.write(20);
+
+    // And comapre the second pid
+    fs.open(TESTNAME, std::ifstream::in);
+    fs >> pid;
+    EXPECT_TRUE(fs.good());
+    EXPECT_EQ(pid, 20);
+    fs.close();
+
+    // Delete the file
+    pid_file.deleteFile();
+
+    // And verify that it's gone
+    fs.open(TESTNAME, std::ifstream::in);
+    EXPECT_FALSE(fs.good());
+    fs.close();
+}
+
+/// @brief Test checking a PID. Write the PID of the current
+/// process to the PID file then verify that check indicates
+/// the process is running.
+TEST(PIDFileTest, pidInUse) {
+    PIDFile pid_file(TESTNAME);
+
+    // Write the current PID
+    pid_file.write();
+
+    // Check if we think the process is running
+    EXPECT_TRUE(pid_file.check());
+
+    // Delete the file
+    pid_file.deleteFile();
+}
+
+/// @brief Test checking a PID. Write a PID that isn't in use
+/// to the PID file and verify that check indicates the process
+/// isn't running. The PID may get used between when we select it
+/// and write the file and when we check it. To minimize false
+/// errors if the first call to check fails we try again with a
+/// different range of values and only if both attempts fail do
+/// we declare the test to have failed.
+TEST(PIDFileTest, pidNotInUse) {
+    PIDFile pid_file(TESTNAME);
+    int pid;
+
+    // get a pid betwen 10000 and 20000
+    for (pid = (random() % 10000) + 10000;
+        kill(pid, 0) == 0;
+        ++pid)
+        ;
+
+    // write it
+    pid_file.write(pid);
+
+    // Check to see if we think the process is running
+    if (pid_file.check() == false) {
+        // Delete the file
+        pid_file.deleteFile();
+        return;
+    }
+
+    // get a pid betwen 40000 and 50000
+    for (pid = (random() % 10000) + 40000;
+        kill(pid, 0) == 0;
+        ++pid)
+        ;
+
+    // write it
+    pid_file.write(pid);
+
+    // Check to see if we think the process is running
+    EXPECT_FALSE(pid_file.check());
+
+    // Delete the file
+    pid_file.deleteFile();
+}
+
+/// @brief Test checking a PID.  Write garbage to the PID file
+/// and verify that check throws an error. In this situation
+/// the caller should probably log an error and may decide to
+/// continue or not depending on the requirements.
+TEST(PIDFileTest, pidGarbage) {
+    PIDFile pid_file(TESTNAME);
+    std::ofstream fs;
+
+    // Remove any leftovers
+    remove(TESTNAME);
+
+    // Open the file and write garbage to it
+    fs.open(TESTNAME, std::ofstream::out);
+    fs << "text" << std::endl;
+    fs.close();
+
+    // Run the check, we expect to find a process "running"
+    EXPECT_TRUE(pid_file.check());
+
+    // And clean up the file
+    pid_file.deleteFile();
+
+}
+
+} // end of anonymous namespace

+ 0 - 21
src/lib/util/tests/pid_unittest.cc

@@ -1,21 +0,0 @@
-// Copyright (C) 2015 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 <util/pid.h>
-
-namespace {
-using namespace isc::util;
-
-} // end of anonymous namespace
-