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
Showing only changes of commit a546092a5f - Show all commits

View File

@@ -90,12 +90,18 @@ set -e
``` ```
This will run the same tests that the CI runs and refuse the push if it fails. 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. The CI script runs 4 parallel builds with different configurations:
Use `./ci-build.sh --coverage-only` to run only build 1 (full configuration with all optional components enabled) with coverage collection. - **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: Output shows test results per build:
``` ```
BUILD 1 PASSED Full PASSED
Unit tests: 437 passed, 0 failed Unit tests: 437 passed, 0 failed
Functional tests: 69 passed, 0 failed Functional tests: 69 passed, 0 failed
Coverage: Lines: 27.5% | Functions: 36.2% | Branches: 18.1% Coverage: Lines: 27.5% | Functions: 36.2% | Branches: 18.1%

View File

@@ -15,9 +15,9 @@ error_handler()
log_content ./test-suite.log log_content ./test-suite.log
log_content ./test-suite-memcheck.log log_content ./test-suite-memcheck.log
echo echo >&2
echo "Error ${ERR_CODE} with command '${BASH_COMMAND}' on line ${BASH_LINENO[0]}. Exiting." echo "Error ${ERR_CODE} with command '${BASH_COMMAND}' on line ${BASH_LINENO[0]}. Exiting." >&2
echo echo >&2
exit ${ERR_CODE} exit ${ERR_CODE}
} }
@@ -27,6 +27,17 @@ trap error_handler ERR
# Constants # 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 # Regex patterns for parsing test output
readonly CMOCKA_PASSED_PATTERN='^\[ PASSED \] [0-9]+ test' readonly CMOCKA_PASSED_PATTERN='^\[ PASSED \] [0-9]+ test'
readonly CMOCKA_FAILED_PATTERN='^\[ FAILED \] [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
@@ -108,11 +119,11 @@ EOF
# Test 1: Single failing test detection # Test 1: Single failing test detection
echo " Testing single test failure detection..." echo " Testing single test failure detection..."
if /tmp/test_must_fail > /tmp/test_must_fail.log 2>&1; then 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 "ERROR: Test that should fail returned success (exit code 0)" >&2
echo "This means the test framework is NOT detecting failures correctly!" echo "This means the test framework is NOT detecting failures correctly!" >&2
echo "--- Test output ---" echo "--- Test output ---" >&2
cat /tmp/test_must_fail.log cat /tmp/test_must_fail.log >&2
echo "--- End output ---" echo "--- End output ---" >&2
rm -f /tmp/test_must_fail /tmp/test_must_fail.c /tmp/test_must_fail.log rm -f /tmp/test_must_fail /tmp/test_must_fail.c /tmp/test_must_fail.log
exit 1 exit 1
fi fi
@@ -132,8 +143,8 @@ EOF
wait $pid4 || failed=$((failed + 1)) wait $pid4 || failed=$((failed + 1))
if [ $failed -ne 2 ]; then if [ $failed -ne 2 ]; then
echo "ERROR: Expected 2 failures in parallel tests, got $failed" echo "ERROR: Expected 2 failures in parallel tests, got $failed" >&2
echo "Parallel failure detection is broken!" 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 rm -f /tmp/test_must_fail /tmp/test_must_fail.c /tmp/test_must_fail.log /tmp/p?.log
exit 1 exit 1
fi fi
@@ -380,34 +391,34 @@ echo "=== Start build ==="
echo echo
if [ "$COVERAGE_ONLY" = "yes" ]; then if [ "$COVERAGE_ONLY" = "yes" ]; then
echo "Running coverage-only mode (build 1 only)..." echo "Running coverage-only mode (${BUILD_NAMES[0]} build)..."
echo echo
run_valgrind="no" run_valgrind="no"
run_coverage="yes" run_coverage="yes"
extra_flags="--enable-coverage" extra_flags="--enable-coverage"
build_and_test "${tests[0]}" "$* $extra_flags" "1" "$run_valgrind" "$run_coverage" & build_and_test "${tests[0]}" "$* $extra_flags" "1" "$run_valgrind" "$run_coverage" &
pids=("$!") pids=("$!")
echo "Build 1: ${tests[0]} [+Coverage]" echo "${BUILD_NAMES[0]}: ${tests[0]} [+Coverage]"
else else
echo "Starting 4 parallel build configurations..." 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 echo
pids=() pids=()
for idx in 1 2 3 4; do for idx in $(seq 1 $TEST_BUILDS); do
if [ $idx -le ${#tests[@]} ]; then if [ $idx -le ${#tests[@]} ]; then
# All builds run Valgrind on Linux # All builds run Valgrind on Linux
if [ "$ARCH" = "linux" ]; then if [ "$ARCH" = "linux" ]; then
extra_flags="--enable-valgrind"
run_valgrind="yes" run_valgrind="yes"
extra_flags="--enable-valgrind"
else else
extra_flags=""
run_valgrind="no" run_valgrind="no"
extra_flags=""
fi fi
run_coverage="no" run_coverage="no"
build_and_test "${tests[$((idx-1))]}" "$* $extra_flags" "$idx" "$run_valgrind" "$run_coverage" & build_and_test "${tests[$((idx-1))]}" "$* $extra_flags" "$idx" "$run_valgrind" "$run_coverage" &
pids+=("$!") pids+=("$!")
flags_desc="" flags_desc=""
[ "$run_valgrind" = "yes" ] && flags_desc=" [+Valgrind]" [ "$run_valgrind" = "yes" ] && flags_desc=" [+Valgrind]"
echo "Build $idx: ${tests[$((idx-1))]}$flags_desc" echo "${BUILD_NAMES[$((idx-1))]}: ${tests[$((idx-1))]}$flags_desc"
fi fi
done done
fi fi
@@ -423,7 +434,7 @@ for i in "${!pids[@]}"; do
if [ -f "build-$idx.log" ]; then if [ -f "build-$idx.log" ]; then
parse_build_stats "build-$idx.log" parse_build_stats "build-$idx.log"
echo "BUILD $idx PASSED" echo "${BUILD_NAMES[$i]} PASSED"
echo " Unit tests: $STAT_UNIT_P passed, $STAT_UNIT_F failed" echo " Unit tests: $STAT_UNIT_P passed, $STAT_UNIT_F failed"
echo " Functional tests: $STAT_FUNC_P passed, $STAT_FUNC_F failed" echo " Functional tests: $STAT_FUNC_P passed, $STAT_FUNC_F failed"
if [ "$STAT_COV_LINES" != "n/a" ] && [ -n "$STAT_COV_LINES" ]; then if [ "$STAT_COV_LINES" != "n/a" ] && [ -n "$STAT_COV_LINES" ]; then
@@ -431,10 +442,10 @@ for i in "${!pids[@]}"; do
fi fi
echo " Duration: ${STAT_TIME:-?}" echo " Duration: ${STAT_TIME:-?}"
else else
echo "Build $idx passed (no stats available)" echo "${BUILD_NAMES[$i]} passed (no stats available)"
fi fi
else else
echo "BUILD $idx FAILED" echo "${BUILD_NAMES[$i]} FAILED" >&2
failed_builds+=("$idx") failed_builds+=("$idx")
fi fi
echo echo
@@ -443,19 +454,19 @@ done
# Show failed builds full logs # Show failed builds full logs
for idx in "${failed_builds[@]}"; do for idx in "${failed_builds[@]}"; do
if [ -f "build-$idx.log" ]; then if [ -f "build-$idx.log" ]; then
echo "=== BUILD $idx FAILURE LOG ===" echo "=== ${BUILD_NAMES[$((idx-1))]} FAILURE LOG ===" >&2
cat "build-$idx.log" cat "build-$idx.log" >&2
echo echo >&2
fi fi
done done
if [ ${#failed_builds[@]} -gt 0 ]; then if [ ${#failed_builds[@]} -gt 0 ]; then
echo "RESULT: FAILED (builds ${failed_builds[*]})" echo "RESULT: FAILED (builds ${failed_builds[*]})" >&2
exit 1 exit 1
else else
if [ "$COVERAGE_ONLY" = "yes" ]; then if [ "$COVERAGE_ONLY" = "yes" ]; then
echo "RESULT: COVERAGE BUILD PASSED ✓" echo "RESULT: COVERAGE BUILD PASSED ✓"
else else
echo "RESULT: ALL 4 BUILDS PASSED ✓" echo "RESULT: ALL $TEST_BUILDS BUILDS PASSED ✓"
fi fi
fi fi

View File

@@ -247,9 +247,9 @@ init_prof_test(void **state)
int build_idx = build_env ? atoi(build_env) : 0; 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. /* Calculate port base: each build uses a different range of TEST_GROUPS ports.
* Build 0 (local/default): 5230-5233, Build 1: 5230-5233, Build 2: 5234-5237, etc. * Build 0 (local/default): 5230-5233, Full: 5230-5233, Minimal: 5234-5237, etc.
* Build 0 and 1 share the same range because build 0 is for local runs (no parallel builds), * Build 0 and Full share the same range because build 0 is for local runs or sequential run (no parallel builds),
* while builds 1-4 are used in CI where they run in parallel. */ * 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); int port_base = 5230 + ((build_idx > 0 ? build_idx - 1 : 0) * TEST_GROUPS);
/* Static resource allocation to avoid conflicts in parallel execution. /* Static resource allocation to avoid conflicts in parallel execution.