Skip to content

Commit 7de61c5

Browse files
committed
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.
1 parent 3d34e50 commit 7de61c5

4 files changed

Lines changed: 89 additions & 22 deletions

File tree

‎.github/workflows/GnuTests.yml‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,16 @@ jobs:
6464
shell: bash
6565
run: |
6666
## Run GNU diffutils testsuite
67-
./tests/run-upstream-testsuite.sh release || true
67+
# Exit code 1 means some tests failed, which is expected and handled
68+
# by the comparison below; 2 means the suite couldn't be run at all
69+
# 'shell: bash' runs with -e, so don't let the expected exit code 1
70+
# abort the step before the exit code is inspected
71+
result=0
72+
./tests/run-upstream-testsuite.sh release || result=$?
73+
if [[ $result -ge 2 ]]; then
74+
echo "::error ::The GNU testsuite could not be run (exit code $result); see the log above"
75+
exit 1
76+
fi
6877
env:
6978
TERM: xterm
7079

@@ -128,12 +137,22 @@ jobs:
128137
exit 1
129138
fi
130139
140+
if ! jq -e . "$RESULT_FILE" > /dev/null; then
141+
echo "::error ::Test results at $RESULT_FILE are not valid JSON"
142+
exit 1
143+
fi
144+
131145
TOTAL=$(jq '[.tests[]] | length' "$RESULT_FILE")
132146
PASS=$(jq '[.tests[] | select(.result=="PASS")] | length' "$RESULT_FILE")
133147
FAIL=$(jq '[.tests[] | select(.result=="FAIL")] | length' "$RESULT_FILE")
134148
SKIP=$(jq '[.tests[] | select(.result=="SKIP")] | length' "$RESULT_FILE")
135149
ERROR=0
136150
151+
if [[ "$TOTAL" -eq 0 ]]; then
152+
echo "::error ::No test was run; refusing to report or compare an empty test run"
153+
exit 1
154+
fi
155+
137156
output="GNU diffutils tests summary = TOTAL: $TOTAL / PASS: $PASS / FAIL: $FAIL / SKIP: $SKIP"
138157
echo "${output}"
139158
@@ -175,14 +194,20 @@ jobs:
175194
176195
IGNORE_INTERMITTENT=".github/workflows/ignore-intermittent.txt"
177196
178-
COMMENT_DIR="reference/comment"
197+
# Build the comment in a directory of its own: 'reference/' holds the
198+
# artifacts downloaded from the reference run, including its own
199+
# 'comment' artifact, and re-uploading those would post a comparison
200+
# belonging to another run (see https://github.com/uutils/diffutils/pull/262)
201+
COMMENT_DIR="comment"
202+
rm -rf ${COMMENT_DIR}
179203
mkdir -p ${COMMENT_DIR}
180204
echo ${{ github.event.number }} > ${COMMENT_DIR}/NR
181205
COMMENT_LOG="${COMMENT_DIR}/result.txt"
206+
: > "${COMMENT_LOG}"
182207
183208
COMPARISON_RESULT=0
184209
if test -f "${CURRENT_SUMMARY_FILE}"; then
185-
if test -f "${REF_SUMMARY_FILE}"; then
210+
if test -s "${REF_SUMMARY_FILE}"; then
186211
echo "Reference summary SHA1/ID: $(sha1sum -- "${REF_SUMMARY_FILE}")"
187212
echo "Current summary SHA1/ID: $(sha1sum -- "${CURRENT_SUMMARY_FILE}")"
188213
@@ -193,7 +218,7 @@ jobs:
193218
194219
COMPARISON_RESULT=$?
195220
else
196-
echo "::warning ::Skipping test comparison; no prior reference summary is available at '${REF_SUMMARY_FILE}'."
221+
echo "::warning ::Skipping test comparison; no usable reference summary is available at '${REF_SUMMARY_FILE}'."
197222
fi
198223
else
199224
echo "::error ::Failed to find summary of test results (missing '${CURRENT_SUMMARY_FILE}'); failing early"
@@ -203,6 +228,11 @@ jobs:
203228
if [ ${COMPARISON_RESULT} -eq 1 ]; then
204229
echo "::error ::Found new non-intermittent test failures"
205230
exit 1
231+
elif [ ${COMPARISON_RESULT} -ge 2 ]; then
232+
# The comparison itself failed (e.g. an unusable reference summary).
233+
# Don't post a comment rather than post a misleading one.
234+
: > "${COMMENT_LOG}"
235+
echo "::warning ::Could not compare the test results against the reference"
206236
else
207237
echo "::notice ::No new test failures detected"
208238
fi
@@ -212,7 +242,7 @@ jobs:
212242
uses: actions/upload-artifact@v4
213243
with:
214244
name: comment
215-
path: reference/comment/
245+
path: comment/
216246

