Improve Functional Test Stability and CI Reliability #79

Manually merged
jabber.developer merged 7 commits from fix/test-CI-stability into master 2026-02-02 16:47:30 +00:00
8 changed files with 395 additions and 90 deletions

View File

@@ -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

6
.gitignore vendored
View File

@@ -62,6 +62,8 @@ tests/unittests/unittests
tests/unittests/unittests.log
tests/unittests/unittests.trs
test-suite.log
test-files/
test-logs/
# valgrind output
profval*
@@ -108,3 +110,7 @@ breaks
*.zip
*.log*
coverage/
*.gcno
*.gcda
*.gcov
coverage.info

View File

@@ -90,6 +90,24 @@ 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:
- **Full** — all features enabled (+ coverage in `--coverage-only` mode)
jabber.developer marked this conversation as resolved Outdated

Could you clarify that "build 1" is a standard build without flags?

Could you clarify that "build 1" is a standard build without flags?

Corrected

Corrected
- **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:
```
✓ 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
what may have been staged/committed.

View File

@@ -313,7 +313,7 @@ FUNC_TEST_GROUPS = 1 2 3 4
check-functional-parallel: tests/functionaltests/functionaltests
@echo "Running functional tests in parallel ($(words $(FUNC_TEST_GROUPS)) groups)..."
@mkdir -p $(builddir)/test-logs
@mkdir -p $(builddir)/test-logs $(builddir)/test-files
@pids=""; \
for g in $(FUNC_TEST_GROUPS); do \
./tests/functionaltests/functionaltests $$g > $(builddir)/test-logs/group$$g.log 2>&1 & \
@@ -372,7 +372,10 @@ clean-local:
rm -f $(git_include) $(git_include).in
endif
.PHONY: my-prof.supp
clean-functional-tests:
rm -rf $(builddir)/test-files $(builddir)/test-logs
.PHONY: my-prof.supp clean-functional-tests
my-prof.supp:
@sed '/^# AUTO-GENERATED START/q' prof.supp > $@
@printf "\n\n# glib\n" >> $@
@@ -388,7 +391,7 @@ check-unit: tests/unittests/unittests
tests/unittests/unittests
@VALGRIND_CHECK_RULES@
VALGRIND_SUPPRESSIONS_FILES=prof.supp
VALGRIND_SUPPRESSIONS_FILES=$(srcdir)/prof.supp
# Code coverage targets (requires --enable-coverage)
coverage-clean:

View File

@@ -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'
jabber.developer marked this conversation as resolved
Review

Is it required? It needs to be updated each time the set variables of the method are updated, yet it does not provide additional data. It would make more sense to explain what each variable means next to their initiation place.

Is it required? It needs to be updated each time the set variables of the method are updated, yet it does not provide additional data. It would make more sense to explain what each variable means next to their initiation place.

Refactored, moved to separate function

Refactored, moved to separate function
# 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=()
@@ -180,30 +249,13 @@ case "$ARCH" in
;;
esac
case "$ARCH" in
linux*)
echo
echo "--> Building with ./configure -C ${tests[0]} --enable-valgrind $*"
echo
# shellcheck disable=SC2086
./configure -C ${tests[0]} --enable-valgrind $*
$MAKE CC="${CC}"
if grep '^ID=' /etc/os-release | grep -q -e debian; then
$MAKE check-valgrind
else
$MAKE check-valgrind || log_content ./test-suite-memcheck.log
fi
$MAKE distclean
;;
esac
# Function to build and test a single configuration
build_and_test() {
local features="$1"
local extra_args="$2"
local idx="$3"
local run_valgrind="$4"
local run_coverage="$5"
local build_dir="build-$idx"
local log_file="build-$idx.log"
@@ -211,61 +263,210 @@ build_and_test() {
echo "=== Build $idx started at $(date) ==="
jabber.developer marked this conversation as resolved Outdated

Wouldn't it get caught in the error handler declared earlier?

Wouldn't it get caught in the error handler declared earlier?

It is done foe log readability.

It is done foe log readability.
echo "--> Building in $build_dir with ./configure -C $features $extra_args"
local start_time=$SECONDS
mkdir -p "$build_dir"
jabber.developer marked this conversation as resolved Outdated

(same as above) Wouldn't it get caught in the error handler declared earlier?

(same as above) Wouldn't it get caught in the error handler declared earlier?

It is done foe log readability.

It is done foe log readability.
cd "$build_dir"
# shellcheck disable=SC2086
../configure -C $features $extra_args
if ! ../configure -C $features $extra_args; then
echo "ERROR: configure failed"
exit 1
fi
$MAKE CC="${CC}"
$MAKE check-functional-parallel
if ! $MAKE CC="${CC}"; then
echo "ERROR: make failed"
exit 1
fi
# Run unit tests
local unit_passed=0 unit_failed=0
if [ "$run_valgrind" = "yes" ]; then
echo "--> Running unit tests under Valgrind..."
# Build unit tests first
$MAKE tests/unittests/unittests
# Run valgrind directly to capture cmocka output
valgrind --error-exitcode=1 --leak-check=full \
--suppressions=../prof.supp \
tests/unittests/unittests 2>&1 | tee unit-tests-output.log
valgrind_exit=${PIPESTATUS[0]}
if [ $valgrind_exit -ne 0 ]; then
echo "ERROR: Valgrind unit tests failed (exit code $valgrind_exit)"
exit 1
fi
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.
export PROF_BUILD_INDEX=$idx
local func_passed=0 func_failed=0
if ! $MAKE check-functional-parallel; then
echo "ERROR: functional tests failed"
exit 1
fi
# Extract functional test counts from group logs
echo "=== Functional test results ==="
for glog in ./test-logs/group*.log; do
if [ -f "$glog" ]; then
cnt=$(extract_test_count "$glog" "$CMOCKA_PASSED_PATTERN")
[ -n "$cnt" ] && func_passed=$((func_passed + cnt))
cnt=$(extract_test_count "$glog" "$CMOCKA_FAILED_PATTERN")
[ -n "$cnt" ] && func_failed=$((func_failed + cnt))
fi
done
echo "FUNC_TESTS: passed=$func_passed failed=$func_failed"
# Collect coverage data if enabled (lines, functions, branches)
# Must be done BEFORE make clean which removes .gcda files
local cov_lines="n/a" cov_funcs="n/a" cov_branches="n/a"
if [ "$run_coverage" = "yes" ]; then
echo "--> Collecting coverage data..."
if command -v lcov >/dev/null 2>&1; then
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
# 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
local summary
summary=$(lcov --summary coverage.info \
--rc lcov_branch_coverage=1 --ignore-errors inconsistent 2>&1 || true)
cov_lines=$(echo "$summary" | grep -E "lines\.*:" | grep -oE "[0-9]+\.[0-9]+%" | head -1)
cov_funcs=$(echo "$summary" | grep -E "functions\.*:" | grep -oE "[0-9]+\.[0-9]+%" | head -1)
cov_branches=$(echo "$summary" | grep -E "branches\.*:" | grep -oE "[0-9]+\.[0-9]+%" | head -1)
[ -z "$cov_lines" ] && cov_lines="n/a"
[ -z "$cov_funcs" ] && cov_funcs="n/a"
[ -z "$cov_branches" ] && cov_branches="n/a"
else
echo "WARNING: coverage.info is empty or not created"
fi
else
echo "WARNING: lcov not found"
jabber.developer marked this conversation as resolved Outdated

Wouldn't it create trash in non-CI (local) run?

Wouldn't it create trash in non-CI (local) run?

Added [ -n "$CI" ] check. Artifacts only created in CI environment now.

Added [ -n "$CI" ] check. Artifacts only created in CI environment now.
fi
echo "COVERAGE: lines=$cov_lines funcs=$cov_funcs branches=$cov_branches"
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 ..
rm -rf "$build_dir"
local elapsed=$((SECONDS - start_time))
local mins=$((elapsed / 60))
local secs=$((elapsed % 60))
echo "=== Build $idx completed at $(date) ==="
echo "STATS: unit_passed=$unit_passed unit_failed=$unit_failed func_passed=$func_passed func_failed=$func_failed cov_lines=$cov_lines cov_funcs=$cov_funcs cov_branches=$cov_branches time=${mins}m${secs}s"
} > "$log_file" 2>&1
}
# Run all 4 configurations in parallel
echo "Starting parallel builds..."
pids=()
for idx in 1 2 3 4; do
if [ $idx -le ${#tests[@]} ]; then
build_and_test "${tests[$((idx-1))]}" "$*" "$idx" &
pids+=("$!")
echo "Started build $idx (PID: $!)"
fi
done
# Run configurations
# Coverage enabled only for build 1 (Full) - it has most code paths
echo
echo "=== Start build ==="
echo
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..."
jabber.developer marked this conversation as resolved Outdated

Can we repeat order of the variables for readability? I.e. run_valgrind and then extra_flags to match order above

Can we repeat order of the variables for readability? I.e. `run_valgrind` and then `extra_flags` to match order above

Unified

Unified
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"
else
run_valgrind="no"
extra_flags=""
fi
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
done
fi
echo
# Wait for all builds and check exit codes
failed=0
echo "Waiting for builds to complete..."
echo
failed_builds=()
for i in "${!pids[@]}"; do
idx=$((i + 1))
if wait "${pids[$i]}"; then
echo "✓ Build $idx passed"
if [ -f "build-$idx.log" ]; then
parse_build_stats "build-$idx.log"
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: ${STAT_TIME:-?}"
else
echo "${BUILD_NAMES[$i]} passed (no stats available)"
fi
else
echo "Build $idx failed"
echo "--- Log for build $idx ---"
cat "build-$idx.log"
echo "--- End log ---"
failed=1
echo "${BUILD_NAMES[$i]} FAILED" >&2
failed_builds+=("$idx")
fi
echo
done
# Show failed builds full logs
for idx in "${failed_builds[@]}"; do
if [ -f "build-$idx.log" ]; then
echo "=== ${BUILD_NAMES[$((idx-1))]} FAILURE LOG ===" >&2
cat "build-$idx.log" >&2
echo >&2
fi
done
# Show all logs on success too
if [ $failed -eq 0 ]; then
echo
echo "All builds passed!"
for idx in 1 2 3 4; do
if [ -f "build-$idx.log" ]; then
echo "--- Log for build $idx ---"
cat "build-$idx.log"
fi
done
else
if [ ${#failed_builds[@]} -gt 0 ]; then
echo "RESULT: FAILED (builds ${failed_builds[*]})" >&2
exit 1
else
if [ "$COVERAGE_ONLY" = "yes" ]; then
echo "RESULT: COVERAGE BUILD PASSED ✓"
else
echo "RESULT: ALL $TEST_BUILDS BUILDS PASSED ✓"
fi
fi

View File

@@ -8,6 +8,29 @@
# * python suppressions file from https://github.com/python/cpython/blob/main/Misc/valgrind-python.supp
#
# ============================================
# glibc AVX2 optimizations (false positives)
# See: https://sourceware.org/bugzilla/show_bug.cgi?id=19796
# ============================================
{
glibc_wcpncpy_avx2
Memcheck:Addr32
fun:__wcpncpy_avx2
fun:wcsxfrm_l
fun:g_utf8_collate_key
...
}
{
glibc_wcsxfrm_avx2
Memcheck:Addr32
...
fun:wcsxfrm_l
fun:g_utf8_collate_key
...
}
# ============================================
# Functional tests suppressions (stabber/pthread)
# ============================================

View File

@@ -69,6 +69,12 @@ main(int argc, char* argv[])
}
}
char group_env[16];
snprintf(group_env, sizeof(group_env), "%d", group);
setenv("PROF_TEST_GROUP", group_env, 1);
fprintf(stderr, "[PROF_TEST] Starting functional tests, group=%d\n", group);
/* ============================================================
* GROUP 1: Connect, Ping, Rooms, Software
* Basic XMPP session establishment and server queries

View File

@@ -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;
@@ -136,8 +139,14 @@ _create_logs_dir(void)
void
_cleanup_dirs(void)
{
jabber.developer marked this conversation as resolved Outdated

IMPORTANT:
Here and everywhere else: can we avoid magic constant of "4"? Can we make "TEST_GROUPS" constant or something similar? This way it would be much easier to adjust in the future

IMPORTANT: Here and everywhere else: can we avoid magic constant of "4"? Can we make "TEST_GROUPS" constant or something similar? This way it would be much easier to adjust in the future

Added #define TEST_GROUPS 4 constant.

Added #define TEST_GROUPS 4 constant.
const char *group_env = getenv("PROF_TEST_GROUP");
int group = group_env ? atoi(group_env) : 0;
int dir_id = (group >= 1 && group <= TEST_GROUPS) ? group : stub_port;
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 ./tests/functionaltests/files/%d", stub_port);
snprintf(cmd, sizeof(cmd), "rm -rf ./test-files/%d", dir_id);
int res = system(cmd);
if (res == -1) {
assert_true(FALSE);
@@ -221,30 +230,74 @@ prof_start(void)
/* Set non-blocking mode for reading */
int flags = fcntl(fd, F_GETFL, 0);
fcntl(fd, F_SETFL, flags | O_NONBLOCK);
/* Brief wait for process to initialize */
usleep(50000); /* 50ms */
}
int
init_prof_test(void **state)
{
/* Get test group from environment for static resource allocation */
const char *group_env = getenv("PROF_TEST_GROUP");
int group = group_env ? atoi(group_env) : 0;
/* Get build index for port offset (for parallel CI builds) */
const char *build_env = getenv("PROF_BUILD_INDEX");
int build_idx = build_env ? atoi(build_env) : 0;
jabber.developer marked this conversation as resolved
Review

Why 2 builds (0 and 1) share the same ports range?

Why 2 builds (0 and 1) share the same ports range?

Added explanation: builds 0/1 share ports because they're mutually exclusive (sequential mode or parallel).

Added explanation: builds 0/1 share ports because they're mutually exclusive (sequential mode or parallel).
/* 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. */
jabber.developer marked this conversation as resolved Outdated

Can we do something with the magic number 4 in the comments as well? Here and elsewhere

Can we do something with the magic number 4 in the comments as well? Here and elsewhere

Config names added additional to predefined values.

Config names added additional to predefined values.
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;
jabber.developer marked this conversation as resolved Outdated

It shouldn't be at the stderr. It would make more sense to put it in the stdout, since it's informational/debug data.

It shouldn't be at the `stderr`. It would make more sense to put it in the stdout, since it's informational/debug data.

Fixed. Info messages now use printf() (stdout), stderr reserved for errors only.

Fixed. Info messages now use printf() (stdout), stderr reserved for errors only.
for (int p = 5230; p < 5250; ++p) {
int ret = stbbr_start(STBBR_LOGDEBUG, p, 0);
if (ret == 0) {
stub_port = p;
if (group >= 1 && group <= TEST_GROUPS) {
/* Static allocation: each group gets a dedicated port */
stub_port = port_base + group - 1;
jabber.developer marked this conversation as resolved
Review

It shouldn't be at the stderr. It would make more sense to put it in the stdout, since it's informational/debug data.

It shouldn't be at the `stderr`. It would make more sense to put it in the stdout, since it's informational/debug data.

Fixed. Info messages now use printf() (stdout), stderr reserved for errors only.

Fixed. Info messages now use printf() (stdout), stderr reserved for errors only.
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;
break;
printf("[PROF_TEST] Started stabber on port %d\n", stub_port);
} else {
printf("[PROF_TEST] Failed to start stabber on port %d\n", stub_port);
jabber.developer marked this conversation as resolved
Review

Should be in stderr. Please check analogous cases

Should be in stderr. Please check analogous cases

Corrected.

Corrected.
}
jabber.developer marked this conversation as resolved
Review

It shouldn't be at the stderr. It would make more sense to put it in the stdout, since it's informational/debug data.

It shouldn't be at the `stderr`. It would make more sense to put it in the stdout, since it's informational/debug data.

Fixed. Info messages now use printf() (stdout), stderr reserved for errors only.

Fixed. Info messages now use printf() (stdout), stderr reserved for errors only.
}
/* Fallback to dynamic allocation if static failed or group=0 */
if (!started) {
assert_true(FALSE); // could not start stabber on any port in range
printf("[PROF_TEST] Using dynamic port allocation\n");
jabber.developer marked this conversation as resolved Outdated

It shouldn't be at the stderr. It would make more sense to put it in the stdout, since it's informational/debug data.

It shouldn't be at the `stderr`. It would make more sense to put it in the stdout, since it's informational/debug data.

Fixed. Info messages now use printf() (stdout), stderr reserved for errors only.

Fixed. Info messages now use printf() (stdout), stderr reserved for errors only.
for (int p = port_base; p < port_base + 20; ++p) {
if (stbbr_start(STBBR_LOGDEBUG, p, 0) == 0) {
stub_port = p;
started = TRUE;
printf("[PROF_TEST] Started stabber on port %d\n", stub_port);
break;
}
}
jabber.developer marked this conversation as resolved Outdated

Why do we need to do this?

Why do we need to do this?

Removed.

Removed.
}
if (!started) {
fprintf(stderr, "[PROF_TEST] ERROR: could not start stabber on any port\n");
return -1;
}
// Generate unique XDG paths based on stub_port for parallel execution
/* 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),
"./tests/functionaltests/files/%d/xdg_config_home", stub_port);
"./test-files/%d/xdg_config_home", dir_id);
jabber.developer marked this conversation as resolved
Review

same as above

same as above

Removed

Removed
snprintf(xdg_data_home, sizeof(xdg_data_home),
"./tests/functionaltests/files/%d/xdg_data_home", stub_port);
"./test-files/%d/xdg_data_home", dir_id);
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
@@ -401,6 +454,16 @@ prof_output_regex(const char *pattern)
usleep(50000); /* 50ms */
}
/* Timeout reached - log diagnostic info */
fprintf(stderr, "Timeout waiting for regex '%s' after %d seconds. Last output:\n", pattern, expect_timeout);
size_t len = strlen(output_buffer);
if (len > 500) {
fprintf(stderr, "...%s", output_buffer + len - 500);
} else {
fprintf(stderr, "%s", output_buffer);
}
fprintf(stderr, "\n");
regfree(&regex);
return 0;
}