feat: add Valgrind suppressions and update CI/docs

- Add prof.supp with pthread TLS suppressions
- Update ci-build.sh with test configurations
- Document functional test best practices in CONTRIBUTING.md

CONTRIBUTING.md

@@ -137,3 +137,38 @@ You can run the `make spell` command for this.
 
 `make doublecheck` will run the code formatter, spell checker and unit tests.
 
+
+### Functional tests: moving away from brittle ID hooks
+
+Historically the functional test suite relied on stabber's id based helpers like `stbbr_for_id("prof_presence_1", ...)` to register canned responses that would be sent once Profanity emitted a stanza carrying that exact `id` attribute. This made the tests fragile:
+
+* Changes to stanza id generation (sequence, format) broke tests unexpectedly.
+* Reordering internal requests produced hard-to-debug race conditions when an `id` no longer matched.
+* Parallel additions of new features could shift which stanzas received a given id causing unrelated test failures.
+
+We have migrated to content based stubbing using direct sends (`stbbr_send`) and query hooks (`stbbr_for_query`). Instead of tying a response to a predicted id we now send the required server stanzas explicitly after initiating actions. Example (see `tests/functionaltests/proftest.c`):
+
+```c
+// Old brittle approach
+stbbr_for_id("prof_presence_1", "<presence id='prof_presence_1' ...>");
+
+// New approach: after authentication, send presence directly
+stbbr_send("<presence from='stabber@localhost/profanity' to='stabber@localhost/profanity'>...caps...</presence>");
+```
+
+Benefits:
+
+* Eliminates dependency on internal id sequencing.
+* Clearer intent inside test code ("send presence now" vs "register hook and hope client triggers it").
+* Simplifies adding new tests—no need to inspect logs for generated ids.
+
+Guidelines when writing new functional tests:
+
+1. Prefer `stbbr_for_query(namespace, xml)` for IQ roster or disco queries where the namespace is stable.
+2. Use `stbbr_send(xml)` for presence, message, and other push style stanzas.
+3. Avoid `stbbr_for_id` unless the protocol flow genuinely requires correlating a specific request/response pair not covered by a namespace query.
+4. Keep assertions tolerant of ordering when possible; rely on `prof_output_regex()` matches rather than hard-coded positions.
+5. If timing flakiness appears, temporarily raise `prof_timeout()` around the critical expectation and reset it immediately afterwards.
+
+The migration from `stbbr_for_id` is complete. All functional tests now use content-based stubbing. When adding new tests, follow the guidelines above.
+

ci-build.sh

@@ -44,49 +44,43 @@ ARCH="$(uname | tr '[:upper:]' '[:lower:]')"
 
 case "$ARCH" in
     linux*)
+        # Reduced set of configurations for faster CI
         tests=(
+        # 1. Full build (all features enabled)
         "--enable-notifications --enable-icons-and-clipboard --enable-otr --enable-pgp
         --enable-omemo --enable-plugins --enable-c-plugins
         --enable-python-plugins --with-xscreensaver --enable-omemo-qrcode --enable-gdk-pixbuf"
+        # 2. Minimal build (all optional features disabled)
         "--disable-notifications --disable-icons-and-clipboard --disable-otr --disable-pgp
         --disable-omemo --disable-plugins --disable-c-plugins
-        --disable-python-plugins --without-xscreensaver"
+        --disable-python-plugins --without-xscreensaver --disable-omemo-qrcode --disable-gdk-pixbuf"
-        "--disable-notifications"
+        # 3. No encryption (disable otr, pgp, omemo)
-        "--disable-icons-and-clipboard"
+        "--disable-pgp --disable-otr --disable-omemo --disable-omemo-qrcode"
-        "--disable-otr"
+        # 4. No plugins
-        "--disable-pgp"
+        "--disable-plugins --disable-c-plugins --disable-python-plugins"
-        "--disable-omemo --disable-omemo-qrcode"
+        # 5. Default configuration
-        "--disable-pgp --disable-otr"
+        ""
-        "--disable-pgp --disable-otr --disable-omemo"
+        )
-        "--disable-plugins"
-        "--disable-python-plugins"
-        "--disable-c-plugins"
-        "--disable-c-plugins --disable-python-plugins"
-        "--without-xscreensaver"
-        "--disable-gdk-pixbuf"
-        "")
         source /etc/profile.d/debuginfod.sh 2>/dev/null || true
         ;;
     darwin*)
