fix(tests): replace libexpect with forkpty() for functional tests

Remove dependency on libexpect which had a segfault bug on Arch Linux.

- Replace exp_spawnl() with forkpty() for PTY creation
- Replace exp_expectl() with custom POSIX regex matching
- Update configure.ac to check for forkpty() instead of libexpect
- Update Makefile.am to link with libutil instead of libexpect/libtcl
- Remove expect/tcl packages from all Dockerfiles
- Add Valgrind suppressions for stabber/pthread false positives

Dockerfile.arch

@@ -36,8 +36,7 @@ RUN pacman -S --needed --noconfirm \
   sqlite \
   valgrind \
   gdk-pixbuf2 \
-  qrencode \
-  expect
+  qrencode
 
 RUN mkdir -p /usr/src/{stabber,profanity}
 
@@ -59,10 +58,6 @@ USER root
 
 RUN pacman -U --noconfirm libstrophe-git/libstrophe-git-*.pkg.tar.zst
 
-# Create symlink for libexpect - Arch puts it in versioned subdirectory
-# which causes configure to fail with "libexpect not found"
-RUN ln -sf /usr/lib/expect5.45.4/libexpect5.45.4.so /usr/lib/libexpect.so
-
 WORKDIR /usr/src
 RUN git clone -c http.sslverify=false https://git.jabber.space/devs/stabber
 

Dockerfile.debian

@@ -8,7 +8,6 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
   autoconf \
   autoconf-archive \
   automake \
-  expect \
   gcc \
   git \
   libcmocka-dev \
@@ -33,8 +32,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
   valgrind \
   libsqlite3-dev \
   libgdk-pixbuf-2.0-dev \
-  libqrencode-dev \
-  tcl-expect-dev
+  libqrencode-dev
 
 RUN mkdir -p /usr/src/{stabber,libstrophe,profanity}
 WORKDIR /usr/src

Dockerfile.fedora

@@ -1,7 +1,6 @@
 # Build the latest Fedora image
 FROM fedora:latest
 
-# expect - for functional tests
 # libmicrohttpd - for stabber
 # glibc-locale - to have en_US locale
 RUN dnf install -y \
@@ -9,7 +8,6 @@ RUN dnf install -y \
   autoconf-archive \
   automake \
   awk \
-  expect-devel \
   gcc \
   git \
   glib2-devel \

Dockerfile.tumbleweed

@@ -1,14 +1,12 @@
 # Build the latest openSUSE Tumbleweed image
 FROM opensuse/tumbleweed
 
-# expect - for functional tests
 # libmicrohttpd - for stabber
 # glibc-locale - to have en_US locale
 RUN zypper --non-interactive in --no-recommends \
   autoconf \
   autoconf-archive \
   automake \
-  expect-devel \
   gcc \
   git \
   glib2-devel \

Dockerfile.ubuntu

@@ -7,7 +7,6 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
   autoconf \
   autoconf-archive \
   automake \
-  expect \
   gcc \
   git \
   libcmocka-dev \
@@ -32,8 +31,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
   valgrind \
   libsqlite3-dev \
   libgdk-pixbuf-2.0-dev \
-  libqrencode-dev \
-  tcl-expect-dev
+  libqrencode-dev
 
 RUN mkdir -p /usr/src/{stabber,libstrophe,profanity}
 WORKDIR /usr/src

Makefile.am

@@ -290,21 +290,21 @@ tests_unittests_unittests_CPPFLAGS = -Itests/
 tests_unittests_unittests_SOURCES = $(unittest_sources)
 tests_unittests_unittests_LDADD = -lcmocka
 
-# Functional tests require libstabber and libexpect.
-# They are only built when both libraries are available.
+# Functional tests require libstabber.
+# They are only built when libstabber is available.
 # See: https://github.com/profanity-im/profanity/pull/1010
 #      https://github.com/profanity-im/stabber/issues/5
 #
-# Note: Debian/Ubuntu's libexpect requires -ltcl8.6 for linkage.
+# Note: We use forkpty() instead of libexpect for PTY handling.
 
 if HAVE_STABBER
