From 584c4053b966bb9c8111ef8e68dd1526c572deed Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Mon, 24 Aug 2026 21:34:56 +0200 Subject: [PATCH 1/2] ci: make the GNU testsuite run and its PR comment resilient GnuTests on #262 reported "PASSED: -8 / FAILED: +12" although the branch changes nothing for the testsuite. --- .github/workflows/GnuTests.yml | 40 ++++++++++++++++++--- .gitignore | 1 + tests/run-upstream-testsuite.sh | 62 +++++++++++++++++++++++++-------- util/compare_test_results.py | 8 +++-- 4 files changed, 89 insertions(+), 22 deletions(-) diff --git a/.github/workflows/GnuTests.yml b/.github/workflows/GnuTests.yml index 1a7cd949..a853a8e5 100644 --- a/.github/workflows/GnuTests.yml +++ b/.github/workflows/GnuTests.yml @@ -64,7 +64,16 @@ jobs: shell: bash run: | ## Run GNU diffutils testsuite - ./tests/run-upstream-testsuite.sh release || true + # Exit code 1 means some tests failed, which is expected and handled + # by the comparison below; 2 means the suite couldn't be run at all + # 'shell: bash' runs with -e, so don't let the expected exit code 1 + # abort the step before the exit code is inspected + result=0 + ./tests/run-upstream-testsuite.sh release || result=$? + if [[ $result -ge 2 ]]; then + echo "::error ::The GNU testsuite could not be run (exit code $result); see the log above" + exit 1 + fi env: TERM: xterm @@ -128,12 +137,22 @@ jobs: exit 1 fi + if ! jq -e . "$RESULT_FILE" > /dev/null; then + echo "::error ::Test results at $RESULT_FILE are not valid JSON" + exit 1 + fi + TOTAL=$(jq '[.tests[]] | length' "$RESULT_FILE") PASS=$(jq '[.tests[] | select(.result=="PASS")] | length' "$RESULT_FILE") FAIL=$(jq '[.tests[] | select(.result=="FAIL")] | length' "$RESULT_FILE") SKIP=$(jq '[.tests[] | select(.result=="SKIP")] | length' "$RESULT_FILE") ERROR=0 + if [[ "$TOTAL" -eq 0 ]]; then + echo "::error ::No test was run; refusing to report or compare an empty test run" + exit 1 + fi + output="GNU diffutils tests summary = TOTAL: $TOTAL / PASS: $PASS / FAIL: $FAIL / SKIP: $SKIP" echo "${output}" @@ -175,14 +194,20 @@ jobs: IGNORE_INTERMITTENT=".github/workflows/ignore-intermittent.txt" - COMMENT_DIR="reference/comment" + # Build the comment in a directory of its own: 'reference/' holds the + # artifacts downloaded from the reference run, including its own + # 'comment' artifact, and re-uploading those would post a comparison + # belonging to another run (see https://github.com/uutils/diffutils/pull/262) + COMMENT_DIR="comment" + rm -rf ${COMMENT_DIR} mkdir -p ${COMMENT_DIR} echo ${{ github.event.number }} > ${COMMENT_DIR}/NR COMMENT_LOG="${COMMENT_DIR}/result.txt" + : > "${COMMENT_LOG}" COMPARISON_RESULT=0 if test -f "${CURRENT_SUMMARY_FILE}"; then - if test -f "${REF_SUMMARY_FILE}"; then + if test -s "${REF_SUMMARY_FILE}"; then echo "Reference summary SHA1/ID: $(sha1sum -- "${REF_SUMMARY_FILE}")" echo "Current summary SHA1/ID: $(sha1sum -- "${CURRENT_SUMMARY_FILE}")" @@ -193,7 +218,7 @@ jobs: COMPARISON_RESULT=$? else - echo "::warning ::Skipping test comparison; no prior reference summary is available at '${REF_SUMMARY_FILE}'." + echo "::warning ::Skipping test comparison; no usable reference summary is available at '${REF_SUMMARY_FILE}'." fi else echo "::error ::Failed to find summary of test results (missing '${CURRENT_SUMMARY_FILE}'); failing early" @@ -203,6 +228,11 @@ jobs: if [ ${COMPARISON_RESULT} -eq 1 ]; then echo "::error ::Found new non-intermittent test failures" exit 1 + elif [ ${COMPARISON_RESULT} -ge 2 ]; then + # The comparison itself failed (e.g. an unusable reference summary). + # Don't post a comment rather than post a misleading one. + : > "${COMMENT_LOG}" + echo "::warning ::Could not compare the test results against the reference" else echo "::notice ::No new test failures detected" fi @@ -212,7 +242,7 @@ jobs: uses: actions/upload-artifact@v4 with: name: comment - path: reference/comment/ + path: comment/ - name: Report test results if: success() || failure() diff --git a/.gitignore b/.gitignore index e9868bd2..515a64d4 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ /target *.swp +/tests/test-results.json diff --git a/tests/run-upstream-testsuite.sh b/tests/run-upstream-testsuite.sh index c73cfe81..0bc09ae3 100755 --- a/tests/run-upstream-testsuite.sh +++ b/tests/run-upstream-testsuite.sh @@ -16,6 +16,11 @@ # tests are run might not match exactly that used when the upstream tests are # run through the autotools. +# Exit codes: 0 if all tests passed, 1 if at least one test failed, and 2 if +# the test suite could not be run at all (e.g. the upstream repository couldn't +# be fetched). Callers must treat 2 as an infrastructure error: no meaningful +# result was produced. + # By default it expects a release build of the diffutils binary, but a # different build profile can be specified as an argument # (e.g. 'dev' or 'test'). @@ -26,6 +31,12 @@ scriptpath=$(dirname "$(readlink -f "$0")") rev=$(git rev-parse HEAD) +# Report an infrastructure error: the test suite could not be run at all +die() { + echo "ERROR: $*" >&2 + exit 2 +} + # Allow passing a specific profile as parameter (default to "release") profile="release" [[ -n $1 ]] && profile="$1" @@ -34,35 +45,47 @@ profile="release" binary="$scriptpath/../target/$profile/diffutils" if [[ ! -x "$binary" ]] then - echo "Missing build for profile $profile" - exit 1 + die "Missing build for profile $profile" fi # Work in a temporary directory tempdir=$(mktemp -d) -cd "$tempdir" +trap 'rm -rf "$tempdir"' EXIT +cd "$tempdir" || die "Cannot enter temporary directory $tempdir" -# Check out the upstream test suite +# Check out the upstream test suite. git.savannah.gnu.org is regularly +# unavailable or slow, so retry a few times before giving up. gitserver="https://git.savannah.gnu.org" testsuite="$gitserver/git/diffutils.git" -echo "Fetching upstream test suite from $testsuite" -git clone -n --depth=1 --filter=tree:0 "$testsuite" &> /dev/null -cd diffutils -git sparse-checkout set --no-cone tests &> /dev/null -git checkout &> /dev/null +attempts=3 +for (( attempt = 1; attempt <= attempts; attempt++ )) +do + echo "Fetching upstream test suite from $testsuite (attempt $attempt/$attempts)" + rm -rf diffutils + git clone -n --depth=1 --filter=tree:0 "$testsuite" && break + (( attempt < attempts )) && sleep $(( attempt * 10 )) +done +[[ -d diffutils ]] || die "Failed to fetch the upstream test suite from $testsuite" +cd diffutils || die "Failed to fetch the upstream test suite from $testsuite" +git sparse-checkout set --no-cone tests &> /dev/null || die "Cannot sparse-checkout the upstream tests" +git checkout &> /dev/null || die "Cannot check out the upstream tests" upstreamrev=$(git rev-parse HEAD) +[[ -d tests ]] || die "The upstream checkout contains no tests directory" # Ensure that calling `diff` invokes the built `diffutils` binary instead of # the upstream `diff` binary that is most likely installed on the system mkdir src -cd src +cd src || die "Cannot create the directory holding the diff and cmp symlinks" ln -s "$binary" diff ln -s "$binary" cmp -cd ../tests +cd ../tests || die "Cannot enter the upstream tests directory" # Fetch tests/init.sh from the gnulib repository (needed since # https://git.savannah.gnu.org/cgit/diffutils.git/commit/tests?id=1d2456f539) -curl -sL "$gitserver/gitweb/?p=gnulib.git;a=blob_plain;f=tests/init.sh;hb=HEAD" -o init.sh +curl -sL --fail --retry 3 --retry-delay 5 --retry-all-errors \ + "$gitserver/gitweb/?p=gnulib.git;a=blob_plain;f=tests/init.sh;hb=HEAD" -o init.sh \ + || die "Failed to fetch tests/init.sh from the gnulib repository" +[[ -s init.sh ]] || die "Fetched an empty tests/init.sh from the gnulib repository" if [[ -n "$TESTS" ]] then @@ -73,6 +96,7 @@ else tests=$(make -f Makefile.am printtests) fi total=$(echo "$tests" | wc -w) +(( total > 0 )) || die "No test to run: the upstream test list is empty" echo "Running $total tests" export LC_ALL=C export KEEP=yes @@ -105,7 +129,9 @@ do # but there isn't much value added in doing so for file in * do - [[ -f "$file" ]] && json+="\"$file\":\"$(base64 -w0 < "$file")\"," + # Encode the name with jq: some tests create files whose name contains + # quotes or control characters, which would produce invalid JSON + [[ -f "$file" ]] && json+="$(jq -Rn --arg name "$file" '$name'):\"$(base64 -w0 < "$file")\"," done json="${json%,}}}," cd - > /dev/null @@ -144,10 +170,16 @@ json="{$metadata $json}" # Clean up cd "$scriptpath" -rm -rf "$tempdir" +# Write the results out only once they are known to be valid JSON, so that a +# malformed (or truncated) file is never left behind for the caller to consume resultsfile="test-results.json" -echo "$json" | jq > "$resultsfile" +if ! echo "$json" | jq > "$resultsfile.tmp" +then + rm -f "$resultsfile.tmp" + die "Generated invalid JSON results" +fi +mv "$resultsfile.tmp" "$resultsfile" echo "Results written to $scriptpath/$resultsfile" (( failed > 0 )) && exit 1 diff --git a/util/compare_test_results.py b/util/compare_test_results.py index 06257827..00469d6e 100755 --- a/util/compare_test_results.py +++ b/util/compare_test_results.py @@ -4,6 +4,10 @@ Compare the current GNU test results to the last results gathered from the main branch to highlight if a PR is making the results better/worse. Don't exit with error code if all failing tests are in the ignore-intermittent.txt list. + +Exit status: 0 if the comparison shows no new failure, 1 if new non-intermittent +failures appeared, and 2 if the comparison couldn't be performed at all (e.g. one +of the result files is missing or malformed). """ import json @@ -49,7 +53,7 @@ def compare_results(current_file, reference_file, ignore_file=None, output_file= current_summary, current_failed = extract_test_results(current_data) except Exception as e: print(f"Error loading current results: {e}") - return 1 + return 2 try: with open(reference_file, "r") as f: @@ -57,7 +61,7 @@ def compare_results(current_file, reference_file, ignore_file=None, output_file= reference_summary, reference_failed = extract_test_results(reference_data) except Exception as e: print(f"Error loading reference results: {e}") - return 1 + return 2 # Calculate differences pass_diff = int(current_summary.get("passed", 0)) - int(reference_summary.get("passed", 0)) From ef2db021133a3b1b2d0791c3b18885127e91f913 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Mon, 24 Aug 2026 21:44:19 +0200 Subject: [PATCH 2/2] ci: fall back to the gnulib GitHub mirror for tests/init.sh The savannah gitweb interface is regularly rate-limited or unavailable, so retry it and then fall back to the official gnulib mirror on GitHub rather than failing the whole GNU testsuite job. --- tests/run-upstream-testsuite.sh | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/run-upstream-testsuite.sh b/tests/run-upstream-testsuite.sh index 0bc09ae3..78f1c405 100755 --- a/tests/run-upstream-testsuite.sh +++ b/tests/run-upstream-testsuite.sh @@ -82,10 +82,20 @@ cd ../tests || die "Cannot enter the upstream tests directory" # Fetch tests/init.sh from the gnulib repository (needed since # https://git.savannah.gnu.org/cgit/diffutils.git/commit/tests?id=1d2456f539) -curl -sL --fail --retry 3 --retry-delay 5 --retry-all-errors \ - "$gitserver/gitweb/?p=gnulib.git;a=blob_plain;f=tests/init.sh;hb=HEAD" -o init.sh \ - || die "Failed to fetch tests/init.sh from the gnulib repository" -[[ -s init.sh ]] || die "Fetched an empty tests/init.sh from the gnulib repository" +# The savannah gitweb interface is often rate-limited or unavailable, so fall +# back to the official gnulib mirror on GitHub +initsh_urls=( + "$gitserver/gitweb/?p=gnulib.git;a=blob_plain;f=tests/init.sh;hb=HEAD" + "https://raw.githubusercontent.com/coreutils/gnulib/master/tests/init.sh" +) +for url in "${initsh_urls[@]}" +do + echo "Fetching tests/init.sh from $url" + curl -sSL --fail --retry 3 --retry-delay 5 --retry-all-errors \ + --connect-timeout 30 --max-time 300 "$url" -o init.sh && [[ -s init.sh ]] && break + rm -f init.sh +done +[[ -s init.sh ]] || die "Failed to fetch tests/init.sh from the gnulib repository" if [[ -n "$TESTS" ]] then