diff --git a/.github/workflows/CI-lint-groups-json.yml b/.github/workflows/CI-lint-groups-json.yml index 5de67b0a2b..fc94a35c44 100644 --- a/.github/workflows/CI-lint-groups-json.yml +++ b/.github/workflows/CI-lint-groups-json.yml @@ -25,6 +25,8 @@ jobs: run: python3 test/tap/groups/lint_groups_json.py - name: Check every TAP source is registered in groups.json run: python3 test/tap/groups/check_groups.py --source + - name: Check coverage collector invariants + run: test/infra/control/validate-coverage-gcov-toolchain.bash - name: Check group infra/workflow coverage (warn-only) # Warns when a group references a missing infra (phantom), or when no # workflow on either branch can select the group at all -- meaning diff --git a/docs/superpowers/plans/2026-08-14-final-gcov-dump.md b/docs/superpowers/plans/2026-08-14-final-gcov-dump.md new file mode 100644 index 0000000000..7eaa8f0c0d --- /dev/null +++ b/docs/superpowers/plans/2026-08-14-final-gcov-dump.md @@ -0,0 +1,54 @@ +# Final GCOV Dump Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Preserve daemon-side coverage from every TAP test by dumping ProxySQL's in-memory GCOV counters exactly once after each isolated TAP group. + +**Architecture:** `run-tests-isolated.bash` already has an EXIT trap that runs after the TAP suite and before ProxySQL is stopped. The trap will issue `PROXYSQL GCOV DUMP`, then decode the resulting GCDA files. The obsolete per-TAP-test dump in `proxysql-tester.py` will be removed because GCC 13 ignores later dumps unless counters are reset. Deliberate dump/decode/reset stages for non-TAP modes remain unchanged. + +**Tech Stack:** Bash, Python TAP harness, GCC 13 GCOV, fastcov/LCOV. + +## Global Constraints + +- Generate real TAP traffic without changing functional tests. +- Dump the long-running ProxySQL process once per isolated TAP group. +- Collect coverage on successful, failed, and timed-out test runs. +- Preserve the original test exit status unless coverage collection itself is the only failure. + +--- + +### Task 1: Add the daemon-dump regression validator + +**Files:** +- Create: `test/infra/control/test-final-gcov-dump.bash` +- Create: `test/infra/control/dump-proxysql-gcov.bash` +- Create: `test/infra/control/coverage-exit-status.bash` +- Create: `test/infra/control/fixtures/record-mysql-argv.bash` +- Modify: `test/infra/control/validate-coverage-gcov-toolchain.bash` +- Modify: `.github/workflows/CI-lint-groups-json.yml` + +- [x] Write a validator requiring the final dump to precede `fastcov`, forbidding per-test dumps, bounding the admin call, and requiring dump failures to propagate. +- [x] Run it and confirm that the current pipeline fails the assertions. +- [x] Wire the validator into the existing lightweight CI lint workflow. + +### Task 2: Dump daemon counters once at group exit + +**Files:** +- Modify: `test/infra/control/run-tests-isolated.bash` +- Modify: `test/scripts/bin/proxysql-tester.py` + +- [x] Remove the dump after each TAP executable. +- [x] Issue one admin dump in the coverage EXIT trap before GCDA decoding. +- [x] Record dump failure without skipping the remaining diagnostic collection. +- [x] Return the test failure when tests failed; otherwise return the coverage failure. +- [x] Run the validator until it passes. + +### Task 3: Verify and publish + +**Files:** +- Verify all modified files. + +- [x] Run the coverage validators, Bash syntax checks, Python compilation, and `git diff --check`. +- [x] Review the final diff against `origin/v3.0` for unrelated changes. +- [ ] Commit and push the focused fix to PR #6062. +- [ ] Inspect the resulting GitHub Actions coverage run and compare daemon-side line coverage with the previous upload. diff --git a/test/infra/control/coverage-exit-status.bash b/test/infra/control/coverage-exit-status.bash new file mode 100644 index 0000000000..8c68dbd313 --- /dev/null +++ b/test/infra/control/coverage-exit-status.bash @@ -0,0 +1,11 @@ +#!/bin/bash + +coverage_exit_status() { + local test_exit="${1:?test exit status required}" + local coverage_exit="${2:?coverage exit status required}" + + if [ "${test_exit}" -ne 0 ]; then + return "${test_exit}" + fi + return "${coverage_exit}" +} diff --git a/test/infra/control/dump-proxysql-gcov.bash b/test/infra/control/dump-proxysql-gcov.bash new file mode 100755 index 0000000000..596ae6a3fb --- /dev/null +++ b/test/infra/control/dump-proxysql-gcov.bash @@ -0,0 +1,24 @@ +#!/bin/bash +set -euo pipefail + +mysql_client="${MYSQL_CLIENT_BIN:-mysql}" +admin_user="${TAP_ADMINUSERNAME:-radmin}" +admin_password="${TAP_ADMINPASSWORD:-radmin}" +admin_host="${TAP_ADMINHOST:-proxysql}" +admin_port="${TAP_ADMINPORT:-6032}" +dump_timeout="${GCOV_DUMP_TIMEOUT_SECONDS:-15}" + +if [[ ! "${dump_timeout}" =~ ^([0-9]+([.][0-9]*)?|[.][0-9]+)$ ]] \ + || [[ "${dump_timeout}" =~ ^0*([.]0*)?$ ]]; then + echo "GCOV_DUMP_TIMEOUT_SECONDS must be a positive number of seconds" >&2 + exit 64 +fi + +exec timeout --signal=TERM --kill-after=5s "${dump_timeout}s" "${mysql_client}" \ + "-u${admin_user}" \ + "-p${admin_password}" \ + "-h${admin_host}" \ + "-P${admin_port}" \ + --batch \ + --skip-column-names \ + -e "PROXYSQL GCOV DUMP" diff --git a/test/infra/control/fixtures/record-mysql-argv.bash b/test/infra/control/fixtures/record-mysql-argv.bash new file mode 100755 index 0000000000..63b1e6bac8 --- /dev/null +++ b/test/infra/control/fixtures/record-mysql-argv.bash @@ -0,0 +1,8 @@ +#!/bin/bash +set -u + +if [ -n "${MYSQL_DELAY_SECONDS:-}" ]; then + sleep "${MYSQL_DELAY_SECONDS}" +fi +printf '%s\n' "$@" > "${MYSQL_RECORD_FILE:?MYSQL_RECORD_FILE is required}" +exit "${MYSQL_EXIT_CODE:-0}" diff --git a/test/infra/control/run-multi-group.bash b/test/infra/control/run-multi-group.bash index 4507a93a7f..775297a101 100755 --- a/test/infra/control/run-multi-group.bash +++ b/test/infra/control/run-multi-group.bash @@ -353,6 +353,7 @@ if [ "${COVERAGE}" -eq 1 ]; then # Each group's test-runner already copied .gcno adjacent to .gcda (in its # EXIT trap). We run fastcov sequentially per group — no concurrent gcov. COVERAGE_LOG="${COMBINED_COVERAGE_DIR}/coverage-generation.log" + COVERAGE_FAILED=0 for group in ${TAP_GROUPS}; do infra_id="${group}-${RUN_ID}" gcov_dir="${WORKSPACE}/ci_infra_logs/${infra_id}/gcov" @@ -360,7 +361,7 @@ if [ "${COVERAGE}" -eq 1 ]; then if [ -d "${gcov_dir}" ] && [ "$(find "${gcov_dir}" -name '*.gcda' 2>/dev/null | head -1)" ]; then echo ">>> Generating coverage for ${group} from ${gcov_dir}..." - docker run --rm \ + if ! docker run --rm \ -v "${WORKSPACE}:${WORKSPACE}" \ -e WORKSPACE="${WORKSPACE}" \ -e GCOV_DIR="${gcov_dir}" \ @@ -368,14 +369,20 @@ if [ "${COVERAGE}" -eq 1 ]; then -e COVERAGE_LOG="${COVERAGE_LOG}" \ proxysql-ci-base:latest \ bash -c ' - if command -v fastcov >/dev/null 2>&1; then - cd "${GCOV_DIR}" - fastcov -b -j4 -l \ - -e /usr deps \ - -d . -o "${GROUP_INFO}" >> "${COVERAGE_LOG}" 2>&1 || \ - echo ">>> WARNING: fastcov failed for ${GCOV_DIR}" >> "${COVERAGE_LOG}" + set -e + cd "${GCOV_DIR}" + fastcov -b -j4 -l \ + -e /usr deps \ + -d . -o "${GROUP_INFO}" >> "${COVERAGE_LOG}" 2>&1 + if [ ! -s "${GROUP_INFO}" ]; then + echo ">>> ERROR: fastcov produced an empty coverage report for ${GCOV_DIR}" >&2 + exit 1 fi - ' || echo ">>> WARNING: Coverage generation failed for ${group}" + ' + then + echo ">>> ERROR: Coverage generation failed for ${group} (see ${COVERAGE_LOG})" >&2 + COVERAGE_FAILED=1 + fi else echo ">>> No .gcda files found for ${group}, skipping" fi @@ -472,6 +479,10 @@ if [ "${COVERAGE}" -eq 1 ]; then else echo ">>> No coverage files found to combine" fi + if [ "${COVERAGE_FAILED}" -ne 0 ]; then + echo ">>> ERROR: One or more groups failed coverage generation" >&2 + OVERALL_FAILED=1 + fi echo "" fi diff --git a/test/infra/control/run-tests-isolated.bash b/test/infra/control/run-tests-isolated.bash index d4734a4bde..12fcff6c13 100755 --- a/test/infra/control/run-tests-isolated.bash +++ b/test/infra/control/run-tests-isolated.bash @@ -334,9 +334,26 @@ docker run \ # when standalone, it runs here. collect_coverage() { local exit_code=\$? + local coverage_exit=0 + trap - EXIT + set +e if [ \"\${COVERAGE_MODE}\" = \"1\" ]; then + ( + set -e + coverage_failed=0 echo \">>> Collecting code coverage data (exit code was: \${exit_code})...\" + # ProxySQL is a long-running process, so its in-memory counters + # are not guaranteed to reach the GCDA files during container + # teardown. GCC 13 also ignores a second __gcov_dump call until + # __gcov_reset is called. Do not dump inside the TAP loop; dump + # once here after the group and before any GCDA decoding starts. + echo \">>> Dumping ProxySQL GCOV counters after the test group...\" + if ! \"${SCRIPT_DIR}/dump-proxysql-gcov.bash\"; then + echo \">>> ERROR: Failed to dump ProxySQL GCOV counters\" >&2 + coverage_failed=1 + fi + if [ -d \"/gcov\" ] && [ \"\$(ls -A /gcov 2>/dev/null)\" ]; then # Match .gcno files to .gcda files by basename and copy # adjacent so fastcov can find them. @@ -397,7 +414,11 @@ docker run \ cd /gcov fastcov -b -j\$(nproc) -l \ -e /usr deps \ - -d . -o \"\${coverage_file}\" >> \"\${coverage_log}\" 2>&1 || echo \">>> WARNING: Coverage generation failed (see \${coverage_log})\" + -d . -o \"\${coverage_file}\" >> \"\${coverage_log}\" 2>&1 + if [ ! -s \"\${coverage_file}\" ]; then + echo \">>> ERROR: fastcov produced an empty coverage report (see \${coverage_log})\" >&2 + exit 1 + fi if [ -f \"\${coverage_file}\" ]; then echo \">>> Coverage report generated: \${coverage_file}\" @@ -440,8 +461,13 @@ docker run \ else echo \">>> WARNING: /gcov directory is empty or missing, skipping coverage\" fi + exit \${coverage_failed} + ) + coverage_exit=\$? fi - exit \${exit_code} + source "${SCRIPT_DIR}/coverage-exit-status.bash" + coverage_exit_status \"\${exit_code}\" \"\${coverage_exit}\" + exit \$? } trap collect_coverage EXIT diff --git a/test/infra/control/test-final-gcov-dump.bash b/test/infra/control/test-final-gcov-dump.bash new file mode 100755 index 0000000000..dddf7145fd --- /dev/null +++ b/test/infra/control/test-final-gcov-dump.bash @@ -0,0 +1,131 @@ +#!/bin/bash +set -euo pipefail + +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +subject="${script_dir}/dump-proxysql-gcov.bash" +fake_mysql="${script_dir}/fixtures/record-mysql-argv.bash" +runner="${script_dir}/run-tests-isolated.bash" +tester="${script_dir}/../../scripts/bin/proxysql-tester.py" +status_helper="${script_dir}/coverage-exit-status.bash" +record_file="$(mktemp)" +trap 'rm -f "${record_file}"' EXIT + +MYSQL_CLIENT_BIN="${fake_mysql}" MYSQL_RECORD_FILE="${record_file}" "${subject}" + +expected=( + -uradmin + -pradmin + -hproxysql + -P6032 + --batch + --skip-column-names + -e + "PROXYSQL GCOV DUMP" +) +mapfile -t actual < "${record_file}" + +if [ "${#actual[@]}" -ne "${#expected[@]}" ]; then + echo "expected ${#expected[@]} mysql arguments, got ${#actual[@]}" >&2 + exit 1 +fi +for i in "${!expected[@]}"; do + if [ "${actual[$i]}" != "${expected[$i]}" ]; then + echo "mysql argument ${i}: expected '${expected[$i]}', got '${actual[$i]}'" >&2 + exit 1 + fi +done + +TAP_ADMINUSERNAME="ci-admin" TAP_ADMINPASSWORD="ci-secret" \ + TAP_ADMINHOST="proxy-under-test" TAP_ADMINPORT="16032" \ + MYSQL_CLIENT_BIN="${fake_mysql}" MYSQL_RECORD_FILE="${record_file}" "${subject}" +mapfile -t actual < "${record_file}" +expected=( + -uci-admin + -pci-secret + -hproxy-under-test + -P16032 + --batch + --skip-column-names + -e + "PROXYSQL GCOV DUMP" +) +for i in "${!expected[@]}"; do + if [ "${actual[$i]}" != "${expected[$i]}" ]; then + echo "custom mysql argument ${i}: expected '${expected[$i]}', got '${actual[$i]}'" >&2 + exit 1 + fi +done + +set +e +MYSQL_CLIENT_BIN="${fake_mysql}" MYSQL_RECORD_FILE="${record_file}" MYSQL_EXIT_CODE=23 "${subject}" +dump_exit=$? +set -e +if [ "${dump_exit}" -ne 23 ]; then + echo "expected mysql failure 23 to propagate, got ${dump_exit}" >&2 + exit 1 +fi + +set +e +MYSQL_CLIENT_BIN="${fake_mysql}" MYSQL_RECORD_FILE="${record_file}" \ + MYSQL_DELAY_SECONDS=2 GCOV_DUMP_TIMEOUT_SECONDS=0.1 "${subject}" +timeout_exit=$? +set -e +if [ "${timeout_exit}" -ne 124 ]; then + echo "expected bounded mysql call to exit 124, got ${timeout_exit}" >&2 + exit 1 +fi + +set +e +MYSQL_CLIENT_BIN="${fake_mysql}" MYSQL_RECORD_FILE="${record_file}" \ + GCOV_DUMP_TIMEOUT_SECONDS=0 "${subject}" 2>/dev/null +zero_timeout_exit=$? +set -e +if [ "${zero_timeout_exit}" -ne 64 ]; then + echo "expected zero timeout to be rejected with exit 64, got ${zero_timeout_exit}" >&2 + exit 1 +fi + +helper_calls=$(grep -Fc 'dump-proxysql-gcov.bash' "${runner}") +if [ "${helper_calls}" -ne 1 ]; then + echo "expected one final dump invocation in isolated runner, got ${helper_calls}" >&2 + exit 1 +fi +dump_line=$(grep -nF 'dump-proxysql-gcov.bash' "${runner}" | cut -d: -f1) +decode_line=$(grep -nF 'fastcov -b' "${runner}" | head -n1 | cut -d: -f1) +if [ "${dump_line}" -ge "${decode_line}" ]; then + echo "final daemon dump must run before GCDA decoding" >&2 + exit 1 +fi +if grep -qF 'self.padmin_command("PROXYSQL GCOV DUMP")' "${tester}"; then + echo "per-test GCOV dumps must remain disabled" >&2 + exit 1 +fi +status_calls=$(grep -Fc 'coverage_exit_status' "${runner}") +if [ "${status_calls}" -ne 1 ]; then + echo "coverage trap must resolve the final exit status exactly once" >&2 + exit 1 +fi + +assert_final_exit() { + local test_exit="$1" + local coverage_exit="$2" + local expected_status="$3" + local actual + + set +e + bash -c 'source "$1"; coverage_exit_status "$2" "$3"' \ + bash "${status_helper}" "${test_exit}" "${coverage_exit}" + actual=$? + set -e + if [ "${actual}" -ne "${expected_status}" ]; then + echo "test exit ${test_exit}, coverage exit ${coverage_exit}: expected ${expected_status}, got ${actual}" >&2 + exit 1 + fi +} + +assert_final_exit 0 0 0 +assert_final_exit 0 1 1 +assert_final_exit 7 0 7 +assert_final_exit 7 1 7 + +echo "final GCOV dump helper tests passed" diff --git a/test/infra/control/validate-coverage-gcov-toolchain.bash b/test/infra/control/validate-coverage-gcov-toolchain.bash new file mode 100755 index 0000000000..322e05cd4c --- /dev/null +++ b/test/infra/control/validate-coverage-gcov-toolchain.bash @@ -0,0 +1,29 @@ +#!/usr/bin/env bash +set -euo pipefail + +root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd)" +dockerfile="${root}/test/infra/docker-base/Dockerfile" +runner="${root}/test/infra/control/run-tests-isolated.bash" +multi="${root}/test/infra/control/run-multi-group.bash" +lint_workflow="${root}/.github/workflows/CI-lint-groups-json.yml" + +if grep -Eq '^[[:space:]]*gcc-11[[:space:]\\]*$' "${dockerfile}"; then + echo "coverage image must use the compiler's default GCOV reader" >&2 + exit 1 +fi +for file in "${runner}" "${multi}"; do + # `-C` combines existing LCOV files and does not decode raw GCOV data. + # Every other fastcov branch-coverage invocation must use the image's + # default gcov, which matches the compiler used by the build handoff. + raw_fastcov_calls="$(awk '/fastcov -b/ && !/ -C / {count++} END {print count + 0}' "${file}")" + test "${raw_fastcov_calls}" -gt 0 + if grep -Eq 'fastcov -b -g gcov-[0-9]+' "${file}"; then + echo "raw GCOV decoding must not force a versioned reader: ${file}" >&2 + exit 1 + fi +done +grep -Eq -- '-s .*coverage_file' "${runner}" +grep -Eq -- '-s .*GROUP_INFO' "${multi}" +grep -Eq 'validate-coverage-gcov-toolchain\.bash' "${lint_workflow}" + +"${root}/test/infra/control/test-final-gcov-dump.bash" diff --git a/test/scripts/bin/proxysql-tester.py b/test/scripts/bin/proxysql-tester.py index 39550c31d3..22adacbf63 100755 --- a/test/scripts/bin/proxysql-tester.py +++ b/test/scripts/bin/proxysql-tester.py @@ -908,9 +908,6 @@ def disk_usage(): self.padmin_command(f"LOGENTRY '{TAP} test {fo_num+1}/{len(tap_tests)} \'{os.path.basename(fo_cmd)}\' RC: {fop.returncode}'") self.padmin_command(f"PROXYSQL FLUSH LOGS") - # Dump gcov counters for coverage collection - if self.coverage: - self.padmin_command("PROXYSQL GCOV DUMP") log.debug(f"{TAP} test {fo_num+1}/{len(tap_tests)} '{os.path.basename(fo_cmd)}' RC: {fop.returncode}") # if returncode print extra info @@ -1920,4 +1917,3 @@ def main(argv): if __name__ == '__main__': main(sys.argv[1:]) -