-if HAVE_EXPECT
+if HAVE_FORKPTY
 TESTS += tests/functionaltests/functionaltests
 check_PROGRAMS += tests/functionaltests/functionaltests
 tests_functionaltests_functionaltests_SOURCES = $(functionaltest_sources)
 tests_functionaltests_functionaltests_CPPFLAGS = -Itests/
-tests_functionaltests_functionaltests_CFLAGS = $(AM_CFLAGS) -I/usr/include/tcl8.6 -I/usr/include/tcl8.5
-tests_functionaltests_functionaltests_LDADD = -lcmocka -lstabber -lexpect -ltcl8.6
+tests_functionaltests_functionaltests_CFLAGS = $(AM_CFLAGS)
+tests_functionaltests_functionaltests_LDADD = -lcmocka -lstabber @FORKPTY_LIB@
 endif
 endif
 

configure.ac

@@ -374,13 +374,13 @@ PKG_CHECK_MODULES([cmocka], [cmocka], [],
 AM_CONDITIONAL([HAVE_STABBER], [false])
 AC_CHECK_LIB([stabber], [stbbr_start], [AM_CONDITIONAL([HAVE_STABBER], [true])],
     [AC_MSG_NOTICE([libstabber not found, will not be able to run functional tests])])
-AM_CONDITIONAL([HAVE_EXPECT], [false])
-dnl libexpect on Debian/Ubuntu requires -ltcl8.6 for linking
-saved_LIBS="$LIBS"
-LIBS="$LIBS -ltcl8.6"
-AC_CHECK_LIB([expect], [exp_expectl], [AM_CONDITIONAL([HAVE_EXPECT], [true])],
-    [AC_MSG_NOTICE([libexpect not found, will not be able to run functional tests])])
-LIBS="$saved_LIBS"
+dnl Check for forkpty (needed for functional tests PTY handling)
+dnl On Linux it's in libutil, on some BSDs it's in libc
+AM_CONDITIONAL([HAVE_FORKPTY], [false])
+AC_CHECK_LIB([util], [forkpty], [AM_CONDITIONAL([HAVE_FORKPTY], [true]) FORKPTY_LIB="-lutil"],
+    [AC_CHECK_FUNC([forkpty], [AM_CONDITIONAL([HAVE_FORKPTY], [true]) FORKPTY_LIB=""],
+        [AC_MSG_NOTICE([forkpty not found, will not be able to run functional tests])])])
+AC_SUBST([FORKPTY_LIB])
 
 ## Default parameters
 AM_CFLAGS="$AM_CFLAGS -Wall -Wno-deprecated-declarations -std=gnu99 -ggdb3"

prof.supp

@@ -8,6 +8,104 @@
 # * python suppressions file from https://github.com/python/cpython/blob/main/Misc/valgrind-python.supp
 #
 
+# ============================================
+# Functional tests suppressions (stabber/pthread)
+# ============================================
+
+{
+  stabber_recv_shutdown
+  Memcheck:Param
+  socketcall.recvfrom(buf)
+  fun:recv
+  fun:_shutdown
+  ...
+  obj:*/libstabber*
+}
+
+{
+  stabber_recv_read_stream
+  Memcheck:Param
+  socketcall.recvfrom(buf)
+  fun:recv
+  ...
+  fun:read_stream
+  ...
+  obj:*/libstabber*
+}
+
+{
+  stabber_pthread_create
+  Memcheck:Leak
+  match-leak-kinds: possible
+  fun:calloc
+  ...
+  fun:allocate_dtv
+  fun:_dl_allocate_tls
+  ...
+  fun:pthread_create*
+  ...
+  fun:server_run
+  ...
+}
+
+{
+  stabber_server_run
+  Memcheck:Leak
+  match-leak-kinds: possible
+  fun:calloc
+  ...
+  fun:pthread_create*
+  ...
+  obj:*/libstabber*
+  ...
+}
+
+{
+  glib_time_zone_cache
+  Memcheck:Leak
+  match-leak-kinds: reachable
+  ...
+  fun:g_time_zone_new*
+  ...
+}
+
+{
+  glib_time_zone_local
+  Memcheck:Leak
+  match-leak-kinds: reachable
+  ...
+  fun:g_strdup
+  ...
+  fun:g_time_zone_new_identifier
+  fun:g_time_zone_new_local
+  ...
+}
+
+{
+  glib_date_time_format
+  Memcheck:Leak
+  match-leak-kinds: reachable
+  ...
+  fun:g_date_time_format
+  ...
+}
+
+{
+  glib_date_time_format_locale
+  Memcheck:Leak
+  match-leak-kinds: reachable
+  fun:*alloc
+  ...
+  obj:*/libglib*
+  ...
+  fun:g_date_time_format
+  ...
+}
+
+# ============================================
+# Original suppressions
+# ============================================
+
 {
   _dl_init
   Memcheck:Leak
@@ -2742,3 +2840,245 @@
    fun:exp_expectl
 }
 
+
+# Additional suppressions for functional tests
+
+# libexpect still reachable allocations
+{
+   exp_spawnv_malloc
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:malloc
+   ...
+   fun:exp_spawnv
+   fun:exp_spawnl
+}
+
+{
+   exp_spawnl_realloc
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:realloc
+   ...
+   fun:exp_spawnv
+   fun:exp_spawnl
+}
+
+# libtcl memory pool allocations (expected)
+{
+   tcl_alloc_pool
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:malloc
+   ...
+   fun:TclpAlloc
+   fun:Tcl_Alloc
+}
+
+{
+   tcl_alloc_pool_calloc
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:calloc
+   ...
+   fun:TclpAlloc
+   fun:Tcl_Alloc
+}
+
+# pthread thread-local storage (normal for multi-threaded programs)
+{
+   pthread_tls_allocate_dtv
+   Memcheck:Leak
+   match-leak-kinds: possible
+   fun:calloc
+   fun:calloc
+   fun:allocate_dtv
+   fun:_dl_allocate_tls
+   fun:allocate_stack
+   fun:pthread_create*
+}
+
+# glib static initializations
+{
+   glib_hash_table_init
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:malloc
+   fun:g_malloc
+   fun:g_hash_table_new_full
+   ...
+   fun:call_init
+   fun:_dl_init
+}
+
+{
+   glib_array_init
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:realloc
+   fun:g_realloc
+   ...
+   fun:call_init
+   fun:_dl_init
+}
+
+# fdopen/fopen allocations (FILE* buffers - normal)
+{
+   fdopen_file_buffer
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:malloc
+   ...
+   fun:fdopen*
+}
+
+{
+   fopen_file_buffer
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:malloc
+   ...
+   fun:fopen*
+}
+
+# stabber log_init (server log file)
+{
+   stabber_log_init
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:malloc
+   ...
+   fun:log_init
+   fun:server_run
+}
+
+# glib time zone (static allocation)
+{
+   g_time_zone_new_local
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   ...
+   fun:g_time_zone_new_local
+}
+
+{
+   g_time_zone_array
+   Memcheck:Leak
+   match-leak-kinds: reachable
+   fun:realloc
+   fun:g_realloc
+   ...
+   fun:g_array_sized_new
+   fun:g_time_zone_new_identifier
+}
+
+# stabber log g_date_time_format invalid read - benign race in logging
+{
+   stabber_g_date_time_format_invalid_read
+   Memcheck:Addr1
+   ...
+   fun:g_date_time_format
+   fun:log_println
+   ...
+}
+
+{
+   stabber_g_date_time_format_invalid_read2
+   Memcheck:Addr2
+   ...
+   fun:g_date_time_format
+   fun:log_println
+   ...
+}
+
+{
+   stabber_g_date_time_format_invalid_read4
+   Memcheck:Addr4
+   ...
+   fun:g_date_time_format
+   fun:log_println
+   ...
+}
+
+{
+   stabber_g_date_time_format_invalid_read8
+   Memcheck:Addr8
+   ...
+   fun:g_date_time_format
+   fun:log_println
+   ...
+}
+
+# More generic suppression for g_date_time_format race condition
+{
+   g_date_time_format_cond
+   Memcheck:Cond
+   ...
+   fun:g_date_time_format
+   ...
+}
+
+{
+   g_date_time_format_value1
+   Memcheck:Value1
+   ...
+   fun:g_date_time_format
+   ...
+}
+
+{
+   g_date_time_format_value2
+   Memcheck:Value2
+   ...
+   fun:g_date_time_format
+   ...
+}
+
+{
+   g_date_time_format_value4
+   Memcheck:Value4
+   ...
+   fun:g_date_time_format
+   ...
+}
+
+{
+   g_date_time_format_value8
+   Memcheck:Value8
+   ...
+   fun:g_date_time_format
+   ...
+}
+
+# Suppress all Addr errors in log_println
+{
+   log_println_addr1
+   Memcheck:Addr1
+   ...
+   fun:log_println
+   ...
+}
+
+{
+   log_println_addr2
+   Memcheck:Addr2
+   ...
+   fun:log_println
+   ...
+}
+
+{
+   log_println_addr4
+   Memcheck:Addr4
+   ...
+   fun:log_println
+   ...
+}
+
+{
+   log_println_addr8
+   Memcheck:Addr8
+   ...
+   fun:log_println
+   ...
+}

tests/functionaltests/proftest.c

@@ -8,9 +8,12 @@
 #include <unistd.h>
 #include <errno.h>
 #include <string.h>
+#include <pty.h>
+#include <fcntl.h>
+#include <sys/select.h>
+#include <regex.h>
 
 #include <stabber.h>
-#include <expect.h>
 
 #include "proftest.h"
 
@@ -19,6 +22,15 @@ char *data_orig;
 
 int fd = 0;
 int stub_port = 5230;
+pid_t child_pid = 0;
+
+/* Buffer for accumulating output from profanity */
+#define OUTPUT_BUF_SIZE 65536
+static char output_buffer[OUTPUT_BUF_SIZE];
+static size_t output_len = 0;
+
+/* Timeout for expect operations in seconds */
+static int expect_timeout = 30;
 
 gboolean
 _create_dir(const char *name)
@@ -119,24 +131,83 @@ _cleanup_dirs(void)
     }
 }
 
