Skip to content

Commit 7434b4d

Browse files
committed
test: Address review feedback on the e2e runner
Per review on antonbabenko#990: - replace the `RETURN` trap with explicit cleanup at the function's single exit (the `RETURN` trap is Bash-only and confused reviewers); - read the expected return code with `$(< file)` instead of `$(cat file)`; - quote the `true`/`false` flag values to avoid confusion with the builtins; - use `&>` instead of `> ... 2>&1`; - drop the redundant trailing slash on the copy destination; - print the summary with `printf` instead of a loop. No behavior change; the suite still passes natively and inside the image. Signed-off-by: penpal <unameme@proton.me>
1 parent cdebf54 commit 7434b4d

1 file changed

Lines changed: 23 additions & 26 deletions

File tree

tests/e2e/run_e2e_tests.sh

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,10 @@ function run_case {
5959
work="$(mktemp -d)"
6060
config="$(mktemp)"
6161
log="$(mktemp)"
62-
# shellcheck disable=SC2064 # expand paths now, on purpose
63-
trap "rm -rf '$work' '$config' '$log' '$log.diff'" RETURN
6462

6563
# Materialize the input working tree as a git repo (hooks and pre-commit
6664
# both expect one; the wrapper hook calls `git rev-parse --show-toplevel`).
67-
cp -R "$case_dir/input/." "$work/"
65+
cp -R "$case_dir/input/." "$work"
6866
git -C "$work" init -q
6967
git -C "$work" add -A
7068
git -C "$work" \
@@ -79,44 +77,46 @@ function run_case {
7977
(
8078
cd "$work"
8179
pre-commit run --config "$config" --all-files
82-
) > "$log" 2>&1 || actual_rc=$?
80+
) &> "$log" || actual_rc=$?
8381

8482
local expected_rc=0
8583
[[ -f "$case_dir/expected_returncode" ]] &&
86-
expected_rc="$(cat "$case_dir/expected_returncode")"
84+
expected_rc="$(< "$case_dir/expected_returncode")"
8785

8886
# Drop the throwaway git dir so the tree compare only sees fixture output.
8987
# `git diff --no-index` is used instead of `diff -r` because the project
9088
# image ships BusyBox `diff`, which lacks `--exclude` and `-u`.
9189
rm -rf "$work/.git"
9290

93-
local ok=true reason=''
91+
local ok="true" reason=''
9492
if [[ $actual_rc -ne $expected_rc ]]; then
95-
ok=false
93+
ok="false"
9694
reason="exit code ${actual_rc}, expected ${expected_rc}"
9795
elif ! git --no-pager diff --no-index --exit-code \
98-
"$case_dir/expected" "$work" > "$log.diff" 2>&1; then
99-
ok=false
96+
"$case_dir/expected" "$work" &> "$log.diff"; then
97+
ok="false"
10098
reason='output differs from expected/'
10199
fi
102100

103-
if [[ $ok == true ]]; then
101+
local rc=0
102+
if [[ $ok == "true" ]]; then
104103
summary+=("${C_GREEN}PASS${C_RESET} ${test_id}")
105104
passed+=1
106-
return 0
105+
else
106+
summary+=("${C_RED}FAIL${C_RESET} ${test_id} (${reason})")
107+
failed+=1
108+
echo "${C_RED}--- FAIL: ${test_id} (${reason}) ---${C_RESET}"
109+
echo "pre-commit output:"
110+
sed 's/^/ /' "$log"
111+
if [[ -s "$log.diff" ]]; then
112+
echo "diff (expected vs actual):"
113+
sed 's/^/ /' "$log.diff"
114+
fi
115+
rc=1
107116
fi
108117

109-
summary+=("${C_RED}FAIL${C_RESET} ${test_id} (${reason})")
110-
failed+=1
111-
echo "${C_RED}--- FAIL: ${test_id} (${reason}) ---${C_RESET}"
112-
echo "pre-commit output:"
113-
sed 's/^/ /' "$log"
114-
if [[ -s "$log.diff" ]]; then
115-
echo "diff (expected vs actual):"
116-
sed 's/^/ /' "$log.diff"
117-
fi
118-
rm -f "$log.diff"
119-
return 1
118+
rm -rf "$work" "$config" "$log" "$log.diff"
119+
return "$rc"
120120
}
121121

122122
function main {
@@ -132,10 +132,7 @@ function main {
132132

133133
echo
134134
echo '==== e2e summary ===='
135-
local line
136-
for line in "${summary[@]}"; do
137-
echo " $line"
138-
done
135+
printf ' %s\n' "${summary[@]}"
139136
echo " ${passed} passed, ${failed} failed, ${skipped} skipped"
140137

141138
[[ $failed -eq 0 ]]

0 commit comments

Comments
 (0)