Stub out system functions for testing (WIP).

This is a more thorough and should be an overall better approach than
stubbing/mocking the base_proc_service class itself.

This commit introduces a baseproc-sys.h header which holds aliases for
the system functions that are used (kill, fcntl, etc). When running
tests the functions will be mocked (currently they are rigged to abort
when called).

diff --git a/src/baseproc-service.cc b/src/baseproc-service.cc
index 1645b0561930693550f3946943bcce0bb6bb6954..a424c2a8a5f17b52d0eff861c8958f4ba8086aba 100644 (file)
--- a/src/baseproc-service.cc
+++ b/src/baseproc-service.cc
@@ -5,6 +5,8 @@
 #include "dinit-socket.h"
 #include "proc-service.h"
 
+#include "baseproc-sys.h"
+
 /*
  * Base process implementation (base_process_service).
  *
@@ -55,7 +57,7 @@ bool base_process_service::start_ps_process(const std::vector<const char *> &cmd
     event_loop.get_time(last_start_time, clock_type::MONOTONIC);
 
     int pipefd[2];
-    if (dasynq::pipe2(pipefd, O_CLOEXEC)) {
+    if (bp_sys::pipe2(pipefd, O_CLOEXEC)) {
         log(loglevel_t::ERROR, get_name(), ": can't create status check pipe: ", strerror(errno));
         return false;
     }
@@ -76,8 +78,8 @@ bool base_process_service::start_ps_process(const std::vector<const char *> &cmd
         }
 
         // Make the server side socket close-on-exec:
-        int fdflags = fcntl(control_socket[0], F_GETFD);
-        fcntl(control_socket[0], F_SETFD, fdflags | FD_CLOEXEC);
+        int fdflags = bp_sys::fcntl(control_socket[0], F_GETFD);
+        bp_sys::fcntl(control_socket[0], F_SETFD, fdflags | FD_CLOEXEC);
 
         try {
             control_conn = new control_conn_t(event_loop, services, control_socket[0]);
@@ -113,9 +115,9 @@ bool base_process_service::start_ps_process(const std::vector<const char *> &cmd
     }
     else {
         // Parent process
-        close(pipefd[1]); // close the 'other end' fd
+        bp_sys::close(pipefd[1]); // close the 'other end' fd
         if (control_socket[1] != -1) {
-            close(control_socket[1]);
+            bp_sys::close(control_socket[1]);
         }
         pid = forkpid;
 
@@ -134,13 +136,13 @@ bool base_process_service::start_ps_process(const std::vector<const char *> &cmd
         delete control_conn;
 
         out_cs:
-        close(control_socket[0]);
-        close(control_socket[1]);
+        bp_sys::close(control_socket[0]);
+        bp_sys::close(control_socket[1]);
     }
 
     out_p:
-    close(pipefd[0]);
-    close(pipefd[1]);
+    bp_sys::close(pipefd[0]);
+    bp_sys::close(pipefd[1]);
 
     return false;
 }
@@ -312,7 +314,7 @@ void base_process_service::kill_pg(int signo) noexcept
         log(loglevel_t::ERROR, get_name(), ": can't signal process: ", strerror(errno));
         return;
     }
-    kill(-pgid, signo);
+    bp_sys::kill(-pgid, signo);
 }
 
 void base_process_service::timer_expired() noexcept

diff --git a/src/includes/baseproc-sys.h b/src/includes/baseproc-sys.h
new file mode 100644 (file)
index 0000000..05ebbd3
--- /dev/null
+++ b/src/includes/baseproc-sys.h
@@ -0,0 +1,9 @@
+namespace bp_sys {
+
+using dasynq::pipe2;
+
+using ::fcntl;
+using ::close;
+using ::kill;
+
+}

diff --git a/src/tests/Makefile b/src/tests/Makefile
index a49dc4a29c88f4008698bb124558c7d2d424dd4b..b4b115310395aec3f411745f8bc85655d02aee91 100644 (file)
--- a/src/tests/Makefile
+++ b/src/tests/Makefile
@@ -1,7 +1,7 @@
 -include ../../mconfig
 
-objects = tests.o test-dinit.o proctests.o test-baseproc.o
-parent_objs = service.o proc-service.o dinit-log.o load_service.o # baseproc-service.o
+objects = tests.o test-dinit.o proctests.o test-run-child-proc.o
+parent_objs = service.o proc-service.o dinit-log.o load_service.o baseproc-service.o
 
 check: build-tests
        ./tests
@@ -16,11 +16,11 @@ prepare-incdir:
        cd includes; ln -sf ../../includes/*.h .
        cd includes; ln -sf ../test-includes/*.h .
 
-tests: prepare-incdir $(parent_objs) tests.o test-dinit.o test-baseproc.o
-       $(CXX) $(SANITIZEOPTS) -o tests $(parent_objs) tests.o test-dinit.o test-baseproc.o $(EXTRA_LIBS)
+tests: prepare-incdir $(parent_objs) tests.o test-dinit.o
+       $(CXX) $(SANITIZEOPTS) -o tests $(parent_objs) tests.o test-dinit.o $(EXTRA_LIBS)
 
-proctests: prepare-incdir $(parent_objs) proctests.o test-dinit.o test-baseproc.o
-       $(CXX) $(SANITIZEOPTS) -o proctests $(parent_objs) proctests.o test-dinit.o test-baseproc.o $(EXTRA_LIBS)
+proctests: prepare-incdir $(parent_objs) proctests.o test-dinit.o
+       $(CXX) $(SANITIZEOPTS) -o proctests $(parent_objs) proctests.o test-dinit.o $(EXTRA_LIBS)
 
 $(objects): %.o: %.cc
        $(CXX) $(CXXOPTS) $(SANITIZEOPTS) -Iincludes -I../dasynq -c $< -o $@

diff --git a/src/tests/test-baseproc.cc b/src/tests/test-baseproc.cc
deleted file mode 100644 (file)
index c1a4103..0000000
--- a/src/tests/test-baseproc.cc
+++ /dev/null
@@ -1,235 +0,0 @@
-#include "dinit.h"
-#include "proc-service.h"
-
-// This is a mock implementation of the base_process_service class, for testing purposes.
-
-base_process_service::base_process_service(service_set *sset, string name,
-        service_type_t service_type_p, string &&command,
-        std::list<std::pair<unsigned,unsigned>> &command_offsets,
-        const std::list<prelim_dep> &deplist_p)
-     : service_record(sset, name, service_type_p, deplist_p), child_listener(this),
-       child_status_listener(this), restart_timer(this)
-{
-    program_name = std::move(command);
-    exec_arg_parts = separate_args(program_name, command_offsets);
-
-    restart_interval_count = 0;
-    restart_interval_time = {0, 0};
-    restart_timer.service = this;
-    //restart_timer.add_timer(event_loop);
-
-    // By default, allow a maximum of 3 restarts within 10.0 seconds:
-    restart_interval.seconds() = 10;
-    restart_interval.nseconds() = 0;
-    max_restart_interval_count = 3;
-
-    waiting_restart_timer = false;
-    reserved_child_watch = false;
-    tracking_child = false;
-    stop_timer_armed = false;
-    start_is_interruptible = false;
-}
-
-bool base_process_service::bring_up() noexcept
-{
-    if (restarting) {
-        if (pid == -1) {
-            return restart_ps_process();
-        }
-        return true;
-    }
-    else {
-        //event_loop.get_time(restart_interval_time, clock_type::MONOTONIC);
-        restart_interval_count = 0;
-        if (start_ps_process(exec_arg_parts, onstart_flags.starts_on_console)) {
-            if (start_timeout != time_val(0,0)) {
-                //restart_timer.arm_timer_rel(event_loop, start_timeout);
-                stop_timer_armed = true;
-            }
-            else if (stop_timer_armed) {
-                //restart_timer.stop_timer(event_loop);
-                stop_timer_armed = false;
-            }
-            return true;
-        }
-        return false;
-    }
-}
-
-void base_process_service::bring_down() noexcept
-{
-    waiting_for_deps = false;
-    if (pid != -1) {
-        // The process is still kicking on - must actually kill it. We signal the process
-        // group (-pid) rather than just the process as there's less risk then of creating
-        // an orphaned process group:
-        if (! onstart_flags.no_sigterm) {
-            //kill_pg(SIGTERM);
-        }
-        if (term_signal != -1) {
-            //kill_pg(term_signal);
-        }
-
-        // In most cases, the rest is done in handle_exit_status.
-        // If we are a BGPROCESS and the process is not our immediate child, however, that
-        // won't work - check for this now:
-        if (get_type() == service_type_t::BGPROCESS && ! tracking_child) {
-            stopped();
-        }
-        else if (stop_timeout != time_val(0,0)) {
-            //restart_timer.arm_timer_rel(event_loop, stop_timeout);
-            stop_timer_armed = true;
-        }
-    }
-    else {
-        // The process is already dead.
-        stopped();
-    }
-}
-
-void base_process_service::do_smooth_recovery() noexcept
-{
-    if (! restart_ps_process()) {
-        emergency_stop();
-        services->process_queues();
-    }
-}
-
-bool base_process_service::start_ps_process(const std::vector<const char *> &cmd, bool on_console) noexcept
-{
-    return false;
-}
-
-void base_process_service::kill_with_fire() noexcept
-{
-    if (pid != -1) {
-        //log(loglevel_t::WARN, "Service ", get_name(), " with pid ", pid, " exceeded allowed stop time; killing.");
-        //kill_pg(SIGKILL);
-    }
-}
-
-void base_process_service::kill_pg(int signo) noexcept
-{
-    //pid_t pgid = getpgid(pid);
-    //if (pgid == -1) {
-        // only should happen if pid is invalid, which should never happen...
-        //log(loglevel_t::ERROR, get_name(), ": can't signal process: ", strerror(errno));
-        //return;
-    //}
-    //kill(-pgid, signo);
-}
-
-bool base_process_service::restart_ps_process() noexcept
-{
-    using time_val = dasynq::time_val;
-
-    time_val current_time;
-    event_loop.get_time(current_time, clock_type::MONOTONIC);
-
-    if (max_restart_interval_count != 0) {
-        // Check whether we're still in the most recent restart check interval:
-        time_val int_diff = current_time - restart_interval_time;
-        if (int_diff < restart_interval) {
-            if (restart_interval_count >= max_restart_interval_count) {
-                //log(loglevel_t::ERROR, "Service ", get_name(), " restarting too quickly; stopping.");
-                return false;
-            }
-        }
-        else {
-            restart_interval_time = current_time;
-            restart_interval_count = 0;
-        }
-    }
-
-    // Check if enough time has lapsed since the prevous restart. If not, start a timer:
-    time_val tdiff = current_time - last_start_time;
-    if (restart_delay <= tdiff) {
-        // > restart delay (normally 200ms)
-        do_restart();
-    }
-    else {
-        time_val timeout = restart_delay - tdiff;
-        //restart_timer.arm_timer_rel(event_loop, timeout);
-        waiting_restart_timer = true;
-    }
-    return true;
-}
-
-void base_process_service::do_restart() noexcept
-{
-    waiting_restart_timer = false;
-    restart_interval_count++;
-    auto service_state = get_state();
-
-    // We may be STARTING (regular restart) or STARTED ("smooth recovery"). This affects whether
-    // the process should be granted access to the console:
-    bool on_console = service_state == service_state_t::STARTING
-            ? onstart_flags.starts_on_console : onstart_flags.runs_on_console;
-
-    if (service_state == service_state_t::STARTING) {
-        // for a smooth recovery, we want to check dependencies are available before actually
-        // starting:
-        if (! check_deps_started()) {
-            waiting_for_deps = true;
-            return;
-        }
-    }
-
-    if (! start_ps_process(exec_arg_parts, on_console)) {
-        restarting = false;
-        if (service_state == service_state_t::STARTING) {
-            failed_to_start();
-        }
-        else {
-            // desired_state = service_state_t::STOPPED;
-            forced_stop();
-        }
-        services->process_queues();
-    }
-}
-
-bool base_process_service::interrupt_start() noexcept
-{
-    if (waiting_restart_timer) {
-        //restart_timer.stop_timer(event_loop);
-        waiting_restart_timer = false;
-        return service_record::interrupt_start();
-    }
-    else {
-        //log(loglevel_t::WARN, "Interrupting start of service ", get_name(), " with pid ", pid, " (with SIGINT).");
-        kill_pg(SIGINT);
-        if (stop_timeout != time_val(0,0)) {
-            restart_timer.arm_timer_rel(event_loop, stop_timeout);
-            stop_timer_armed = true;
-        }
-        else if (stop_timer_armed) {
-            restart_timer.stop_timer(event_loop);
-            stop_timer_armed = false;
-        }
-        set_state(service_state_t::STOPPING);
-        notify_listeners(service_event_t::STARTCANCELLED);
-        return false;
-    }
-}
-
-void base_process_service::timer_expired() noexcept
-{
-    stop_timer_armed = false;
-
-    // Timer expires if:
-    // We are stopping, including after having startup cancelled (stop timeout, state is STOPPING); We are
-    // starting (start timeout, state is STARTING); We are waiting for restart timer before restarting,
-    // including smooth recovery (restart timeout, state is STARTING or STARTED).
-    if (get_state() == service_state_t::STOPPING) {
-        kill_with_fire();
-    }
-    else if (pid != -1) {
-        // Starting, start timed out.
-        stop_dependents();
-        interrupt_start();
-    }
-    else {
-        // STARTING / STARTED, and we have a pid: must be restarting (smooth recovery if STARTED)
-        do_restart();
-    }
-}

diff --git a/src/tests/test-includes/baseproc-sys.h b/src/tests/test-includes/baseproc-sys.h
new file mode 100644 (file)
index 0000000..b12c495
--- /dev/null
+++ b/src/tests/test-includes/baseproc-sys.h
@@ -0,0 +1,31 @@
+#include <sys/types.h>
+
+// Mock system functions for testing.
+
+namespace bp_sys {
+
+inline int pipe2(int pipefd[2], int flags)
+{
+    abort();
+    return 0;
+}
+
+inline int fcntl(int fd, int cmd, ...)
+{
+    // This is used for setting the CLOEXEC flag, we can just return 0:
+    return 0;
+}
+
+inline int close(int fd)
+{
+    abort();
+    return 0;
+}
+
+inline int kill(pid_t pid, int sig)
+{
+    abort();
+    return 0;
+}
+
+}

diff --git a/src/tests/test-run-child-proc.cc b/src/tests/test-run-child-proc.cc
new file mode 100644 (file)
index 0000000..3cd81fc
--- /dev/null
+++ b/src/tests/test-run-child-proc.cc
@@ -0,0 +1,9 @@
+#include "service.h"
+
+// Stub out run_child_proc function, for testing purposes.
+
+void service_record::run_child_proc(const char * const *args, const char *logfile, bool on_console,
+        int wpipefd, int csfd) noexcept
+{
+
+}