+/*
+ * Read available data from fd into output_buffer with timeout.
+ * Returns number of bytes read, 0 on timeout, -1 on error.
+ */
+static int
+_read_output(int timeout_ms)
+{
+    fd_set readfds;
+    struct timeval tv;
+    
+    FD_ZERO(&readfds);
+    FD_SET(fd, &readfds);
+    
+    tv.tv_sec = timeout_ms / 1000;
+    tv.tv_usec = (timeout_ms % 1000) * 1000;
+    
+    int ret = select(fd + 1, &readfds, NULL, NULL, &tv);
+    if (ret <= 0) {
+        return ret;
+    }
+    
+    size_t space = OUTPUT_BUF_SIZE - output_len - 1;
+    if (space <= 0) {
+        /* Buffer full, shift content */
+        memmove(output_buffer, output_buffer + OUTPUT_BUF_SIZE/2, OUTPUT_BUF_SIZE/2);
+        output_len = OUTPUT_BUF_SIZE/2;
+        space = OUTPUT_BUF_SIZE - output_len - 1;
+    }
+    
+    ssize_t n = read(fd, output_buffer + output_len, space);
+    if (n > 0) {
+        output_len += n;
+        output_buffer[output_len] = '\0';
+    }
+    return n;
+}
+
+/*
+ * Custom implementation of exp_spawnl using forkpty.
+ * This avoids the segfault bug in libexpect on Arch Linux.
+ */
 void
 prof_start(void)
 {
-    // Set timeout for expect operations (default is too short for CI)
-    exp_timeout = 30;
-
-    // helper script sets terminal columns, avoids assertions failing
-    // based on the test runner terminal size
-    fd = exp_spawnl("sh",
-        "sh",
-        "-c",
-        "./tests/functionaltests/start_profanity.sh",
-        NULL);
-    FILE *fp = fdopen(fd, "r+");
-
-    assert_true(fp != NULL);
-
-    setbuf(fp, (char *)0);
+    struct winsize ws;
+    ws.ws_row = 24;
+    ws.ws_col = 300;  /* Match COLUMNS=300 from start_profanity.sh */
+    ws.ws_xpixel = 0;
+    ws.ws_ypixel = 0;
+    
+    /* Reset output buffer */
+    output_len = 0;
+    output_buffer[0] = '\0';
+    
+    child_pid = forkpty(&fd, NULL, NULL, &ws);
+    
+    if (child_pid < 0) {
+        fd = -1;
+        return;
+    }
+    
+    if (child_pid == 0) {
+        /* Child process */
+        setenv("COLUMNS", "300", 1);
+        setenv("TERM", "xterm", 1);
+        
+        execl("./profanity", "./profanity", "-l", "DEBUG", NULL);
+        
+        /* If exec fails */
+        fprintf(stderr, "execl failed: %s\n", strerror(errno));
+        _exit(127);
+    }
+    
+    /* Parent process */
+    /* Set non-blocking mode for reading */
+    int flags = fcntl(fd, F_GETFL, 0);
+    fcntl(fd, F_SETFL, flags | O_NONBLOCK);
 }
 
 int
