Skip to content

Commit 73bc4c0

Browse files
myasnikovdaniilmyasnikovdaniil
committed
fix(ci): build the backstop's reason list without a blind pipeline
`unmatched=$(echo … | tr … | sort -u | grep -v '^$' | paste -sd ' ' -)` takes its exit status from paste, so a failure in tr, sort or grep is invisible under set -e. Demonstrated with a sort stub that exits 1: the pipeline reports rc=0 and yields an empty string, so the line becomes `no runnable suite is named by ''`. That is the same last-command blindness this script already handles twice -- for the suite-list find and for the two yq indexes -- and the comments there explain why each was a bug. Selection stays fail-safe because the escalation is decided before the message is built, so the only casualty is the reason line; but the reason line is a contract as of this change set, and a contract that degrades in silence is what the change set exists to remove. Replaced with the shell's own split plus a `case` membership test: no external command, nothing that can half-succeed, dedup preserved, and the same idiom resolve_suites already uses a few lines up. A test feeds two unresolved directories with one repeated and pins the whole line, so a regression to a form that can emit a partial or empty name turns the file red rather than printing a worse message. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent c28d780 commit 73bc4c0

2 files changed

Lines changed: 34 additions & 1 deletion

File tree

‎hack/select-e2e.sh‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,8 +518,24 @@ final_apps=$(intersect_suites "$group_suites $selected_apps")
518518
# or contributed a suite that exists — so the names worth naming are the
519519
# directly-selected ones, and printing them says which directory or file the
520520
# selector could not resolve.
521+
#
522+
# Deduplicated in the shell rather than through `tr | sort -u | grep -v | paste`,
523+
# which is the same blindness this file fixes twice above: a pipeline reports its
524+
# LAST command's status, so a failing tr or sort would be invisible under set -e
525+
# and the message would name a partial list or nothing at all. That is only a
526+
# cosmetic loss — the escalation itself is already decided — but the reason line
527+
# is a contract now, and a contract that degrades silently is the thing this
528+
# change set exists to remove. Unquoted expansion is the split, `case` is the
529+
# membership test, and neither can half-succeed; it is also the idiom
530+
# resolve_suites already uses.
521531
if [ -z "$final_apps" ]; then
522-
unmatched=$(echo "$selected_apps" | tr ' ' '\n' | sort -u | grep -v '^$' | paste -sd ' ' -)
532+
unmatched=''
533+
for a in $selected_apps; do
534+
case " $unmatched " in
535+
*" $a "*) ;;
536+
*) unmatched="${unmatched:+$unmatched }$a" ;;
537+
esac
538+
done
523539
echo "select-e2e: no runnable suite is named by '$unmatched' — escalating to the full suite" >&2
524540
escalate_to_full_suite
525541
fi

‎hack/select-e2e_test.bats‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -900,6 +900,23 @@ assert_full_suite() {
900900
cat "$tmp/err" >&2
901901
exit 1
902902
fi
903+
# Several unresolved directories, with one of them repeated: every distinct
904+
# name must appear, exactly once, in one line. The list used to be built by
905+
# `tr | sort -u | grep -v | paste`, whose exit status is paste's, so a failure
906+
# anywhere earlier in it would have gone unseen under set -e and printed a
907+
# partial name or none at all -- the same last-command blindness this script
908+
# fixes for the suite list and the yq indexes. The escalation is already
909+
# decided by then, so the only casualty is the reason line, which is exactly
910+
# what these asserts exist to defend.
911+
[ ! -d hack/e2e-chainsaw/_zz ]
912+
printf '%s\n' hack/e2e-chainsaw/_fixtures/a.yaml \
913+
hack/e2e-chainsaw/_fixtures/b.yaml \
914+
hack/e2e-chainsaw/_zz/c.yaml > "$tmp/diff"
915+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources" 2>"$tmp/err")
916+
assert_full_suite "$output"
917+
line=$(grep 'no runnable suite is named by' "$tmp/err")
918+
assert_selection "the backstop must name every unresolved directory once" \
919+
"$line" "select-e2e: no runnable suite is named by '_fixtures _zz' — escalating to the full suite"
903920
rm -rf "$tmp"
904921
}
905922

0 commit comments

Comments
 (0)