fix: add build names, use TEST_BUILDS constant, redirect errors to stderr

- Add TEST_GROUPS constant to eliminate magic number
- Use stdout for info messages, stderr for errors only
- Remove redundant comments and dead code
- Add CI check before copying coverage.info
- Unify variable declaration order
- Add port allocation explanation comment
- Restore /* 50ms */ inline comment
- Update CONTRIBUTING.md with build 1 description

.github/workflows/ci-code.yml

@@ -106,20 +106,5 @@ jobs:
       - uses: actions/checkout@v4
       - name: Build and run coverage
         run: |
-          docker build -f Dockerfile.debian -t profanity-cov .
-          docker run -v ${{ github.workspace }}/coverage:/coverage profanity-cov bash -c '
-            ./bootstrap.sh
-            ./configure --enable-coverage --enable-otr --enable-pgp --enable-omemo --enable-plugins
-            make -j$(nproc)
-            make check || true
-            make check-functional-parallel || true
-            lcov --capture --directory . --output-file /coverage/coverage.info \
-              --rc branch_coverage=1 \
-              --ignore-errors inconsistent
-            lcov --remove /coverage/coverage.info \
-              "/usr/include/*" "*/tests/*" \
-              --output-file /coverage/coverage.info \
-              --rc branch_coverage=1 \
-              --ignore-errors inconsistent,empty,unused
-            lcov --summary /coverage/coverage.info
-          '
+          docker build -f Dockerfile.arch -t profanity-cov .
+          docker run profanity-cov ./ci-build.sh --coverage-only

CONTRIBUTING.md

@@ -90,13 +90,22 @@ set -e
 ```
 
 This will run the same tests that the CI runs and refuse the push if it fails.
-The CI script runs 4 parallel builds with different configurations, each with Valgrind and functional tests.
-Build 1 also collects code coverage (lines, functions, branches).
+The CI script runs 4 parallel builds with different configurations:
+- **Full** — all features enabled (+ coverage in `--coverage-only` mode)
+- **Minimal** — all optional features disabled
+- **NoEncrypt** — no encryption (OTR, PGP, OMEMO disabled)
+- **Default** — default ./configure options
+
+Each build runs Valgrind and functional tests on Linux.
+Use `./ci-build.sh --coverage-only` to run only the Full build with coverage collection.
 
 Output shows test results per build:
 ```