217247
- name: Report test results
218248
if: success() || failure()

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
/target
22
*.swp
3+
/tests/test-results.json

‎tests/run-upstream-testsuite.sh‎

Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@
1616
# tests are run might not match exactly that used when the upstream tests are
1717
# run through the autotools.
1818

19+
# Exit codes: 0 if all tests passed, 1 if at least one test failed, and 2 if
20+
# the test suite could not be run at all (e.g. the upstream repository couldn't
21+
# be fetched). Callers must treat 2 as an infrastructure error: no meaningful
22+
# result was produced.
23+
1924
# By default it expects a release build of the diffutils binary, but a
2025
# different build profile can be specified as an argument
2126
# (e.g. 'dev' or 'test').
@@ -26,6 +31,12 @@
2631
scriptpath=$(dirname "$(readlink -f "$0")")
2732
rev=$(git rev-parse HEAD)
2833

34+
# Report an infrastructure error: the test suite could not be run at all
35+
die() {
36+
echo "ERROR: $*" >&2
37+
exit 2
38+
}
39+
2940
# Allow passing a specific profile as parameter (default to "release")
3041
profile="release"
3142
[[ -n $1 ]] && profile="$1"
@@ -34,35 +45,47 @@ profile="release"
3445
binary="$scriptpath/../target/$profile/diffutils"
3546
if [[ ! -x "$binary" ]]
3647
then
37-
echo "Missing build for profile $profile"
38-
exit 1
48+
die "Missing build for profile $profile"
3949
fi
4050

4151
# Work in a temporary directory
4252
tempdir=$(mktemp -d)
43-
cd "$tempdir"
53+
trap 'rm -rf "$tempdir"' EXIT
54+
cd "$tempdir" || die "Cannot enter temporary directory $tempdir"
4455

45-
# Check out the upstream test suite
56+
# Check out the upstream test suite. git.savannah.gnu.org is regularly
57+
# unavailable or slow, so retry a few times before giving up.
4658
gitserver="https://git.savannah.gnu.org"
4759
testsuite="$gitserver/git/diffutils.git"
48-
echo "Fetching upstream test suite from $testsuite"
49-
git clone -n --depth=1 --filter=tree:0 "$testsuite" &> /dev/null
50-
cd diffutils
51-
git sparse-checkout set --no-cone tests &> /dev/null
52-
git checkout &> /dev/null
60+
attempts=3
61+
for (( attempt = 1; attempt <= attempts; attempt++ ))
62+
do
63+
echo "Fetching upstream test suite from $testsuite (attempt $attempt/$attempts)"
64+
rm -rf diffutils
65+
git clone -n --depth=1 --filter=tree:0 "$testsuite" && break
66+
(( attempt < attempts )) && sleep $(( attempt * 10 ))
67+
done
68+
[[ -d diffutils ]] || die "Failed to fetch the upstream test suite from $testsuite"
69+
cd diffutils || die "Failed to fetch the upstream test suite from $testsuite"
70+
git sparse-checkout set --no-cone tests &> /dev/null || die "Cannot sparse-checkout the upstream tests"
71+
git checkout &> /dev/null || die "Cannot check out the upstream tests"
5372
upstreamrev=$(git rev-parse HEAD)
73+
[[ -d tests ]] || die "The upstream checkout contains no tests directory"
5474