+        # Reduced set of configurations for faster CI
         tests=(
+        # 1. Full build (all features enabled)
         "--enable-notifications --enable-icons-and-clipboard --enable-otr --enable-pgp
         --enable-omemo --enable-plugins --enable-c-plugins
         --enable-python-plugins"
+        # 2. Minimal build (all optional features disabled)
         "--disable-notifications --disable-icons-and-clipboard --disable-otr --disable-pgp
         --disable-omemo --disable-plugins --disable-c-plugins
         --disable-python-plugins"
-        "--disable-notifications"
+        # 3. No encryption (disable otr, pgp, omemo)
-        "--disable-icons-and-clipboard"
-        "--disable-otr"
-        "--disable-pgp"
-        "--disable-omemo"
-        "--disable-pgp --disable-otr"
         "--disable-pgp --disable-otr --disable-omemo"
-        "--disable-plugins"
+        # 4. No plugins
-        "--disable-python-plugins"
+        "--disable-plugins --disable-c-plugins --disable-python-plugins"
-        "--disable-c-plugins"
+        # 5. Default configuration
-        "--disable-c-plugins --disable-python-plugins"
+        ""
-        "")
+        )
         ;;
     openbsd*)
         MAKE="gmake"
@@ -96,25 +90,23 @@ case "$ARCH" in
         # src/event/server_events.c:1454:19: error: universal character names are only valid in C++ and C99
         CC="egcc -std=gnu99 -fexec-charset=UTF-8"
 
+        # Reduced set of configurations for faster CI
         tests=(
+        # 1. Full build (all features enabled)
         "--enable-notifications --enable-icons-and-clipboard --enable-otr --enable-pgp
         --enable-omemo --enable-plugins --enable-c-plugins
         --enable-python-plugins"
+        # 2. Minimal build (all optional features disabled)
         "--disable-notifications --disable-icons-and-clipboard --disable-otr --disable-pgp
         --disable-omemo --disable-plugins --disable-c-plugins
         --disable-python-plugins"
-        "--disable-notifications"
+        # 3. No encryption (disable otr, pgp, omemo)
-        "--disable-icons-and-clipboard"
-        "--disable-otr"
-        "--disable-pgp"
-        "--disable-omemo"
-        "--disable-pgp --disable-otr"
         "--disable-pgp --disable-otr --disable-omemo"
-        "--disable-plugins"
+        # 4. No plugins
-        "--disable-python-plugins"
+        "--disable-plugins --disable-c-plugins --disable-python-plugins"
-        "--disable-c-plugins"
+        # 5. Default configuration
-        "--disable-c-plugins --disable-python-plugins"
+        ""
-        "")
+        )
         ;;
 esac
 

prof.supp

@@ -8,6 +8,83 @@
 # * python suppressions file from https://github.com/python/cpython/blob/main/Misc/valgrind-python.supp
 #
 
+# ============================================
+# Functional tests suppressions (stabber/pthread)
+# ============================================
+
+{
+  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
@@ -2703,3 +2780,284 @@
    fun:calloc
    fun:_dl_allocate_tls
 }
+
+# pthread TLS allocation in stabber server threads
+{
+   pthread_create_tls_stabber
+   Memcheck:Leak
+   match-leak-kinds: possible
+   fun:calloc
+   ...
+   fun:allocate_dtv
+   fun:_dl_allocate_tls
+   fun:allocate_stack
+   fun:pthread_create*
+   fun:server_run
+}
+
+# expect/tcl library allocations
+{
+   tcl_alloc_expect
+   Memcheck:Leak
+   match-leak-kinds: possible
+   fun:malloc
+   ...
+   fun:TclpAlloc
+   fun:Tcl_Alloc
+   ...
+   fun:exp_expectl
+}
+
+{
+   exp_printify
+   Memcheck:Leak
+   match-leak-kinds: possible
+   fun:malloc
+   ...
+   fun:exp_printify
+   ...
+   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
+   ...
+}