From 6d45087b010116a7f59d7c856defaac8fc4eee7b Mon Sep 17 00:00:00 2001 From: monsieurleberre Date: Sat, 11 Jul 2026 23:30:54 +0200 Subject: [PATCH] fix(badges): surface git-fetch errors and guard malformed badge entries write-badges.sh swallowed all git-fetch failures via '|| true' and fell through to the orphan-rebuild path, and had no guard for malformed matrix entries (which would write ci-null-null-null.json). Detect an absent badge branch up front with 'git ls-remote --exit-code' so only that benign case rebuilds the orphan; let real fetch failures abort. Reject non-array coverage-data/matrix-data and entries missing required fields with a loud ::error::. Add test/write-badges_test.sh pinning the filename contract, null-percent skip, and fail-loud paths, wired into build-and-test. Closes #24 --- .github/workflows/build-and-test.yaml | 1 + CHANGELOG.md | 4 + scripts/write-badges.sh | 37 ++++- test/write-badges_test.sh | 193 ++++++++++++++++++++++++++ 4 files changed, 232 insertions(+), 3 deletions(-) create mode 100755 test/write-badges_test.sh diff --git a/.github/workflows/build-and-test.yaml b/.github/workflows/build-and-test.yaml index e3a6d6f..c970a12 100644 --- a/.github/workflows/build-and-test.yaml +++ b/.github/workflows/build-and-test.yaml @@ -79,6 +79,7 @@ jobs: bash test/push-nuget_test.sh bash test/normalize-ci-matrix_test.sh bash test/install-dotnet-coverage_test.sh + bash test/write-badges_test.sh - name: Run python tests run: python3 -m unittest discover -s test -p '*_test.py' diff --git a/CHANGELOG.md b/CHANGELOG.md index e8e87d2..1311419 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Harden `scripts/write-badges.sh` (the `update-badges.yaml` helper) against silent failures. `git fetch origin "$badge_branch" || true` previously swallowed every failure — auth, network, or a genuinely absent branch alike — then fell through to the orphan-rebuild path; a real fetch failure is now aborted loud, and only the benign "branch does not exist yet" case (detected up front with `git ls-remote --exit-code`) rebuilds the orphan branch. Malformed input now fails the badge job instead of writing a bogus badge file: a non-array `coverage-data`/`matrix-data`, a coverage entry missing `slug`/`label`, or a matrix entry missing `lang`/`os`/`arch`/`passed` (which would have written `ci-null-null-null.json`) is rejected with a `::error::` annotation. Added `test/write-badges_test.sh` pinning the `coverage-.json` / `ci---.json` filename contract, the null-percent skip, and the new fail-loud paths, wired into the `build-and-test` self-test job. Healthy consumers on the built-in `matrix-status` → `update-badges.yaml` flow see no change. (#24) + ## [2.3.4] - 2026-07-10 ### Fixed diff --git a/scripts/write-badges.sh b/scripts/write-badges.sh index 36e6021..8a79265 100755 --- a/scripts/write-badges.sh +++ b/scripts/write-badges.sh @@ -9,10 +9,35 @@ badge_branch="${BADGE_BRANCH:?BADGE_BRANCH is required}" script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +require_json_array() { + local source_name="$1" value="$2" + if ! jq -e 'type == "array"' <<<"$value" >/dev/null 2>&1; then + echo "::error::$source_name is not a JSON array: $value" >&2 + exit 1 + fi +} + +require_field() { + local source_name="$1" entry="$2" field_name="$3" field_value="$4" + if [ -z "$field_value" ] || [ "$field_value" = "null" ]; then + echo "::error::$source_name entry is missing '$field_name': $entry" >&2 + exit 1 + fi +} + +require_json_array COVERAGE_DATA "$coverage_data" +require_json_array MATRIX_DATA "$matrix_data" + git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" -git fetch origin "$badge_branch" || true +badge_ref="refs/heads/$badge_branch" +ls_remote_rc=0 +git ls-remote --exit-code origin "$badge_ref" >/dev/null || ls_remote_rc=$? +if [ "$ls_remote_rc" -ne 0 ] && [ "$ls_remote_rc" -ne 2 ]; then + echo "::error::git ls-remote for $badge_ref failed with exit code $ls_remote_rc" >&2 + exit "$ls_remote_rc" +fi worktree_dir="$(mktemp -d)" cleanup() { @@ -21,12 +46,12 @@ cleanup() { } trap cleanup EXIT -if git rev-parse --verify "origin/$badge_branch" >/dev/null 2>&1; then +if [ "$ls_remote_rc" -eq 0 ]; then + git fetch origin "+$badge_ref:refs/remotes/origin/$badge_branch" git worktree add "$worktree_dir" "origin/$badge_branch" git -C "$worktree_dir" checkout -B "$badge_branch" else git worktree add --orphan -b "$badge_branch" "$worktree_dir" - git -C "$worktree_dir" rm -rf . 2>/dev/null || true fi while IFS= read -r entry; do @@ -34,6 +59,8 @@ while IFS= read -r entry; do slug="$(jq -r '.slug' <<<"$entry")" label="$(jq -r '.label' <<<"$entry")" percent="$(jq -r '.percent' <<<"$entry")" + require_field coverage-data "$entry" slug "$slug" + require_field coverage-data "$entry" label "$label" if [ -z "$percent" ] || [ "$percent" = "null" ]; then continue fi @@ -46,6 +73,10 @@ while IFS= read -r entry; do os_name="$(jq -r '.os' <<<"$entry")" arch="$(jq -r '.arch' <<<"$entry")" passed="$(jq -r '.passed' <<<"$entry")" + require_field matrix-data "$entry" lang "$lang" + require_field matrix-data "$entry" os "$os_name" + require_field matrix-data "$entry" arch "$arch" + require_field matrix-data "$entry" passed "$passed" python3 "$script_dir/matrix_badge_json.py" "$os_name" "$arch" "$passed" > "$worktree_dir/ci-$lang-$os_name-$arch.json" done < <(jq -c '.[]' <<<"$matrix_data") diff --git a/test/write-badges_test.sh b/test/write-badges_test.sh new file mode 100755 index 0000000..f61a529 --- /dev/null +++ b/test/write-badges_test.sh @@ -0,0 +1,193 @@ +#!/usr/bin/env bash +# Copyright (c) 2026 Peaceful Studio OÜ +# SPDX-License-Identifier: Apache-2.0 +set -euo pipefail + +here="$(cd "$(dirname "$0")" && pwd)" +script="$here/../scripts/write-badges.sh" + +export GIT_CONFIG_GLOBAL=/dev/null +export GIT_CONFIG_SYSTEM=/dev/null + +fail=0 +check() { + local name="$1" expected="$2" actual="$3" + if [ "$expected" = "$actual" ]; then + echo "ok - $name" + else + echo "FAIL - $name: expected [$expected] got [$actual]" + fail=1 + fi +} + +check_rc() { + local name="$1" expected="$2" actual="$3" + if [ "$expected" = "$actual" ]; then + echo "ok - $name" + else + echo "FAIL - $name: expected rc [$expected] got [$actual]" + fail=1 + fi +} + +check_contains() { + local name="$1" needle="$2" hay="$3" + case "$hay" in + *"$needle"*) echo "ok - $name" ;; + *) echo "FAIL - $name: [$hay] does not contain [$needle]"; fail=1 ;; + esac +} + +sandbox="" +origin="" +work="" + +setup_repos() { + sandbox="$(mktemp -d)" + origin="$sandbox/origin.git" + work="$sandbox/work" + git init -q --bare "$origin" + git init -q -b main "$work" + git -C "$work" config user.name test + git -C "$work" config user.email test@example.com + git -C "$work" remote add origin "$origin" + git -C "$work" commit -q --allow-empty -m init + git -C "$work" push -q origin main +} + +seed_remote_badge_branch() { + local branch="$1" filename="$2" + local seed="$sandbox/seed" + git clone -q "$origin" "$seed" + git -C "$seed" config user.name test + git -C "$seed" config user.email test@example.com + git -C "$seed" checkout -q --orphan "$branch" + git -C "$seed" rm -r -f -q --ignore-unmatch . + echo '{"seed":true}' > "$seed/$filename" + git -C "$seed" add "$filename" + git -C "$seed" commit -qm seed + git -C "$seed" push -q origin "$branch" + rm -rf "$seed" +} + +badge_branch_files() { + local branch="$1" + git -C "$origin" ls-tree --name-only "refs/heads/$branch" 2>/dev/null | sort | tr '\n' ' ' | sed 's/ $//' +} + +run() { + local coverage="$1" matrix="$2" branch="$3" + local errf; errf="$(mktemp)" + set +e + out="$(cd "$work" && COVERAGE_DATA="$coverage" MATRIX_DATA="$matrix" BADGE_BRANCH="$branch" bash "$script" 2>"$errf")" + rc=$? + set -e + err="$(cat "$errf")"; rm -f "$errf" +} + +setup_repos +run '[{"slug":"csharp","label":"C#","percent":91}]' \ + '[{"lang":"csharp","os":"ubuntu","arch":"x64","passed":true}]' badges +check_rc "fresh repo happy path exit 0" 0 "$rc" +check "fresh repo writes contract filenames" \ + "ci-csharp-ubuntu-x64.json coverage-csharp.json" "$(badge_branch_files badges)" +check_contains "coverage badge content on branch" '"message": "91%"' \ + "$(git -C "$origin" show badges:coverage-csharp.json)" +check_contains "matrix badge content on branch" '"message": "passing"' \ + "$(git -C "$origin" show badges:ci-csharp-ubuntu-x64.json)" +rm -rf "$sandbox" + +setup_repos +run '[{"slug":"csharp","label":"C#","percent":null},{"slug":"go","label":"Go","percent":""}]' '[]' badges +check_rc "all coverage percents null exit 0" 0 "$rc" +check_contains "all coverage percents null skips commit" "badges unchanged; skipping commit" "$out" +check "all coverage percents null pushes nothing" "" "$(badge_branch_files badges)" +rm -rf "$sandbox" + +setup_repos +run '[{"slug":"csharp","label":"C#","percent":null},{"slug":"go","label":"Go","percent":87}]' '[]' badges +check_rc "mixed null percent exit 0" 0 "$rc" +check "mixed null percent writes only the non-null badge" \ + "coverage-go.json" "$(badge_branch_files badges)" +rm -rf "$sandbox" + +setup_repos +run '[{"label":"C#","percent":91}]' '[]' badges +check_rc "coverage entry missing slug exit 1" 1 "$rc" +check_contains "coverage entry missing slug annotated" \ + "::error::coverage-data entry is missing 'slug'" "$err" +rm -rf "$sandbox" + +setup_repos +run '[]' '[{"os":"ubuntu","arch":"x64","passed":true}]' badges +check_rc "matrix entry missing lang exit 1" 1 "$rc" +check_contains "matrix entry missing lang annotated" \ + "::error::matrix-data entry is missing 'lang'" "$err" +check "matrix entry missing lang pushes nothing" "" "$(badge_branch_files badges)" +rm -rf "$sandbox" + +setup_repos +run '[]' '[{"lang":"csharp","os":null,"arch":"x64","passed":true}]' badges +check_rc "matrix entry null os exit 1" 1 "$rc" +check_contains "matrix entry null os annotated" \ + "::error::matrix-data entry is missing 'os'" "$err" +rm -rf "$sandbox" + +setup_repos +run '[]' '[{"lang":"csharp","os":"ubuntu","arch":"x64"}]' badges +check_rc "matrix entry missing passed exit 1" 1 "$rc" +check_contains "matrix entry missing passed annotated" \ + "::error::matrix-data entry is missing 'passed'" "$err" +rm -rf "$sandbox" + +setup_repos +run '[]' '[{"lang":"csharp","os":"ubuntu","arch":"x64","passed":"cancelled"}]' badges +check_rc "matrix entry unknown status exit 2" 2 "$rc" +check_contains "matrix entry unknown status surfaced" "unknown matrix status" "$err" +rm -rf "$sandbox" + +setup_repos +run '[bad' '[]' badges +check_rc "malformed coverage-data json exit 1" 1 "$rc" +check_contains "malformed coverage-data annotated" \ + "::error::COVERAGE_DATA is not a JSON array" "$err" +rm -rf "$sandbox" + +setup_repos +run '[]' '{"lang":"csharp"}' badges +check_rc "non-array matrix-data exit 1" 1 "$rc" +check_contains "non-array matrix-data annotated" \ + "::error::MATRIX_DATA is not a JSON array" "$err" +rm -rf "$sandbox" + +setup_repos +seed_remote_badge_branch badges coverage-csharp.json +run '[{"slug":"csharp","label":"C#","percent":42}]' '[]' badges +check_rc "existing badge branch exit 0" 0 "$rc" +check "existing badge branch keeps single file" \ + "coverage-csharp.json" "$(badge_branch_files badges)" +check_contains "existing badge branch content updated" '"message": "42%"' \ + "$(git -C "$origin" show badges:coverage-csharp.json)" +check "existing badge branch fast-forwards to two commits" 2 \ + "$(git -C "$origin" rev-list --count refs/heads/badges)" +rm -rf "$sandbox" + +setup_repos +seed_remote_badge_branch badges coverage-csharp.json +git -C "$work" remote set-url origin "$sandbox/does-not-exist.git" +run '[{"slug":"csharp","label":"C#","percent":42}]' '[]' badges +check_rc "unreachable origin exit non-zero" 128 "$rc" +check_contains "unreachable origin annotated" \ + "::error::git ls-remote for refs/heads/badges failed with exit code 128" "$err" +rm -rf "$sandbox" + +setup_repos +run '[{"slug":"csharp","label":"C#","percent":91}]' '[]' badges +run '[{"slug":"csharp","label":"C#","percent":91}]' '[]' badges +check_rc "unchanged rerun exit 0" 0 "$rc" +check_contains "unchanged rerun skips commit" "badges unchanged; skipping commit" "$out" +check "unchanged rerun keeps single commit" 1 \ + "$(git -C "$origin" rev-list --count refs/heads/badges)" +rm -rf "$sandbox" + +exit $fail