5575
# Ensure that calling `diff` invokes the built `diffutils` binary instead of
5676
# the upstream `diff` binary that is most likely installed on the system
5777
mkdir src
58-
cd src
78+
cd src || die "Cannot create the directory holding the diff and cmp symlinks"
5979
ln -s "$binary" diff
6080
ln -s "$binary" cmp
61-
cd ../tests
81+
cd ../tests || die "Cannot enter the upstream tests directory"
6282

6383
# Fetch tests/init.sh from the gnulib repository (needed since
6484
# https://git.savannah.gnu.org/cgit/diffutils.git/commit/tests?id=1d2456f539)
65-
curl -sL "$gitserver/gitweb/?p=gnulib.git;a=blob_plain;f=tests/init.sh;hb=HEAD" -o init.sh
85+
curl -sL --fail --retry 3 --retry-delay 5 --retry-all-errors \
86+
"$gitserver/gitweb/?p=gnulib.git;a=blob_plain;f=tests/init.sh;hb=HEAD" -o init.sh \
87+
|| die "Failed to fetch tests/init.sh from the gnulib repository"
88+
[[ -s init.sh ]] || die "Fetched an empty tests/init.sh from the gnulib repository"
6689

6790
if [[ -n "$TESTS" ]]
6891
then
@@ -73,6 +96,7 @@ else
7396
tests=$(make -f Makefile.am printtests)
7497
fi
7598
total=$(echo "$tests" | wc -w)
99+
(( total > 0 )) || die "No test to run: the upstream test list is empty"
76100
echo "Running $total tests"
77101
export LC_ALL=C
78102
export KEEP=yes
@@ -105,7 +129,9 @@ do
105129
# but there isn't much value added in doing so
106130
for file in *
107131
do
108-
[[ -f "$file" ]] && json+="\"$file\":\"$(base64 -w0 < "$file")\","
132+
# Encode the name with jq: some tests create files whose name contains
133+
# quotes or control characters, which would produce invalid JSON
134+
[[ -f "$file" ]] && json+="$(jq -Rn --arg name "$file" '$name'):\"$(base64 -w0 < "$file")\","
109135
done
110136
json="${json%,}}},"
111137
cd - > /dev/null
@@ -144,10 +170,16 @@ json="{$metadata $json}"
144170

145171
# Clean up
146172
cd "$scriptpath"
147-
rm -rf "$tempdir"
148173

174+
# Write the results out only once they are known to be valid JSON, so that a
175+
# malformed (or truncated) file is never left behind for the caller to consume
149176
resultsfile="test-results.json"
150-
echo "$json" | jq > "$resultsfile"
177+
if ! echo "$json" | jq > "$resultsfile.tmp"
178+
then
179+
rm -f "$resultsfile.tmp"
180+
die "Generated invalid JSON results"
181+
fi
182+
mv "$resultsfile.tmp" "$resultsfile"
151183
echo "Results written to $scriptpath/$resultsfile"
152184

153185
(( failed > 0 )) && exit 1

‎util/compare_test_results.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
Compare the current GNU test results to the last results gathered from the main branch to
55
highlight if a PR is making the results better/worse.
66
Don't exit with error code if all failing tests are in the ignore-intermittent.txt list.
7+
8+
Exit status: 0 if the comparison shows no new failure, 1 if new non-intermittent
9+
failures appeared, and 2 if the comparison couldn't be performed at all (e.g. one
10+
of the result files is missing or malformed).
711
"""
812

913
import json
@@ -49,15 +53,15 @@ def compare_results(current_file, reference_file, ignore_file=None, output_file=
4953
current_summary, current_failed = extract_test_results(current_data)
5054
except Exception as e:
5155
print(f"Error loading current results: {e}")
52-
return 1
56+
return 2
5357

5458
try:
5559
with open(reference_file, "r") as f:
5660
reference_data = json.load(f)
5761
reference_summary, reference_failed = extract_test_results(reference_data)
5862
except Exception as e:
5963
print(f"Error loading reference results: {e}")
60-
return 1
64+
return 2
6165

6266
# Calculate differences
6367
pass_diff = int(current_summary.get("passed", 0)) - int(reference_summary.get("passed", 0))

0 commit comments

Comments
 (0)