refactor(utils): centralize and test failure reporting in SystemUtils

src/utils/system_utils.cpp

@@ -14,62 +14,62 @@ SystemUtils::SystemUtils(std::shared_ptr<UserInterface> ui) : ui(ui) {
 }
 
 /**
- * Evaluates the status code from a system() call and handles errors appropriately.
+ * Reports a failure and ends the program with EXIT_FAILURE.
  *
- * This method interprets POSIX wait status codes using WIFEXITED and WEXITSTATUS macros.
- * If an error is detected (status -1, non-zero exit code, or abnormal termination),
- * an error message is displayed via the UserInterface or printed non-interactively,
- * and the program exits with status code 1.
+ * In interactive mode, the message is shown through the UserInterface and the
+ * session is ended with EXIT_FAILURE. Otherwise, the message is printed
+ * non-interactively and the process exits immediately with EXIT_FAILURE.
  *
- * A successful command (exit code 0) produces no output and returns normally.
+ * @param message The error message to display. The messages passed from
+ *                catch_error each end with a newline.
  *
- * Error cases handled:
- * - system() call failure (status == -1): displays "system() failed" error.
- * - Non-zero exit code: displays the exit code and terminates the program.
- * - Abnormal termination: displays "Command terminated abnormally" and terminates the program.
+ * @note In interactive mode, this relies on UserInterface::end_session() not
+ *       returning. If it does return, report_failure returns to its caller.
+ */
+void SystemUtils::report_failure(const std::string& message) {
+    if (UserInterface::ui_running(ui)) {
+        ui->print_error(message);
+        ui->end_session(EXIT_FAILURE);
+    } else {
+        UserInterface::print_error_noninteractive(message);
+        std::exit(EXIT_FAILURE);
+    }
+}
+
+/**
+ * Evaluates the wait status from a system() call and reports any failure.
+ *
+ * This method interprets the status with the POSIX macros WIFEXITED and
+ * WEXITSTATUS. A status of -1 means system() itself failed, and only the
+ * "system() failed." message is reported. A status where the process did not
+ * exit normally is reported as "Command terminated abnormally." A normal exit
+ * with a non-zero code is reported with that code. A successful command
+ * (exit code 0) produces no output and returns normally.
  *
- * Output routing:
- * - If UserInterface is in interactive mode, errors are sent via ui->print_error().
- * - Otherwise, errors are printed non-interactively via UserInterface::print_message_noninteractive().
+ * Each failure is passed to report_failure(), which shows the message through
+ * the UserInterface in interactive mode or prints it non-interactively
+ * otherwise, then ends the program with EXIT_FAILURE.
  *
- * @param status The wait status code returned from a system() call.
- *               Interpret using POSIX macros WIFEXITED and WEXITSTATUS.
+ * @param status The wait status returned from a system() call, as interpreted
+ *               by the POSIX macros WIFEXITED and WEXITSTATUS.
  *
- * @throws Terminates the program with exit code 1 on any detected error condition.
+ * @throws Does not throw. Ends the program with EXIT_FAILURE on any detected
+ *         error, through report_failure().
  *
- * @note This method never returns after encountering an error; callers should not
- *       expect execution to continue beyond a non-zero exit code or abnormal termination.
+ * @note Non-interactive failures terminate the process immediately. Interactive
+ *       failures return to the caller only if UserInterface::end_session() returns.
  */
 void SystemUtils::catch_error(int status) {
-    std::string message{};
     if (status == -1) {
-        message = "system() failed.\n";
-        if (UserInterface::ui_running(this->ui) == false) {
-            UserInterface::print_error_noninteractive(message);
-        } else {
-            this->ui->print_error(message);
-        }
+        this->report_failure("system() failed.\n");
+        return;
     }
-    if (WIFEXITED(status)) {
-        int exit_code = WEXITSTATUS(status);
-        if (exit_code != 0) {
-            message = std::format("Command failed with exit code {}.\n", exit_code);
-            if (UserInterface::ui_running(this->ui) == false) {
-                UserInterface::print_error_noninteractive(message);
-                std::exit(EXIT_FAILURE);
-            } else {
-                this->ui->print_error(message);
-                this->ui->end_session(EXIT_FAILURE);
-            }
-        }
-    } else {
-        message = std::format("Command terminated abnormally.\n");
-        if (UserInterface::ui_running(this->ui) == false) {
-            UserInterface::print_error_noninteractive(message);
-            std::exit(EXIT_FAILURE);
-        } else {
-            this->ui->print_error(message);
-            this->ui->end_session(EXIT_FAILURE);
-        }
+    if (!WIFEXITED(status)) {
+        this->report_failure("Command terminated abnormally.\n");
+        return;
+    }
+    int exit_code = WEXITSTATUS(status);
+    if (exit_code != 0) {
+        this->report_failure(std::format("Command failed with exit code {}.\n", exit_code));
     }
 }

src/utils/system_utils.hpp

@@ -10,6 +10,8 @@ class SystemUtils {
      */
     std::shared_ptr<UserInterface> ui;
 
+    void report_failure(const std::string& message);
+
   public:
     explicit SystemUtils(std::shared_ptr<UserInterface> ui);
 

tests/interface/user_interface_mock.cpp

@@ -0,0 +1,43 @@
+#include "interface/user_interface.hpp"
+
+#include <cstdio>
+#include <string>
+#include <vector>
+
+namespace fake_ui {
+
+struct Calls {
+    bool running = false;
+    std::vector<std::string> errors;
+    std::vector<int> end_codes;
+    std::vector<std::string> noninteractive;
+};
+
+inline Calls calls; // Test code resets and inspects this.
+
+} // namespace fake_ui
+
+UserInterface::UserInterface() : input_cursor_x(0), layout({}), use_dark_theme(true) {
+}
+
+UserInterface::UserInterface(bool dark_theme) : input_cursor_x(0), layout({}), use_dark_theme(dark_theme) {
+}
+
+UserInterface::~UserInterface() = default;
+
+bool UserInterface::ui_running(const std::shared_ptr<UserInterface>&) {
+    return fake_ui::calls.running;
+}
+
+void UserInterface::print_error(const std::string& text) {
+    fake_ui::calls.errors.push_back(text);
+}
+
+void UserInterface::end_session(int code) {
+    fake_ui::calls.end_codes.push_back(code);
+}
+
+void UserInterface::print_error_noninteractive(const std::string& text) {
+    fake_ui::calls.noninteractive.push_back(text);
+    std::fputs(text.c_str(), stderr);
+}

tests/interface/user_interface_mock.hpp

@@ -0,0 +1,14 @@
+#pragma once
+
+#include <string>
+#include <vector>
+
+namespace fake_ui {
+struct Calls {
+    bool running = false;
+    std::vector<std::string> errors;
+    std::vector<int> end_codes;
+    std::vector<std::string> noninteractive;
+};
+extern Calls calls;
+} // namespace fake_ui

tests/utils/system_utils_tests.cpp

@@ -0,0 +1,144 @@
+#include <gtest/gtest.h>
+
+#include <csignal>
+#include <cstdlib>
+#include <memory>
+#include <sys/wait.h>
+#include <vector>
+
+#include "../tests/interface/user_interface_mock.hpp"
+#include "interface/user_interface.hpp"
+#include "utils/system_utils.hpp"
+
+namespace {
+
+// Builds the wait status for a normal exit with the given code.
+constexpr int exited_with(int code) {
+    return code << 8;
+}
+
+// Builds the wait status for a process stopped by a signal (WIFSTOPPED).
+constexpr int stopped_by(int sig) {
+    return 0x7f | (sig << 8);
+}
+
+// Used by death tests so the whole call runs in the child process.
+void run_catch_error(int status) {
+    SystemUtils su(std::make_shared<UserInterface>());
+    su.catch_error(status);
+}
+
+} // namespace
+
+class SystemUtils_catch_error : public ::testing::Test {
+  protected:
+    // cppcheck-suppress unusedFunction
+    void SetUp() override {
+        fake_ui::calls = {};
+    }
+};
+
+// ---------- Zero exit codes ----------
+
+TEST_F(SystemUtils_catch_error, zero_exit_is_silent_interactive) {
+    fake_ui::calls.running = true;
+    SystemUtils su(std::make_shared<UserInterface>());
+
+    su.catch_error(exited_with(0));
+
+    EXPECT_TRUE(fake_ui::calls.errors.empty());
+    EXPECT_TRUE(fake_ui::calls.end_codes.empty());
+    EXPECT_TRUE(fake_ui::calls.noninteractive.empty());
+}
+
+TEST_F(SystemUtils_catch_error, zero_exit_is_silent_non_interactive) {
+    fake_ui::calls.running = false;
+    SystemUtils su(std::make_shared<UserInterface>());
+
+    su.catch_error(exited_with(0));
+
+    EXPECT_TRUE(fake_ui::calls.errors.empty());
+    EXPECT_TRUE(fake_ui::calls.end_codes.empty());
+    EXPECT_TRUE(fake_ui::calls.noninteractive.empty());
+}
+
+// ---------- Non-zero exit codes ----------
+
+TEST_F(SystemUtils_catch_error, non_zero_exit_interactive_reports_and_ends_session) {
+    fake_ui::calls.running = true;
+    SystemUtils su(std::make_shared<UserInterface>());
+
+    su.catch_error(exited_with(3));
+
+    ASSERT_EQ(fake_ui::calls.errors.size(), 1u);
+    EXPECT_EQ(fake_ui::calls.errors[0], "Command failed with exit code 3.\n");
+    EXPECT_EQ(fake_ui::calls.end_codes, std::vector<int>{EXIT_FAILURE});
+    EXPECT_TRUE(fake_ui::calls.noninteractive.empty());
+}
+
+TEST_F(SystemUtils_catch_error, non_zero_exit_non_interactive_reports_and_exits) {
+    fake_ui::calls.running = false;
+
+    EXPECT_EXIT(run_catch_error(exited_with(3)), ::testing::ExitedWithCode(EXIT_FAILURE),
+                "Command failed with exit code 3\\.");
+}
+
+TEST_F(SystemUtils_catch_error, exit_code_255_is_reported_exactly) {
+    fake_ui::calls.running = true;
+    SystemUtils su(std::make_shared<UserInterface>());
+
+    su.catch_error(exited_with(255));
+
+    ASSERT_EQ(fake_ui::calls.errors.size(), 1u);
+    EXPECT_EQ(fake_ui::calls.errors[0], "Command failed with exit code 255.\n");
+    EXPECT_EQ(fake_ui::calls.end_codes, std::vector<int>{EXIT_FAILURE});
+}
+
+// ---------- Abnormal termination ----------
+
+TEST_F(SystemUtils_catch_error, killed_by_signal_interactive_reports_abnormal_termination) {
+    fake_ui::calls.running = true;
+    SystemUtils su(std::make_shared<UserInterface>());
+
+    su.catch_error(SIGKILL); // Low 7 bits hold the signal, so WIFEXITED is false.
+
+    ASSERT_EQ(fake_ui::calls.errors.size(), 1u);
+    EXPECT_EQ(fake_ui::calls.errors[0], "Command terminated abnormally.\n");
+    EXPECT_EQ(fake_ui::calls.end_codes, std::vector<int>{EXIT_FAILURE});
+}
+
+TEST_F(SystemUtils_catch_error, killed_by_signal_non_interactive_exits) {
+    fake_ui::calls.running = false;
+
+    EXPECT_EXIT(run_catch_error(SIGKILL), ::testing::ExitedWithCode(EXIT_FAILURE), "Command terminated abnormally\\.");
+}
+
+TEST_F(SystemUtils_catch_error, stopped_process_is_treated_as_abnormal) {
+    fake_ui::calls.running = true;
+    SystemUtils su(std::make_shared<UserInterface>());
+
+    su.catch_error(stopped_by(SIGSTOP));
+
+    ASSERT_EQ(fake_ui::calls.errors.size(), 1u);
+    EXPECT_EQ(fake_ui::calls.errors[0], "Command terminated abnormally.\n");
+    EXPECT_EQ(fake_ui::calls.end_codes, std::vector<int>{EXIT_FAILURE});
+}
+
+// ---------- system() failure (-1) ----------
+
+TEST_F(SystemUtils_catch_error, system_failure_interactive_reports_only_once) {
+    fake_ui::calls.running = true;
+    SystemUtils su(std::make_shared<UserInterface>());
+
+    su.catch_error(-1);
+
+    ASSERT_EQ(fake_ui::calls.errors.size(), 1u);
+    EXPECT_EQ(fake_ui::calls.errors[0], "system() failed.\n");
+    EXPECT_EQ(fake_ui::calls.end_codes, std::vector<int>{EXIT_FAILURE});
+}
+
+TEST_F(SystemUtils_catch_error, system_failure_non_interactive_reports_only_once) {
+    fake_ui::calls.running = false;
+
+    EXPECT_EXIT(run_catch_error(-1), ::testing::ExitedWithCode(EXIT_FAILURE), "system\\(\\) failed\\.");
+}

tools/shell/format.sh

@@ -48,6 +48,7 @@ cppcheck_check() {
     --error-exitcode=1 \
     --suppress=missingIncludeSystem \
     --suppress=unusedLabel \
+    --inline-suppr \
     -I src/ \
     -I src/interface \
     -I src/ollama \