-✓ Build 1 passed (unit: 437/0, func: 69/0, cov: L:27.5% F:36.2% B:18.1%, 5m39s)
-✓ Build 2 passed (unit: 398/0, func: 69/0, 5m00s)
+✓ Full PASSED
+  Unit tests:       437 passed, 0 failed
+  Functional tests: 69 passed, 0 failed
+  Coverage:         Lines: 27.5% | Functions: 36.2% | Branches: 18.1%
+  Duration:         5m39s
 ```
 
 Note that it will run on the actual content of the repository directory and not

ci-build.sh

@@ -15,14 +15,72 @@ error_handler()
     log_content ./test-suite.log
     log_content ./test-suite-memcheck.log
 
-    echo
-    echo "Error ${ERR_CODE} with command '${BASH_COMMAND}' on line ${BASH_LINENO[0]}. Exiting."
-    echo
+    echo >&2
+    echo "Error ${ERR_CODE} with command '${BASH_COMMAND}' on line ${BASH_LINENO[0]}. Exiting." >&2
+    echo >&2
     exit ${ERR_CODE}
 }
 
 trap error_handler ERR
 
+# =============================================================================
+# Constants
+# =============================================================================
+
+# Number of parallel build configurations
+readonly TEST_BUILDS=4
+
+# Human-readable names for each build configuration
+readonly BUILD_NAMES=(
+    "Full"      # 1. All features enabled
+    "Minimal"   # 2. All optional features disabled
+    "NoEncrypt" # 3. No encryption (otr, pgp, omemo disabled)
+    "Default"   # 4. Default ./configure options
+)
+
+# Regex patterns for parsing test output
+readonly CMOCKA_PASSED_PATTERN='^\[  PASSED  \] [0-9]+ test'
+readonly CMOCKA_FAILED_PATTERN='^\[  FAILED  \] [0-9]+ test'
+
+# Coverage extraction patterns (matches both Docker and CI paths)
+readonly COVERAGE_PATTERNS='*/profanity/src/* */src/src/*'
+
+# =============================================================================
+# Helper Functions
+# =============================================================================
+
+# Parse STATS line from build log and set global variables
+# Usage: parse_build_stats <log_file>
+parse_build_stats() {
+    local log_file="$1"
+    local stats_line
+    stats_line=$(grep "^STATS:" "$log_file" 2>/dev/null | tail -1)
+    
+    STAT_UNIT_P=$(echo "$stats_line" | grep -oE "unit_passed=[0-9]+" | cut -d= -f2)
+    STAT_UNIT_F=$(echo "$stats_line" | grep -oE "unit_failed=[0-9]+" | cut -d= -f2)
+    STAT_FUNC_P=$(echo "$stats_line" | grep -oE "func_passed=[0-9]+" | cut -d= -f2)
+    STAT_FUNC_F=$(echo "$stats_line" | grep -oE "func_failed=[0-9]+" | cut -d= -f2)
+    STAT_COV_LINES=$(echo "$stats_line" | grep -oE "cov_lines=[0-9.]+%|cov_lines=n/a" | cut -d= -f2)
+    STAT_COV_FUNCS=$(echo "$stats_line" | grep -oE "cov_funcs=[0-9.]+%|cov_funcs=n/a" | cut -d= -f2)
+    STAT_COV_BRANCHES=$(echo "$stats_line" | grep -oE "cov_branches=[0-9.]+%|cov_branches=n/a" | cut -d= -f2)
+    STAT_TIME=$(echo "$stats_line" | grep -oE "time=[0-9]+m[0-9]+s" | cut -d= -f2)
+    
+    : "${STAT_UNIT_P:=0}"
+    : "${STAT_UNIT_F:=0}"
+    : "${STAT_FUNC_P:=0}"
+    : "${STAT_FUNC_F:=0}"
+}
+
+# Extract test count from log file
+# Usage: extract_test_count <log_file> <pattern>
+extract_test_count() {
+    grep -E "$2" "$1" 2>/dev/null | grep -oE "[0-9]+" | head -1
+}
+
+# =============================================================================
+# Test Verification
+# =============================================================================
+
 # Verify that test failures are properly detected
 # This is a meta-test: it runs a deliberately failing test
 # and checks that the test framework reports the failure correctly
@@ -61,11 +119,11 @@ EOF
     # Test 1: Single failing test detection
     echo "  Testing single test failure detection..."
     if /tmp/test_must_fail > /tmp/test_must_fail.log 2>&1; then
-        echo "ERROR: Test that should fail returned success (exit code 0)"
-        echo "This means the test framework is NOT detecting failures correctly!"
-        echo "--- Test output ---"
-        cat /tmp/test_must_fail.log
-        echo "--- End output ---"
+        echo "ERROR: Test that should fail returned success (exit code 0)" >&2
+        echo "This means the test framework is NOT detecting failures correctly!" >&2
+        echo "--- Test output ---" >&2
+        cat /tmp/test_must_fail.log >&2
+        echo "--- End output ---" >&2
         rm -f /tmp/test_must_fail /tmp/test_must_fail.c /tmp/test_must_fail.log
         exit 1
     fi
@@ -85,8 +143,8 @@ EOF
     wait $pid4 || failed=$((failed + 1))
     
     if [ $failed -ne 2 ]; then
-        echo "ERROR: Expected 2 failures in parallel tests, got $failed"
-        echo "Parallel failure detection is broken!"
+        echo "ERROR: Expected 2 failures in parallel tests, got $failed" >&2
+        echo "Parallel failure detection is broken!" >&2
         rm -f /tmp/test_must_fail /tmp/test_must_fail.c /tmp/test_must_fail.log /tmp/p?.log
         exit 1
     fi
@@ -110,6 +168,17 @@ num_cores()
 # Run test failure detection verification first
 verify_test_failure_detection
 
+# Parse arguments
+COVERAGE_ONLY=no
+for arg in "$@"; do
+    case "$arg" in
+        --coverage-only)
+            COVERAGE_ONLY=yes
+            shift
+            ;;
+    esac
+done
+
 ./bootstrap.sh
 
 tests=()
@@ -210,7 +279,7 @@ build_and_test() {
             exit 1
         fi
 
-        # Run unit tests under Valgrind
+        # Run unit tests
         local unit_passed=0 unit_failed=0
         if [ "$run_valgrind" = "yes" ]; then
             echo "--> Running unit tests under Valgrind..."
@@ -225,13 +294,20 @@ build_and_test() {
                 echo "ERROR: Valgrind unit tests failed (exit code $valgrind_exit)"
                 exit 1
             fi
-            # Extract unit test counts from cmocka output
-            unit_passed=$(grep -E "^\[  PASSED  \] [0-9]+ test" unit-tests-output.log | grep -oE "[0-9]+" | head -1)
-            unit_failed=$(grep -E "^\[  FAILED  \] [0-9]+ test" unit-tests-output.log | grep -oE "[0-9]+" | head -1)
-            [ -z "$unit_passed" ] && unit_passed=0
-            [ -z "$unit_failed" ] && unit_failed=0
-            echo "UNIT_TESTS: passed=$unit_passed failed=$unit_failed"
+        else
+            echo "--> Running unit tests..."
+            $MAKE tests/unittests/unittests
+            tests/unittests/unittests 2>&1 | tee unit-tests-output.log
+            if [ ${PIPESTATUS[0]} -ne 0 ]; then
+                echo "ERROR: Unit tests failed"
+                exit 1
+            fi
         fi
+        # Extract unit test counts from cmocka output
+        unit_passed=$(extract_test_count unit-tests-output.log "$CMOCKA_PASSED_PATTERN")
+        unit_failed=$(extract_test_count unit-tests-output.log "$CMOCKA_FAILED_PATTERN")
+        : "${unit_passed:=0}" "${unit_failed:=0}"
+        echo "UNIT_TESTS: passed=$unit_passed failed=$unit_failed"
 
         # Set build index for port allocation: build 1 uses ports 5230-5233,
         # build 2 uses 5234-5237, etc. This prevents port conflicts in parallel builds.
@@ -245,9 +321,9 @@ build_and_test() {
         echo "=== Functional test results ==="
         for glog in ./test-logs/group*.log; do
             if [ -f "$glog" ]; then
-                cnt=$(grep -E "^\[  PASSED  \] [0-9]+ test" "$glog" | grep -oE "[0-9]+" | head -1)
+                cnt=$(extract_test_count "$glog" "$CMOCKA_PASSED_PATTERN")
                 [ -n "$cnt" ] && func_passed=$((func_passed + cnt))
-                cnt=$(grep -E "^\[  FAILED  \] [0-9]+ test" "$glog" | grep -oE "[0-9]+" | head -1)
+                cnt=$(extract_test_count "$glog" "$CMOCKA_FAILED_PATTERN")
                 [ -n "$cnt" ] && func_failed=$((func_failed + cnt))
             fi
         done
@@ -262,8 +338,8 @@ build_and_test() {
                 lcov --capture --directory . --output-file coverage-full.info \
                     --rc lcov_branch_coverage=1 --ignore-errors inconsistent 2>&1 || true
                 # Extract only production code from src/ directory, exclude tests
-                # Pattern matches both Docker (/src/src/*) and CI (/usr/src/profanity/src/*)
-                lcov --extract coverage-full.info '*/profanity/src/*' '*/src/src/*' \
+                # shellcheck disable=SC2086
+                lcov --extract coverage-full.info $COVERAGE_PATTERNS \
                     --output-file coverage.info \
                     --rc lcov_branch_coverage=1 --ignore-errors inconsistent 2>&1 || true
                 if [ -f coverage.info ] && [ -s coverage.info ]; then
@@ -286,6 +362,14 @@ build_and_test() {
         fi
 
         ./profanity -v
+
+        # Save coverage.info before cleanup (for CI artifact)
+        # Only copy in CI environment to avoid leaving artifacts during local runs
+        if [ "$run_coverage" = "yes" ] && [ -f coverage.info ] && [ -n "$CI" ]; then
+            cp coverage.info ../coverage.info
+            echo "Coverage report saved to coverage.info"
+        fi
+
         $MAKE clean
 
         cd ..
@@ -300,40 +384,44 @@ build_and_test() {
     } > "$log_file" 2>&1
 }
 
-# Run all 4 configurations in parallel
+# Run configurations
 # Coverage enabled only for build 1 (Full) - it has most code paths
 echo
 echo "=== Start build ==="
 echo
-echo "Starting 4 parallel build configurations..."
-echo
-pids=()
-for idx in 1 2 3 4; do
-    if [ $idx -le ${#tests[@]} ]; then
-        # All builds run Valgrind
-        if [ "$ARCH" = "linux" ]; then
-            run_valgrind="yes"
-            if [ $idx -eq 1 ]; then
-                extra_flags="--enable-valgrind --enable-coverage"
-                run_coverage="yes"
-            else
+
+if [ "$COVERAGE_ONLY" = "yes" ]; then
+    echo "Running coverage-only mode (${BUILD_NAMES[0]} build)..."
+    echo
+    run_valgrind="no"
+    run_coverage="yes"
+    extra_flags="--enable-coverage"
+    build_and_test "${tests[0]}" "$* $extra_flags" "1" "$run_valgrind" "$run_coverage" &
+    pids=("$!")
+    echo "  → ${BUILD_NAMES[0]}: ${tests[0]} [+Coverage]"
+else
+    echo "Starting $TEST_BUILDS parallel build configurations..."
+    echo
+    pids=()
+    for idx in $(seq 1 $TEST_BUILDS); do
+        if [ $idx -le ${#tests[@]} ]; then
+            # All builds run Valgrind on Linux
+            if [ "$ARCH" = "linux" ]; then
+                run_valgrind="yes"
                 extra_flags="--enable-valgrind"
-                run_coverage="no"
+            else
+                run_valgrind="no"
+                extra_flags=""
             fi
-        else
-            extra_flags=""
-            run_valgrind="no"
             run_coverage="no"
+            build_and_test "${tests[$((idx-1))]}" "$* $extra_flags" "$idx" "$run_valgrind" "$run_coverage" &
+            pids+=("$!")
+            flags_desc=""
+            [ "$run_valgrind" = "yes" ] && flags_desc=" [+Valgrind]"
+            echo "  → ${BUILD_NAMES[$((idx-1))]}: ${tests[$((idx-1))]}$flags_desc"
         fi
-        build_and_test "${tests[$((idx-1))]}" "$* $extra_flags" "$idx" "$run_valgrind" "$run_coverage" &
-        pids+=("$!")
-        flags_desc=""
-        [ "$run_valgrind" = "yes" ] && flags_desc="Valgrind"
-        [ "$run_coverage" = "yes" ] && flags_desc="${flags_desc:+$flags_desc+}Coverage"
-        [ -n "$flags_desc" ] && flags_desc=" [+$flags_desc]"
-        echo "  → Build $idx: ${tests[$((idx-1))]}$flags_desc"
-    fi
-done
+    done
+fi
 echo
 
 # Wait for all builds and check exit codes
@@ -344,33 +432,20 @@ for i in "${!pids[@]}"; do
     idx=$((i + 1))
     if wait "${pids[$i]}"; then
         if [ -f "build-$idx.log" ]; then
-            stats_line=$(grep "^STATS:" "build-$idx.log" | tail -1)
-            unit_p=$(echo "$stats_line" | grep -oE "unit_passed=[0-9]+" | cut -d= -f2)
-            unit_f=$(echo "$stats_line" | grep -oE "unit_failed=[0-9]+" | cut -d= -f2)
-            func_p=$(echo "$stats_line" | grep -oE "func_passed=[0-9]+" | cut -d= -f2)
-            func_f=$(echo "$stats_line" | grep -oE "func_failed=[0-9]+" | cut -d= -f2)
-            cov_lines=$(echo "$stats_line" | grep -oE "cov_lines=[0-9.]+%|cov_lines=n/a" | cut -d= -f2)
-            cov_funcs=$(echo "$stats_line" | grep -oE "cov_funcs=[0-9.]+%|cov_funcs=n/a" | cut -d= -f2)
-            cov_branches=$(echo "$stats_line" | grep -oE "cov_branches=[0-9.]+%|cov_branches=n/a" | cut -d= -f2)
-            time=$(echo "$stats_line" | grep -oE "time=[0-9]+m[0-9]+s" | cut -d= -f2)
+            parse_build_stats "build-$idx.log"
             
-            unit_p=${unit_p:-0}
-            unit_f=${unit_f:-0}
-            func_p=${func_p:-0}
-            func_f=${func_f:-0}
-            
-            echo "✓ BUILD $idx PASSED"
-            echo "  Unit tests:       $unit_p passed, $unit_f failed"
-            echo "  Functional tests: $func_p passed, $func_f failed"
-            if [ "$cov_lines" != "n/a" ] && [ -n "$cov_lines" ]; then
-                echo "  Coverage:         Lines: $cov_lines | Functions: $cov_funcs | Branches: $cov_branches"
+            echo "✓ ${BUILD_NAMES[$i]} PASSED"
+            echo "  Unit tests:       $STAT_UNIT_P passed, $STAT_UNIT_F failed"
+            echo "  Functional tests: $STAT_FUNC_P passed, $STAT_FUNC_F failed"
+            if [ "$STAT_COV_LINES" != "n/a" ] && [ -n "$STAT_COV_LINES" ]; then
+                echo "  Coverage:         Lines: $STAT_COV_LINES | Functions: $STAT_COV_FUNCS | Branches: $STAT_COV_BRANCHES"
             fi
-            echo "  Duration:         ${time:-?}"
+            echo "  Duration:         ${STAT_TIME:-?}"
         else
-            echo "✓ Build $idx passed (no stats available)"
+            echo "✓ ${BUILD_NAMES[$i]} passed (no stats available)"
         fi
     else
-        echo "✗ BUILD $idx FAILED"
+        echo "✗ ${BUILD_NAMES[$i]} FAILED" >&2
         failed_builds+=("$idx")
     fi
     echo
@@ -379,15 +454,19 @@ done
 # Show failed builds full logs
 for idx in "${failed_builds[@]}"; do
     if [ -f "build-$idx.log" ]; then
-        echo "=== BUILD $idx FAILURE LOG ==="
-        cat "build-$idx.log"
-        echo
+        echo "=== ${BUILD_NAMES[$((idx-1))]} FAILURE LOG ===" >&2
+        cat "build-$idx.log" >&2
+        echo >&2
     fi
 done
 
 if [ ${#failed_builds[@]} -gt 0 ]; then
-    echo "RESULT: FAILED (builds ${failed_builds[*]})"
+    echo "RESULT: FAILED (builds ${failed_builds[*]})" >&2
     exit 1
 else
-    echo "RESULT: ALL 4 BUILDS PASSED ✓"
+    if [ "$COVERAGE_ONLY" = "yes" ]; then
+        echo "RESULT: COVERAGE BUILD PASSED ✓"
+    else
+        echo "RESULT: ALL $TEST_BUILDS BUILDS PASSED ✓"
+    fi
 fi

tests/functionaltests/proftest.c

@@ -17,6 +17,9 @@
 
 #include "proftest.h"
 
+/* Number of parallel test groups for CI builds */
+#define TEST_GROUPS 4
+
 char *config_orig;
 char *data_orig;
 
@@ -138,9 +141,9 @@ _cleanup_dirs(void)
 {
     const char *group_env = getenv("PROF_TEST_GROUP");
     int group = group_env ? atoi(group_env) : 0;
-    int dir_id = (group >= 1 && group <= 4) ? group : stub_port;
+    int dir_id = (group >= 1 && group <= TEST_GROUPS) ? group : stub_port;
     
-    fprintf(stderr, "[PROF_TEST] Cleaning up directories for group %d (dir_id %d)\n", group, dir_id);
+    printf("[PROF_TEST] Cleaning up directories for group %d (dir_id %d)\n", group, dir_id);
     
     char cmd[512];
     snprintf(cmd, sizeof(cmd), "rm -rf ./test-files/%d", dir_id);
@@ -243,36 +246,38 @@ init_prof_test(void **state)
     const char *build_env = getenv("PROF_BUILD_INDEX");
     int build_idx = build_env ? atoi(build_env) : 0;
     
-    /* Calculate port base: each build uses a different range of 4 ports
-     * Build 0/1: 5230-5233, Build 2: 5234-5237, Build 3: 5238-5241, Build 4: 5242-5245 */
-    int port_base = 5230 + ((build_idx > 0 ? build_idx - 1 : 0) * 4);
+    /* Calculate port base: each build uses a different range of TEST_GROUPS ports.
+     * Build 0 (local/default): 5230-5233, Full: 5230-5233, Minimal: 5234-5237, etc.
+     * Build 0 and Full share the same range because build 0 is for local runs or sequential run (no parallel builds),
+     * while Full/Minimal/NoEncrypt/Default are used in CI where they run in parallel. */
+    int port_base = 5230 + ((build_idx > 0 ? build_idx - 1 : 0) * TEST_GROUPS);
     
     /* Static resource allocation to avoid conflicts in parallel execution.
      * Group 1-4: use static port assignment.
      * Group 0 (all groups): use dynamic allocation as fallback. */
     gboolean started = FALSE;
     
-    if (group >= 1 && group <= 4) {
+    if (group >= 1 && group <= TEST_GROUPS) {
         /* Static allocation: each group gets a dedicated port */
         stub_port = port_base + group - 1;
-        fprintf(stderr, "[PROF_TEST] Build %d, Group %d: trying port %d\n", build_idx, group, stub_port);
+        printf("[PROF_TEST] Build %d, Group %d: trying port %d\n", build_idx, group, stub_port);
         
         if (stbbr_start(STBBR_LOGDEBUG, stub_port, 0) == 0) {
             started = TRUE;
-            fprintf(stderr, "[PROF_TEST] Started stabber on port %d\n", stub_port);
+            printf("[PROF_TEST] Started stabber on port %d\n", stub_port);
         } else {
-            fprintf(stderr, "[PROF_TEST] Failed to start stabber on port %d\n", stub_port);
+            printf("[PROF_TEST] Failed to start stabber on port %d\n", stub_port);
         }
     }
     
     /* Fallback to dynamic allocation if static failed or group=0 */
     if (!started) {
-        fprintf(stderr, "[PROF_TEST] Using dynamic port allocation\n");
+        printf("[PROF_TEST] Using dynamic port allocation\n");
         for (int p = port_base; p < port_base + 20; ++p) {
             if (stbbr_start(STBBR_LOGDEBUG, p, 0) == 0) {
                 stub_port = p;
                 started = TRUE;
-                fprintf(stderr, "[PROF_TEST] Started stabber on port %d\n", stub_port);
+                printf("[PROF_TEST] Started stabber on port %d\n", stub_port);
                 break;
             }
         }
@@ -280,20 +285,19 @@ init_prof_test(void **state)
     
     if (!started) {
         fprintf(stderr, "[PROF_TEST] ERROR: could not start stabber on any port\n");
-        assert_true(FALSE);
         return -1;
     }
 
-    // Generate unique XDG paths based on group for parallel execution
-    // Use ./test-files/ in current (build) directory for out-of-tree builds compatibility
-    int dir_id = (group >= 1 && group <= 4) ? group : stub_port;
+    /* Generate unique XDG paths based on group for parallel execution.
+     * Use ./test-files/ in current (build) directory for out-of-tree builds compatibility. */
+    int dir_id = (group >= 1 && group <= TEST_GROUPS) ? group : stub_port;
     snprintf(xdg_config_home, sizeof(xdg_config_home),
              "./test-files/%d/xdg_config_home", dir_id);
     snprintf(xdg_data_home, sizeof(xdg_data_home),
              "./test-files/%d/xdg_data_home", dir_id);
     
-    fprintf(stderr, "[PROF_TEST] Group %d using directories: config=%s, data=%s\n", 
-            group, xdg_config_home, xdg_data_home);
+    printf("[PROF_TEST] Group %d using directories: config=%s, data=%s\n", 
+           group, xdg_config_home, xdg_data_home);
 
     // Give stabber server thread time to start listening
     usleep(100000); // 100ms
@@ -447,7 +451,7 @@ prof_output_regex(const char *pattern)
             return 1;
         }
         
-        usleep(50000);
+        usleep(50000);  /* 50ms */
     }
     
     /* Timeout reached - log diagnostic info */