Skip to content

Commit 3b2a564

Browse files
myasnikovdaniilmyasnikovdaniil
committed
fix(e2e): fail closed on unknown Chainsaw suites
Validate each direct Chainsaw suite name against the runnable inventory before unioning selections. This prevents valid direct, graph, or broad neighbours from hiding deleted or renamed suite paths. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent de6468d commit 3b2a564

2 files changed

Lines changed: 81 additions & 58 deletions

File tree

‎hack/select-e2e.sh‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,11 @@
1515
# tier: every suite except kubernetes-previous, for the build
1616
# inputs in broad_suite_pattern
1717
# - full list any path that affects all tests, OR an unrecognised
18-
# packages/* path, OR a changed package that reaches no
19-
# runnable suite through the graph, OR a path matching NEITHER
20-
# the full-suite nor the inert list, OR a yq that failed to
21-
# build the dependency graph (conservative fallbacks)
18+
# Chainsaw suite, OR an unrecognised packages/* path, OR a
19+
# changed package that reaches no runnable suite through the
20+
# graph, OR a path matching NEITHER the full-suite nor the
21+
# inert list, OR a yq that failed to build the dependency
22+
# graph (conservative fallbacks)
2223
# - nothing, and the suite list itself is unavailable: find failed, or
2324
# a non-zero hack/e2e-chainsaw holds no chainsaw-test.yaml. Unlike the
2425
# exit yq case there is no fallback left to take — an empty list
@@ -417,17 +418,19 @@ while IFS= read -r file || [ -n "$file" ]; do
417418
# A suite parked as chainsaw-test.yaml.disabled is registered nowhere and
418419
# executed by nothing, so an edit to it cannot regress a test. Matched
419420
# before the per-suite rule below, which reads the suite name off the
420-
# directory and ignores the suffix: the name it derives matches no
421-
# existing suite, the intersection at the bottom empties, and the
422-
# safety net for a genuinely unclassified selection escalates to the
423-
# full run — the most expensive outcome, bought by the one file class
424-
# that provably cannot affect anything. Inert is the classification;
425-
# the safety net keeps its job for paths no rule has looked at.
421+
# directory and would otherwise escalate an unknown name. Inert is the
422+
# classification for the one file class that provably cannot affect a
423+
# runnable suite.
426424
continue ;;
427425
hack/e2e-chainsaw/*/*)
428426
app=$(echo "$file" | sed -nE 's,^hack/e2e-chainsaw/([^/]+)/.*,\1,p')
429-
selected_apps="$selected_apps $app"
430-
trigger_any=1
427+
if echo "$all_apps" | grep -Fxq "$app"; then
428+
selected_apps="$selected_apps $app"
429+
trigger_any=1
430+
else
431+
echo "select-e2e: '$file' names no runnable suite ('$app') — escalating to the full suite" >&2
432+
trigger_full=1
433+
fi
431434
continue ;;
432435
hack/e2e-apps/*)
433436
# The pre-Chainsaw per-app BATS suites. One file per app, named after it,
@@ -623,12 +626,10 @@ done
623626
final_apps=$(intersect_suites "$group_suites $selected_apps")
624627

625628
# Backstop. Every graph path above either escalates or contributes a suite that
626-
# exists, and both rules that select a suite by name — the per-app BATS one and
627-
# the examples/backups/<app>/ one — membership-test it first, so what still
628-
# reaches this is a per-suite Chainsaw edit naming a directory that holds no
629-
# chainsaw-test.yaml: shared material beside _lib/, or a suite nested deeper than
630-
# the depth-2 scan looks. Selecting nothing for those would skip E2E outright, so
631-
# failing towards the full suite is the only safe way to be wrong here.
629+
# exists, and every rule that selects a suite by name membership-tests it first.
630+
# Keep this as a final invariant for future selection rules: selecting an
631+
# unknown name must escalate rather than letting the intersection silently drop
632+
# it and skip E2E.
632633
#
633634
# group_suites is empty whenever this fires — every group either escalated above
634635
# or contributed a suite that exists — so the names worth naming are the

‎hack/select-e2e_test.bats‎

Lines changed: 62 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -385,17 +385,16 @@ assert_broad_tier() {
385385
rm -rf "$tmp"
386386
}
387387

388-
# Regression, found in independent review of this change: the broad tier used to
389-
# resolve BEFORE the unresolved-suite backstop, so an unrelated go.mod bump in
390-
# the same diff swallowed that per-path escalation and its stderr line. That is
391-
# the merge-before-escalate shape #3330 removed from the graph walk.
388+
# A broad path must not weaken an unknown path's per-path escalation. This small
389+
# case keeps the original regression trigger visible beside the full mixed-path
390+
# matrix below.
392391
@test "an unresolved suite still escalates when a broad path is in the diff" {
393392
tmp=$(mktemp -d)
394393
cp -r packages/core/platform/sources "$tmp/sources"
395394
printf 'go.mod\nhack/e2e-chainsaw/nosuch/a.yaml\n' > "$tmp/diff"
396395
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources" 2>"$tmp/err")
397396
assert_full_suite "$output"
398-
grep -q 'no runnable suite is named' "$tmp/err" \
397+
grep -Fq "'hack/e2e-chainsaw/nosuch/a.yaml' names no runnable suite ('nosuch')" "$tmp/err" \
399398
|| { echo "expected the unresolved-suite reason on stderr, got: $(cat "$tmp/err")" >&2; exit 1; }
400399
rm -rf "$tmp"
401400
}
@@ -1045,46 +1044,69 @@ assert_broad_tier() {
10451044
done
10461045
}
10471046

1048-
@test "an edit to a non-suite directory under e2e-chainsaw escalates" {
1049-
# Only a switched-off suite is ignorable. Shared material next to _lib/, or
1050-
# a suite nested deeper than the depth-2 scan looks, is invisible to
1051-
# all_apps — selecting nothing for it would skip E2E outright, so it
1052-
# escalates through the backstop instead. That backstop is the one path
1053-
# still reaching the bottom of the script now that the graph decides per
1054-
# path, so pin it rather than assume it stays reachable.
1047+
@test "an unknown Chainsaw suite path escalates independently of valid neighbours" {
1048+
# A deleted suite path and the old side of a suite rename both name a
1049+
# directory absent from the runnable inventory. Each must force a full run
1050+
# on its own account: merging names before checking membership lets a valid
1051+
# direct or graph-selected neighbour hide the unknown one. A broad path is
1052+
# another neighbour, and its 20-suite answer must not pre-empt the full run.
10551053
tmp=$(mktemp -d)
10561054
cp -r packages/core/platform/sources "$tmp/sources"
1057-
# Premise: the directory must not exist, or all_apps would hold it and the
1058-
# test would be measuring the ordinary per-suite rule.
1059-
[ ! -d hack/e2e-chainsaw/_fixtures ]
1060-
echo "hack/e2e-chainsaw/_fixtures/tenant.yaml" > "$tmp/diff"
1055+
[ ! -d hack/e2e-chainsaw/deleted-suite ]
1056+
[ ! -d hack/e2e-chainsaw/renamed-postgres ]
1057+
1058+
for neighbour in \
1059+
hack/e2e-chainsaw/postgres/chainsaw-test.yaml \
1060+
packages/apps/postgres/Chart.yaml; do
1061+
for broad in '' go.mod; do
1062+
for order in unknown-first unknown-last; do
1063+
unknown=hack/e2e-chainsaw/deleted-suite/chainsaw-test.yaml
1064+
if [ "$order" = unknown-first ]; then
1065+
printf '%s\n' "$unknown" "$neighbour" "$broad" > "$tmp/diff"
1066+
else
1067+
printf '%s\n' "$broad" "$neighbour" "$unknown" > "$tmp/diff"
1068+
fi
1069+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources" 2>"$tmp/err")
1070+
assert_full_suite "$output"
1071+
if ! grep -Fq "'$unknown' names no runnable suite ('deleted-suite')" "$tmp/err"; then
1072+
echo "unknown path must explain the full run ($neighbour, ${broad:-no broad}, $order); stderr was:" >&2
1073+
cat "$tmp/err" >&2
1074+
exit 1
1075+
fi
1076+
done
1077+
done
1078+
done
1079+
1080+
# A rename diff contains both the old unknown directory and the new valid
1081+
# one. The old side still escalates even though the new side selects a suite.
1082+
old=hack/e2e-chainsaw/renamed-postgres/chainsaw-test.yaml
1083+
printf '%s\n' "$old" hack/e2e-chainsaw/postgres/chainsaw-test.yaml > "$tmp/diff"
10611084
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources" 2>"$tmp/err")
10621085
assert_full_suite "$output"
1063-
# The backstop is the last branch that could reach the full suite without
1064-
# saying so. It names the directly-selected names it could not resolve, which
1065-
# for this diff is the directory the per-suite rule derived.
1066-
if ! grep -q "select-e2e:.*no runnable suite is named by.*_fixtures" "$tmp/err"; then
1067-
echo "the backstop must name what it could not resolve; stderr was:" >&2
1068-
cat "$tmp/err" >&2
1069-
exit 1
1070-
fi
1071-
# Several unresolved directories, with one of them repeated: every distinct
1072-
# name must appear, exactly once, in one line. The list used to be built by
1073-
# `tr | sort -u | grep -v | paste`, whose exit status is paste's, so a failure
1074-
# anywhere earlier in it would have gone unseen under set -e and printed a
1075-
# partial name or none at all -- the same last-command blindness this script
1076-
# fixes for the suite list and the yq indexes. The escalation is already
1077-
# decided by then, so the only casualty is the reason line, which is exactly
1078-
# what these asserts exist to defend.
1079-
[ ! -d hack/e2e-chainsaw/_zz ]
1080-
printf '%s\n' hack/e2e-chainsaw/_fixtures/a.yaml \
1081-
hack/e2e-chainsaw/_fixtures/b.yaml \
1082-
hack/e2e-chainsaw/_zz/c.yaml > "$tmp/diff"
1086+
grep -Fq "'$old' names no runnable suite ('renamed-postgres')" "$tmp/err"
1087+
rm -rf "$tmp"
1088+
}
1089+
1090+
@test "known and disabled Chainsaw paths keep their scoped behavior" {
1091+
tmp=$(mktemp -d)
1092+
cp -r packages/core/platform/sources "$tmp/sources"
1093+
1094+
echo hack/e2e-chainsaw/postgres/chainsaw-test.yaml > "$tmp/diff"
10831095
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources" 2>"$tmp/err")
1084-
assert_full_suite "$output"
1085-
line=$(grep 'no runnable suite is named by' "$tmp/err")
1086-
assert_selection "the backstop must name every unresolved directory once" \
1087-
"$line" "select-e2e: no runnable suite is named by '_fixtures _zz' — escalating to the full suite"
1096+
assert_selection "a known direct path must stay scoped" "$output" postgres
1097+
[ ! -s "$tmp/err" ]
1098+
1099+
# Disabled files are inert even when their directory names no runnable
1100+
# suite, alone and beside a valid direct path.
1101+
disabled=hack/e2e-chainsaw/no-such-suite/chainsaw-test.yaml.disabled
1102+
echo "$disabled" > "$tmp/diff"
1103+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources" 2>"$tmp/err")
1104+
[ -z "$output" ]
1105+
[ ! -s "$tmp/err" ]
1106+
printf '%s\n' "$disabled" hack/e2e-chainsaw/postgres/chainsaw-test.yaml > "$tmp/diff"
1107+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources" 2>"$tmp/err")
1108+
assert_selection "a disabled neighbour must stay inert" "$output" postgres
1109+
[ ! -s "$tmp/err" ]
10881110
rm -rf "$tmp"
10891111
}
10901112

0 commit comments

Comments
 (0)