fix(ci): use matching GCOV for TAP coverage #6062
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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}" | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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}" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Align the validator with the intended GCOV contract.
This step runs
validate-coverage-gcov-toolchain.bash, which rejectsgcc-11in the collector image and rejects rawfastcov -b -g gcov-*calls. The PR objective requires installing GCC 11 and usinggcov-11for raw conversion. Align the validator and implementation before enabling this CI gate, or CI will reject the intended configuration.🤖 Prompt for AI Agents