From: Tomek Mrugalski <tomasz@isc.org>
Date: Thu, 20 Aug 2015 14:31:03 +0000 (+0200)
Subject: [trac4000] Unit-tests for process spawn + errno, verbose exceptions added.
X-Git-Tag: trac4003_base~1^2~2
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=f71885fb8a70be809aa30cde1031fc55658c6b0f;p=thirdparty%2Fkea.git

[trac4000] Unit-tests for process spawn + errno, verbose exceptions added.
---

diff --git a/src/lib/exceptions/tests/exceptions_unittest.cc b/src/lib/exceptions/tests/exceptions_unittest.cc
index 44cbc17737..4e93ce8955 100644
--- a/src/lib/exceptions/tests/exceptions_unittest.cc
+++ b/src/lib/exceptions/tests/exceptions_unittest.cc
@@ -16,6 +16,7 @@
 #include <string>
 
 #include <exceptions/exceptions.h>
+#include <sstream>
 
 #include <gtest/gtest.h>
 
@@ -47,4 +48,24 @@ TEST_F(ExceptionTest, stdInheritance) {
         EXPECT_EQ(std::string(ex.what()), std::string(teststring));
     }
 }
+
+// Tests whether verbose is disabled by default and can be enabled, if
+// needed.
+TEST_F(ExceptionTest, verbose) {
+
+    // This code is line numbers sensitive. Make sure no edits are done between
+    // this line and isc_throw below. Update that +3 offset, if needed.
+    std::stringstream expected;
+    expected << teststring << "[" << std::string(__FILE__)
+             << ":" << int(__LINE__ + 3) << "]";
+
+    try {
+        isc_throw(Exception, teststring);
+    } catch (const isc::Exception& ex) {
+        EXPECT_EQ(std::string(ex.what()), std::string(teststring));
+        EXPECT_EQ(std::string(ex.what(false)), std::string(teststring));
+        EXPECT_EQ(expected.str(), std::string(ex.what(true)));
+    }
+
+}
 }
diff --git a/src/lib/util/tests/process_spawn_unittest.cc b/src/lib/util/tests/process_spawn_unittest.cc
index 50e217d888..1aea10df15 100644
--- a/src/lib/util/tests/process_spawn_unittest.cc
+++ b/src/lib/util/tests/process_spawn_unittest.cc
@@ -16,6 +16,7 @@
 #include <gtest/gtest.h>
 #include <signal.h>
 #include <stdint.h>
+#include <errno.h>
 #include <sys/types.h>
 #include <unistd.h>
 
@@ -36,24 +37,67 @@ std::string getApp() {
     return (s.str());
 }
 
-/// @brief Waits for the specified process to finish.
+/// @brief Waits for the specified process to finish (using sleep)
 ///
 /// @param process An object which started the process.
 /// @param pid ID of the spawned process.
 /// @param timeout Timeout in seconds.
+/// @param sleep specifies whether usleep(1ms) should be used or not.
+///
+/// This method uses usleep() to wait between checks. For an alternative,
+/// see waitForProcessFast(). Note: If the signal comes in when the
+/// loop calls usleep(), usleep() will be aborted and errno will be set
+/// to EINTR to indicate that interruption. Therefore this method is not
+/// suitable for tests that want to observe errno value. See
+/// @ref waitForProcessFast as an alternative.
 ///
 /// @return true if the process ended, false otherwise
 bool waitForProcess(const ProcessSpawn& process, const pid_t pid,
                     const uint8_t timeout) {
     uint32_t iterations = 0;
     const uint32_t iterations_max = timeout * 1000;
+
+    int errno_save = errno;
     while (process.isRunning(pid) && (iterations < iterations_max)) {
         usleep(1000);
         ++iterations;
     }
+    errno = errno_save;
     return (iterations < iterations_max);
 }
 
+/// @brief Waits for the specified process to finish (using fast loop)
+///
+/// @param process An object which started the process.
+/// @param pid ID of the spawned process.
+/// @param timeout Timeout in seconds.
+/// @param sleep specifies whether usleep(1ms) should be used or not.
+///
+/// This method does not use any sleep() calls, but rather iterates through
+/// the loop very fast. This is not recommended in general, but is necessary
+/// to avoid updating errno by sleep() after receving a signal.
+///
+/// Note: the timeout is only loosely accurate. Depending on the fraction
+/// of second it was started on, it may terminate later by up to almost
+/// whole second.
+///
+/// @return true if the process ended, false otherwise
+bool waitForProcessFast(const ProcessSpawn& process, const pid_t pid,
+                        const uint8_t timeout) {
+    time_t before = time(NULL);
+    while (process.isRunning(pid)) {
+        time_t now = time(NULL);
+
+        // The difference before we started and now is greater than
+        // the timeout, we should give up.
+        if (now - before > timeout) {
+            return (false);
+        }
+    }
+
+    return (true);
+}
+
 // This test verifies that the external application can be ran with
 // arguments and that the exit code is gathered.
 TEST(ProcessSpawn, spawnWithArgs) {
@@ -169,4 +213,30 @@ TEST(ProcessSpawn, isRunning) {
     ASSERT_TRUE(waitForProcess(process, pid, 2));
 }
 
+// This test verifies that the signal handler does not modify value of
+// errno.
+TEST(ProcessSpawn, errnoInvariance) {
+
+    // Set errno to an arbitrary value. We'll later check that it was not
+    // stumped on.
+    errno = 123;
+
+    std::vector<std::string> args;
+    args.push_back("-e");
+    args.push_back("64");
+    ProcessSpawn process(getApp(), args);
+    pid_t pid = 0;
+    ASSERT_NO_THROW(pid = process.spawn());
+
+    ASSERT_TRUE(waitForProcessFast(process, pid, 2));
+
+    EXPECT_EQ(64, process.getExitStatus(pid));
+
+    // errno value should NOT be set to be preserved, despite the SIGCHILD
+    // handler (ProcessSpawnImpl::waitForProcess) calling waitpid(), which
+    // will likely set errno to ECHILD. See trac4000 and support 8785.
+    EXPECT_EQ(123, errno);
+}
+
+
 } // end of anonymous namespace