@@ -144,7 +215,8 @@ init_prof_test(void **state)
 {
     gboolean started = FALSE;
     for (int p = 5230; p < 5250; ++p) {
-        if (stbbr_start(STBBR_LOGDEBUG, p, 0) == 0) {
+        int ret = stbbr_start(STBBR_LOGDEBUG, p, 0);
+        if (ret == 0) {
             stub_port = p;
             started = TRUE;
             break;
@@ -172,7 +244,8 @@ init_prof_test(void **state)
     _create_logs_dir();
 
     prof_start();
-    assert_true(prof_output_regex("CProof\\. Type /help for help information\\."));
+    int prof_started = prof_output_regex("CProof\\. Type /help for help information\\.");
+    assert_true(prof_started);
 
     // set UI options to make expect assertions faster and more reliable
     prof_input("/inpblock timeout 5");
@@ -208,11 +281,14 @@ init_prof_test(void **state)
 int
 close_prof_test(void **state)
 {
-    if (fd > 0) {
+    if (fd > 0 && child_pid > 0) {
         prof_input("/quit");
         // Give profanity time to process quit command
         sleep(1);
-        waitpid(exp_pid, NULL, 0);
+        waitpid(child_pid, NULL, 0);
+        close(fd);
+        fd = 0;
+        child_pid = 0;
     }
     _cleanup_dirs();
 
@@ -237,18 +313,72 @@ prof_input(const char *input)
     ssize_t _wn = write(fd, inp_str->str, inp_str->len);
     (void)_wn;
     g_string_free(inp_str, TRUE);
+    
+    /* Small delay to let profanity process input */
+    usleep(10000);
 }
 
+/*
+ * Wait for exact text to appear in output.
+ * Returns 1 if found, 0 if timeout.
+ */
 int
 prof_output_exact(const char *text)
 {
-    return (1 == exp_expectl(fd, exp_exact, text, 1, exp_end));
+    time_t start = time(NULL);
+    
+    while (time(NULL) - start < expect_timeout) {
+        /* Read any available output */
+        while (_read_output(100) > 0) {
+            /* Keep reading while data available */
+        }
+        
+        /* Check if text is in buffer */
+        if (strstr(output_buffer, text) != NULL) {
+            return 1;
+        }
+        
+        usleep(50000);  /* 50ms */
+    }
+    
+    return 0;
 }
 
+/*
+ * Wait for regex pattern to match in output.
+ * Returns 1 if found, 0 if timeout.
+ */
 int
-prof_output_regex(const char *text)
+prof_output_regex(const char *pattern)
 {
-    return (1 == exp_expectl(fd, exp_regexp, text, 1, exp_end));
+    regex_t regex;
+    int ret;
+    
+    ret = regcomp(&regex, pattern, REG_EXTENDED | REG_NOSUB);
+    if (ret != 0) {
+        return 0;
+    }
+    
+    time_t start = time(NULL);
+    
+    while (time(NULL) - start < expect_timeout) {
+        /* Read any available output */
+        while (_read_output(100) > 0) {
+            /* Keep reading while data available */
+        }
+        
+        /* Check if pattern matches */
+        ret = regexec(&regex, output_buffer, 0, NULL, 0);
+        if (ret == 0) {
+            regfree(&regex);
+            return 1;
+        }
+        
+        usleep(50000);  /* 50ms */
+    }
+    
+    regfree(&regex);
+    return 0;
 }
 
 void
@@ -276,12 +406,12 @@ prof_connect_with_roster(const char *roster)
     assert_true(prof_output_regex("password:"));
     prof_input("password");
 
-    exp_timeout = 60;
+    expect_timeout = 60;
     assert_true(prof_output_regex("Connecting as stabber@localhost"));
     assert_true(prof_output_regex("logged in successfully"));
     assert_true(prof_output_regex(".+online.+ \\(priority 0\\)\\."));
     
-    exp_timeout = 60;
+    expect_timeout = 60;
     // Wait for presence stanza to be sent (content-based, not ID-based)
     // Match the actual attribute order from stanza_attach_caps
     assert_true(stbbr_received(
@@ -294,13 +424,13 @@ prof_connect_with_roster(const char *roster)
 void
 prof_timeout(int timeout)
 {
-    exp_timeout = timeout;
+    expect_timeout = timeout;
 }
 
 void
 prof_timeout_reset(void)
 {
-    exp_timeout = 60;
+    expect_timeout = 60;
 }
 
 void

tests/functionaltests/test_carbons.c

@@ -4,7 +4,6 @@
 #include <string.h>
 
 #include <stabber.h>
-#include <expect.h>
 
 #include "proftest.h"
 

tests/functionaltests/test_chat_session.c

@@ -5,7 +5,6 @@
 #include <string.h>
 
 #include <stabber.h>
-#include <expect.h>
 
 #include "proftest.h"
 

tests/functionaltests/test_connect.c

@@ -2,9 +2,9 @@
 #include "prof_cmocka.h"
 #include <stdlib.h>
 #include <string.h>
+#include <stdio.h>
 
 #include <stabber.h>
-#include <expect.h>
 
 #include "proftest.h"
 

tests/functionaltests/test_disconnect.c

@@ -4,7 +4,6 @@
 #include <string.h>
 
 #include <stabber.h>
-#include <expect.h>
 
 #include "proftest.h"
 

tests/functionaltests/test_message.c

@@ -4,7 +4,6 @@
 #include <string.h>
 
 #include <stabber.h>
-#include <expect.h>
 
 #include "proftest.h"
 

tests/functionaltests/test_muc.c

@@ -5,7 +5,6 @@
 #include <string.h>
 
 #include <stabber.h>
-#include <expect.h>
 
 #include "proftest.h"
 

tests/functionaltests/test_ping.c

@@ -5,7 +5,6 @@
 #include <unistd.h>
 
 #include <stabber.h>
-#include <expect.h>
 
 #include "proftest.h"
 

tests/functionaltests/test_presence.c

@@ -4,7 +4,6 @@
 #include <string.h>
 
 #include <stabber.h>
-#include <expect.h>
 
 #include "proftest.h"
 

tests/functionaltests/test_receipts.c

@@ -4,7 +4,6 @@
 #include <string.h>
 
 #include <stabber.h>
-#include <expect.h>
 
 #include "proftest.h"
 

tests/functionaltests/test_rooms.c

@@ -4,7 +4,6 @@
 #include <string.h>
 
 #include <stabber.h>
-#include <expect.h>
 
 #include "proftest.h"
 

tests/functionaltests/test_roster.c

@@ -4,7 +4,6 @@
 #include <string.h>
 
 #include <stabber.h>
-#include <expect.h>
 
 #include "proftest.h"
 

tests/functionaltests/test_software.c

@@ -4,7 +4,6 @@
 #include <string.h>
 
 #include <stabber.h>
-#include <expect.h>
 
 #include "proftest.h"