From 28f1819cf9b254c6f9b8e168542cf0fe67ed748d Mon Sep 17 00:00:00 2001 From: bladehan1 Date: Mon, 20 Apr 2026 17:30:39 +0800 Subject: [PATCH 1/3] ci(coverage): gate changed-line coverage Use diff-cover changed-line coverage as the changed-code gate, keep the overall coverage delta gate at -0.1%, and keep Jacoco reports as summary-only snapshots. --- .github/workflows/pr-build.yml | 102 ++++++++++++++++++++++++--------- 1 file changed, 75 insertions(+), 27 deletions(-) diff --git a/.github/workflows/pr-build.yml b/.github/workflows/pr-build.yml index 8ef800e15f..1ba06a870d 100644 --- a/.github/workflows/pr-build.yml +++ b/.github/workflows/pr-build.yml @@ -279,6 +279,60 @@ jobs: echo "base_xmls=$BASE_XMLS" >> "$GITHUB_OUTPUT" echo "pr_xmls=$PR_XMLS" >> "$GITHUB_OUTPUT" + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.11' + + - name: Changed-line coverage (diff-cover) + id: diff-cover + env: + BASE_REF: ${{ github.event.pull_request.base.ref }} + run: | + set -euo pipefail + pip install --quiet 'diff-cover==9.2.0' + + # Ensure the base branch ref is available locally for diff-cover. + git fetch --no-tags origin "+refs/heads/${BASE_REF}:refs/remotes/origin/${BASE_REF}" + + PR_XMLS=$(find coverage/pr -name "jacocoTestReport.xml" | sort) + if [ -z "$PR_XMLS" ]; then + echo "No PR jacoco XML reports found, skipping diff-cover." + exit 0 + fi + + SRC_ROOTS=$(find . -type d -path '*/src/main/java' \ + -not -path './coverage/*' -not -path './.git/*' | sort) + + set +e + diff-cover $PR_XMLS \ + --compare-branch="origin/${BASE_REF}" \ + --src-roots $SRC_ROOTS \ + --fail-under=0 \ + --json-report=diff-cover.json \ + --markdown-report=diff-cover.md + DIFF_RC=$? + set -e + + CHANGED_LINE_COVERAGE=$(jq -r '.total_percent_covered // empty' diff-cover.json) + if [ -z "$CHANGED_LINE_COVERAGE" ]; then + echo "Unable to parse changed-line coverage from diff-cover.json." + exit 1 + fi + echo "changed_line_coverage=${CHANGED_LINE_COVERAGE}" >> "$GITHUB_OUTPUT" + + { + echo "### Changed-line Coverage (diff-cover)" + echo "" + if [ -f diff-cover.md ] && [ -s diff-cover.md ]; then + cat diff-cover.md + else + echo "_diff-cover produced no report._" + fi + } >> "$GITHUB_STEP_SUMMARY" + + echo "diff-cover exit code: ${DIFF_RC}" + - name: Aggregate base coverage id: jacoco-base uses: madrapps/jacoco-report@v1.7.2 @@ -288,6 +342,7 @@ jobs: min-coverage-overall: 0 min-coverage-changed-files: 0 skip-if-no-changes: true + comment-type: summary title: '## Base Coverage Snapshot' update-comment: false @@ -300,6 +355,7 @@ jobs: min-coverage-overall: 0 min-coverage-changed-files: 0 skip-if-no-changes: true + comment-type: summary title: '## PR Code Coverage Report' update-comment: false @@ -307,7 +363,7 @@ jobs: env: BASE_OVERALL_RAW: ${{ steps.jacoco-base.outputs.coverage-overall }} PR_OVERALL_RAW: ${{ steps.jacoco-pr.outputs.coverage-overall }} - PR_CHANGED_RAW: ${{ steps.jacoco-pr.outputs.coverage-changed-files }} + CHANGED_LINE_RAW: ${{ steps.diff-cover.outputs.changed_line_coverage }} run: | set -euo pipefail @@ -329,7 +385,7 @@ jobs: # 1) Parse metrics from jacoco-report outputs BASE_OVERALL="$(sanitize "$BASE_OVERALL_RAW")" PR_OVERALL="$(sanitize "$PR_OVERALL_RAW")" - PR_CHANGED="$(sanitize "$PR_CHANGED_RAW")" + CHANGED_LINE="$(sanitize "$CHANGED_LINE_RAW")" if ! is_number "$BASE_OVERALL" || ! is_number "$PR_OVERALL"; then echo "Failed to parse coverage values: base='${BASE_OVERALL}', pr='${PR_OVERALL}'." @@ -340,19 +396,16 @@ jobs: DELTA=$(awk -v pr="$PR_OVERALL" -v base="$BASE_OVERALL" 'BEGIN { printf "%.4f", pr - base }') DELTA_OK=$(compare_float "${DELTA} >= ${MAX_DROP}") - CHANGED_STATUS="SKIPPED (no changed coverage value)" - CHANGED_OK=1 - if [ -n "$PR_CHANGED" ] && [ "$PR_CHANGED" != "NaN" ]; then - if ! is_number "$PR_CHANGED"; then - echo "Failed to parse changed-files coverage: changed='${PR_CHANGED}'." - exit 1 - fi - CHANGED_OK=$(compare_float "${PR_CHANGED} > ${MIN_CHANGED}") - if [ "$CHANGED_OK" -eq 1 ]; then - CHANGED_STATUS="PASS (> ${MIN_CHANGED}%)" - else - CHANGED_STATUS="FAIL (<= ${MIN_CHANGED}%)" - fi + if [ -z "$CHANGED_LINE" ] || [ "$CHANGED_LINE" = "NaN" ] || ! is_number "$CHANGED_LINE"; then + echo "Failed to parse changed-line coverage: changed-line='${CHANGED_LINE}'." + exit 1 + fi + + CHANGED_LINE_OK=$(compare_float "${CHANGED_LINE} > ${MIN_CHANGED}") + if [ "$CHANGED_LINE_OK" -eq 1 ]; then + CHANGED_LINE_STATUS="PASS (> ${MIN_CHANGED}%)" + else + CHANGED_LINE_STATUS="FAIL (<= ${MIN_CHANGED}%)" fi # 3) Output base metrics (always visible in logs + step summary) @@ -362,11 +415,11 @@ jobs: fi METRICS_TEXT=$(cat <> "$GITHUB_STEP_SUMMARY" @@ -391,14 +444,9 @@ jobs: exit 1 fi - if [ -z "$PR_CHANGED" ] || [ "$PR_CHANGED" = "NaN" ]; then - echo "No changed-files coverage value detected, skip changed-files gate." - exit 0 - fi - - if [ "$CHANGED_OK" -ne 1 ]; then - echo "Coverage gate failed: changed files coverage must be > 60%." - echo "changed=${PR_CHANGED}%" + if [ "$CHANGED_LINE_OK" -ne 1 ]; then + echo "Coverage gate failed: changed-line coverage must be > 60%." + echo "changed-line=${CHANGED_LINE}%" exit 1 fi From ece6e0930fa57ecad17c047c3f66a582522dac01 Mon Sep 17 00:00:00 2001 From: bladehan1 Date: Fri, 24 Apr 2026 11:56:44 +0800 Subject: [PATCH 2/3] ci(coverage): harden diff-cover step and handle non-Java PRs - Drop the unreachable PR_XMLS empty-check; collect-xml already exits 1 when jacoco reports are missing, and the previous `exit 0` would have left changed_line_coverage unset and triggered a misleading "Failed to parse changed-line coverage" in the enforcement step. - Parse total_num_lines from diff-cover.json: when a PR has no changed Java source lines (workflow-only, docs-only, proto-only), emit the sentinel NA and treat it as SKIPPED in the enforcement step so the gate does not wrongly fail. - Display NA without a trailing %% in the step summary and logs. Addresses review comments from CodeRabbit and cubic on PR #23. --- .github/workflows/pr-build.yml | 48 ++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/.github/workflows/pr-build.yml b/.github/workflows/pr-build.yml index 1ba06a870d..5c71aba493 100644 --- a/.github/workflows/pr-build.yml +++ b/.github/workflows/pr-build.yml @@ -296,11 +296,6 @@ jobs: git fetch --no-tags origin "+refs/heads/${BASE_REF}:refs/remotes/origin/${BASE_REF}" PR_XMLS=$(find coverage/pr -name "jacocoTestReport.xml" | sort) - if [ -z "$PR_XMLS" ]; then - echo "No PR jacoco XML reports found, skipping diff-cover." - exit 0 - fi - SRC_ROOTS=$(find . -type d -path '*/src/main/java' \ -not -path './coverage/*' -not -path './.git/*' | sort) @@ -314,12 +309,18 @@ jobs: DIFF_RC=$? set -e - CHANGED_LINE_COVERAGE=$(jq -r '.total_percent_covered // empty' diff-cover.json) - if [ -z "$CHANGED_LINE_COVERAGE" ]; then - echo "Unable to parse changed-line coverage from diff-cover.json." - exit 1 + TOTAL_NUM_LINES=$(jq -r '.total_num_lines // 0' diff-cover.json) + if [ "${TOTAL_NUM_LINES}" = "0" ]; then + echo "No changed Java source lines; skipping changed-line gate." + echo "changed_line_coverage=NA" >> "$GITHUB_OUTPUT" + else + CHANGED_LINE_COVERAGE=$(jq -r '.total_percent_covered // empty' diff-cover.json) + if [ -z "$CHANGED_LINE_COVERAGE" ]; then + echo "Unable to parse changed-line coverage from diff-cover.json." + exit 1 + fi + echo "changed_line_coverage=${CHANGED_LINE_COVERAGE}" >> "$GITHUB_OUTPUT" fi - echo "changed_line_coverage=${CHANGED_LINE_COVERAGE}" >> "$GITHUB_OUTPUT" { echo "### Changed-line Coverage (diff-cover)" @@ -396,16 +397,19 @@ jobs: DELTA=$(awk -v pr="$PR_OVERALL" -v base="$BASE_OVERALL" 'BEGIN { printf "%.4f", pr - base }') DELTA_OK=$(compare_float "${DELTA} >= ${MAX_DROP}") - if [ -z "$CHANGED_LINE" ] || [ "$CHANGED_LINE" = "NaN" ] || ! is_number "$CHANGED_LINE"; then + if [ "$CHANGED_LINE" = "NA" ]; then + CHANGED_LINE_OK=1 + CHANGED_LINE_STATUS="SKIPPED (no changed Java source lines)" + elif [ -z "$CHANGED_LINE" ] || [ "$CHANGED_LINE" = "NaN" ] || ! is_number "$CHANGED_LINE"; then echo "Failed to parse changed-line coverage: changed-line='${CHANGED_LINE}'." exit 1 - fi - - CHANGED_LINE_OK=$(compare_float "${CHANGED_LINE} > ${MIN_CHANGED}") - if [ "$CHANGED_LINE_OK" -eq 1 ]; then - CHANGED_LINE_STATUS="PASS (> ${MIN_CHANGED}%)" else - CHANGED_LINE_STATUS="FAIL (<= ${MIN_CHANGED}%)" + CHANGED_LINE_OK=$(compare_float "${CHANGED_LINE} > ${MIN_CHANGED}") + if [ "$CHANGED_LINE_OK" -eq 1 ]; then + CHANGED_LINE_STATUS="PASS (> ${MIN_CHANGED}%)" + else + CHANGED_LINE_STATUS="FAIL (<= ${MIN_CHANGED}%)" + fi fi # 3) Output base metrics (always visible in logs + step summary) @@ -414,8 +418,14 @@ jobs: OVERALL_STATUS="FAIL (< ${MAX_DROP}%)" fi + if [ "$CHANGED_LINE" = "NA" ]; then + CHANGED_LINE_DISPLAY="NA" + else + CHANGED_LINE_DISPLAY="${CHANGED_LINE}%" + fi + METRICS_TEXT=$(cat < Date: Fri, 24 Apr 2026 17:17:26 +0800 Subject: [PATCH 3/3] ci(coverage): polish diff-cover step and clarify metric semantics - Guard against empty SRC_ROOTS before invoking diff-cover; exits with a readable error instead of letting --fail-under be consumed as a src-root value. - Verify that diff-cover actually wrote diff-cover.json; surface the exit code in the failure message so a runner/binary crash is diagnosable without hunting through logs. - Drop the trailing "diff-cover exit code" log; --fail-under=0 means DIFF_RC is always 0 in the happy path, so the line carries no signal. - Clarify the metrics panel: Changed-line is LINE coverage (diff-cover) while Overall / Delta are INSTRUCTION coverage (jacoco-report). Add a footnote to both the console output and the step summary so readers do not silently compare the two counters. --- .github/workflows/pr-build.yml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-build.yml b/.github/workflows/pr-build.yml index 5c71aba493..249bcaf28f 100644 --- a/.github/workflows/pr-build.yml +++ b/.github/workflows/pr-build.yml @@ -298,6 +298,10 @@ jobs: PR_XMLS=$(find coverage/pr -name "jacocoTestReport.xml" | sort) SRC_ROOTS=$(find . -type d -path '*/src/main/java' \ -not -path './coverage/*' -not -path './.git/*' | sort) + if [ -z "$SRC_ROOTS" ]; then + echo "No src/main/java directories found; cannot run diff-cover." >&2 + exit 1 + fi set +e diff-cover $PR_XMLS \ @@ -309,6 +313,11 @@ jobs: DIFF_RC=$? set -e + if [ ! -f diff-cover.json ]; then + echo "diff-cover did not produce JSON report (exit=${DIFF_RC})." >&2 + exit 1 + fi + TOTAL_NUM_LINES=$(jq -r '.total_num_lines // 0' diff-cover.json) if [ "${TOTAL_NUM_LINES}" = "0" ]; then echo "No changed Java source lines; skipping changed-line gate." @@ -332,8 +341,6 @@ jobs: fi } >> "$GITHUB_STEP_SUMMARY" - echo "diff-cover exit code: ${DIFF_RC}" - - name: Aggregate base coverage id: jacoco-base uses: madrapps/jacoco-report@v1.7.2 @@ -431,6 +438,7 @@ jobs: Delta (PR - Base): ${DELTA}% Changed-line Gate: ${CHANGED_LINE_STATUS} Overall Delta Gate: ${OVERALL_STATUS} + Note: Changed-line uses LINE coverage (diff-cover); Overall/Delta use INSTRUCTION coverage (jacoco-report). The two counters are not directly comparable. EOF ) @@ -445,6 +453,8 @@ jobs: echo "- Delta (PR - Base): ${DELTA}%" echo "- Changed-line Gate: ${CHANGED_LINE_STATUS}" echo "- Overall Delta Gate: ${OVERALL_STATUS}" + echo "" + echo "_Note: Changed-line uses LINE coverage (diff-cover); Overall/Delta use INSTRUCTION coverage (jacoco-report). The two counters are not directly comparable._" } >> "$GITHUB_STEP_SUMMARY" # 4) Decide CI pass/fail