Skip to content

Commit ae4e9e5

Browse files
myasnikovdaniillexfrei
authored andcommitted
fix(ci): classify every TIA path and escalate the unclassified
`select-e2e.sh` silently selected nothing for a changed path it did not recognise outside `packages/`. Both e2e lanes read an empty selection as "skip Chainsaw" and then post the required "E2E Tests" status green, so an unrecognised path produced a green merge gate with no suite run. The worked example is `hack/common-envs.mk`, which supplies the tag, push and output flags of every image in the tree and matched nothing: `full_suite_pattern` covered `hack/*.sh` and `hack/*.bats` but not `hack/*.mk`. `pkg/`, `tools/`, `go.mod`, `hack/lib/` and `hack/buildkitd.toml` were in the same position, as was `.github/workflows/e2e-fork.yaml` while `pull-requests.yaml` was already escalated. A blanket "empty means full suite" fallback would be the wrong fix: it escalates the inert paths the design deliberately skips. Instead every path now lands in one of three classes by an explicit rule — the full-suite list for shared build inputs and the e2e harness, an inert list for paths that cannot affect what e2e exercises, and the PackageSource graph for `packages/(apps|system|extra)/`. A path matching none of them escalates and logs why, so the next path someone adds fails safe instead of failing open. The two lists are checked in that order, so a specific escalation beats a broad inert directory: `.github/` is inert, the workflows that run the suite are not. `*.md` and `docs/`/`dashboards/` stay ahead of both, so a README under `packages/core/` does not inherit that tree's escalation. Ten tests cover both sides of the classification. Mutation-checked: reverting the fall-through fails the unclassified-path test, dropping `.github/` from the inert list fails the non-e2e-workflow test, and swapping the two list checks fails the existing pull-requests.yaml test. Note the per-entry escalations are belt-and-braces — removing `hack/*.mk` from the pattern still escalates via the fall-through, which is the point of having it. Closes #3392 Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent 7dca642 commit ae4e9e5

3 files changed

Lines changed: 165 additions & 9 deletions

File tree

‎docs/agents/e2e-testing.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ A parent HelmRelease that hit its wait timeout, uninstalled, and reinstalled is
6868

6969
`hack/select-e2e.sh` walks the `packages/core/platform/sources/*.yaml` dependency graph and runs only the Chainsaw suites affected by a diff. A suite is a directory under `hack/e2e-chainsaw/` containing a `chainsaw-test.yaml`.
7070

71-
- Conservative escalation: edits to `packages/library/`, `packages/core/`, `api/`, `cmd/`, `internal/`, top-level `hack/*.sh|*.bats` helpers, the shared `hack/e2e-chainsaw/_lib/`, the chainsaw config `hack/e2e-chainsaw/.chainsaw.yaml`, the `Makefile`, or `.github/workflows/pull-requests.yaml` escalate to the **full suite**. A per-suite edit (any file under `hack/e2e-chainsaw/<app>/`) selects **only** that suite. Note the exception, since "the E2E workflows" would be the natural reading: the selector's list names `pull-requests.yaml` and not `e2e-fork.yaml`, so a change touching only the fork workflow selects nothing and the Chainsaw step is skipped, while the platform install and the OpenAPI tests still run. That is tolerable rather than designed, and it is tolerable for a specific reason: a `workflow_run` always executes the default-branch copy of `e2e-fork.yaml`, so running the suite against a change to it would not exercise that change anyway. It is the same fail-open shape as #3392.
71+
- Conservative escalation: edits to `packages/library/`, `packages/core/`, the Go trees (`api/`, `cmd/`, `internal/`, `pkg/`), the codegen under `tools/`, `go.mod`/`go.sum`, top-level `hack/*.sh|*.bats` helpers and the `hack/lib/` they source, `hack/*.mk`, `hack/buildkitd.toml`, `hack/e2e-*.yaml`, the shared `hack/e2e-chainsaw/_lib/`, the chainsaw config `hack/e2e-chainsaw/.chainsaw.yaml`, the `Makefile`, or any workflow that runs the suite escalate to the **full suite**. A per-suite edit (any file under `hack/e2e-chainsaw/<app>/`) selects **only** that suite.
72+
- **Every path is classified, and an unclassified one escalates.** The selector has three outcomes — full suite, a scoped set, or nothing — and each is reached by an explicit rule: `full_suite_pattern` for shared build inputs and the e2e harness, `inert_config_pattern` for paths that cannot affect what e2e exercises (repo meta, `.github/` bar the e2e workflows, agent config, `hack/testdata/`), and the graph lookup for `packages/(apps|system|extra)/`. A path matching none of them escalates to the full suite and logs why, because both lanes read an empty selection as "skip Chainsaw" and then post `E2E Tests` green — so a silent default is a green gate with nothing tested (#3392). When that escalation fires for something genuinely inert, add it to `inert_config_pattern`; do not relax the fall-through. The escalation list and the inert list are checked in that order, so a specific escalation beats a broad inert directory.
7273
- The `full-e2e` PR label forces the whole suite.
7374
- The selection is passed to `make test-chainsaw CHAINSAW_SUITES="<names>"`, which runs `chainsaw test <names>` from `hack/e2e-chainsaw/` (empty = the whole suite).
7475
- The coverage gap TIA opens on PRs is closed at release-cut time: the "Prepare release" commit bakes image digests into `packages/core/` (and other packages), which TIA escalates to the **full** suite — so the release PR exercises the whole suite against the commit the tag will point at.
@@ -82,7 +83,7 @@ Be precise about TIA's scope — it is narrower than "skip E2E for unrelated PRs
8283
- **It trims only which suites `chainsaw test` runs, not the expensive stages.** `Prepare environment` and `Install Cozystack` (the full platform install) run regardless of the selection. TIA only decides the `CHAINSAW_SUITES` passed to the final E2E step. A perfect narrow saves the suite tail, not the bulk of wall-time. (Package image builds are scoped separately — by `hack/build-matrix.sh`, on the PR diff — while the Talos and installer/finalize legs still run for non-docs PRs; that is a distinct mechanism from TIA's suite trimming.)
8384
- **`Select E2E tests` runs *after* `Install Cozystack` and has no `always()`.** GitHub implicitly ANDs a step's `if:` with `success()`, so if install fails the selector (and `Run E2E tests`) are skipped entirely. On a red install you will not see TIA narrow anything — that is the ordering, not a selector bug.
8485
- **Most PRs legitimately escalate to the full suite.** Platform manifests live under `packages/core/`, Go under `api/`/`internal/`, helpers under `hack/`, plus the `Makefile` — touching any one triggers the full suite. So "all suites selected" is usually correct, not a failure to narrow.
85-
- **Two skip layers, with a gap.** The `plan` job computes a coarse docs-only gate (`code`, true for any change outside `docs/**`) that skips the whole build+E2E pipeline for docs-only PRs, plus a per-package build matrix (`hack/build-matrix.sh`) that scopes which images build. TIA's finer skip (`select-e2e.sh` ignoring `docs/`, `dashboards/`, and `*.md`) only trims the suite selection. A PR touching only `dashboards/*.json` therefore still runs install, then skips just the Chainsaw suites — near-zero savings. If you need such PRs to skip the pipeline, widen the `plan` docs-gate, not `select-e2e.sh`.
86+
- **Two skip layers, with a gap.** The `plan` job computes a coarse docs-only gate (`code`, true for any change outside `docs/**`) that skips the whole build+E2E pipeline for docs-only PRs, plus a per-package build matrix (`hack/build-matrix.sh`) that scopes which images build. TIA's finer skip (`select-e2e.sh` ignoring `docs/`, `dashboards/`, `*.md` and the rest of `inert_config_pattern`) only trims the suite selection. A PR touching only `dashboards/*.json` therefore still runs install, then skips just the Chainsaw suites — near-zero savings. If you need such PRs to skip the pipeline, widen the `plan` docs-gate, not `select-e2e.sh`.
8687

8788
### 8. Failure must be self-explanatory — diagnostics are first-class
8889

‎hack/select-e2e.sh‎

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,21 @@
66
# Defaults: sources-dir = packages/core/platform/sources
77
#
88
# Output:
9-
# - empty no E2E impact (docs / dashboards / *.md only)
9+
# - empty no E2E impact — an explicitly INERT path (docs, dashboards,
10+
# *.md, repo meta files; see inert_config_pattern)
1011
# - <suite names> selected per the PackageSource dependency graph
1112
# - full list any path that affects all tests, OR an unrecognised
1213
# packages/* path, OR a per-app source whose graph has
13-
# no *-application descendants (conservative fallback)
14+
# no *-application descendants, OR a path matching NEITHER
15+
# the full-suite nor the inert list (conservative fallbacks)
16+
#
17+
# Every changed path must land in exactly one of those three classes by an
18+
# explicit rule. An unclassified path escalates to the full suite rather than
19+
# selecting nothing: both e2e lanes read an empty selection as "skip Chainsaw"
20+
# and then post the required "E2E Tests" status green, so a silent default is a
21+
# green gate with no suite run (#3392). When that escalation fires for a path
22+
# that genuinely cannot affect e2e, add it to inert_config_pattern — do not
23+
# widen the fall-through.
1424
set -eu
1525

1626
CHANGED="${1:?missing changed-files arg}"
@@ -20,7 +30,35 @@ SOURCES_DIR="${2:-packages/core/platform/sources}"
2030
# edits (hack/e2e-chainsaw/<name>/...) are matched BEFORE this so editing one
2131
# suite doesn't escalate to the full suite; the shared _lib helpers and the
2232
# .chainsaw.yaml config affect every suite and DO escalate (handled inline).
23-
full_suite_pattern='^(packages/library/|packages/core/|api/|cmd/|internal/|hack/[^/]+\.sh$|hack/[^/]+\.bats$|Makefile$|\.github/workflows/pull-requests\.yaml$)'
33+
#
34+
# Three groups, all "cannot be scoped to one app":
35+
# - shipped code and its build inputs: packages/library, packages/core, the Go
36+
# trees (api, cmd, internal, pkg), the codegen under tools/, go.mod/go.sum,
37+
# the root Makefile, hack/*.mk (the tag/push/output flags of every image),
38+
# hack/buildkitd.toml (the builder every image is built with), and hack/lib/
39+
# (sourced by the hack/*.sh scripts that already escalate);
40+
# - the e2e harness itself: hack/*.sh, hack/*.bats and hack/e2e-*.yaml;
41+
# - the workflows that RUN the suite — enumerated rather than matched by
42+
# prefix, so an unrelated workflow does not burn a full run. Keep this list
43+
# in step with `rg -l test-chainsaw .github/workflows/`.
44+
full_suite_pattern='^(packages/library/|packages/core/|api/|cmd/|internal/|pkg/|tools/|hack/lib/|hack/[^/]+\.sh$|hack/[^/]+\.bats$|hack/[^/]+\.mk$|hack/buildkitd\.toml$|hack/e2e-[^/]+\.ya?ml$|go\.(mod|sum)$|Makefile$|\.github/workflows/(pull-requests|e2e-fork|e2e-tag|nightly)\.yaml$)'
45+
46+
# Paths with no bearing on what e2e exercises. Checked AFTER full_suite_pattern,
47+
# so a specific escalation wins over a broad directory here (.github/ is inert,
48+
# .github/workflows/pull-requests.yaml is not). This list is the whole reason
49+
# the fall-through at the bottom of the loop can escalate safely: the cost of
50+
# forgetting an inert path is a wasted full run, the cost of forgetting a live
51+
# one used to be a green gate with nothing tested.
52+
# - examples/ demo manifests; examples/backups/<app>/ is handled above
53+
# as a real test harness before this check is reached
54+
# - .github/ templates, CODEOWNERS, labels, renovate, linter config —
55+
# minus the e2e workflows escalated above
56+
# - .claude/ .gemini/ agent config, never shipped
57+
# - img/ README assets
58+
# - hack/testdata/ fixtures for the bats unit tests, not for e2e
59+
# - boilerplate.go.txt / dcgm-default-counters.csv codegen header; a CSV read
60+
# only by check-gpu-recording-rules.bats
61+
inert_config_pattern='^(examples/|\.github/|\.claude/|\.gemini/|img/|hack/testdata/|hack/boilerplate\.go\.txt$|hack/dcgm-default-counters\.csv$|LICENSE$|\.gitignore$|\.pre-commit-config\.yaml$|\.coderabbit\.yaml$)'
2462

2563
# All known Chainsaw suites: every dir under hack/e2e-chainsaw/ holding a
2664
# chainsaw-test.yaml (this excludes _lib/ and the top-level config files).
@@ -74,7 +112,9 @@ selected_apps=""
74112
while IFS= read -r file; do
75113
[ -z "$file" ] && continue
76114

77-
# 1. Skip: docs, dashboards, *.md
115+
# 1. Skip: docs, dashboards, *.md. Checked before everything else because
116+
# these can never matter wherever they live — a README inside
117+
# packages/core/ must not escalate the way its templates do.
78118
if echo "$file" | grep -qE '^(docs/|dashboards/)' || echo "$file" | grep -qE '\.md$'; then
79119
continue
80120
fi
@@ -111,7 +151,12 @@ while IFS= read -r file; do
111151
continue
112152
fi
113153

114-
# 4. Component change: lookup in PackageSource graph
154+
# 4. Explicitly inert: repo meta, agent config, non-e2e workflows, fixtures.
155+
if echo "$file" | grep -qE "$inert_config_pattern"; then
156+
continue
157+
fi
158+
159+
# 5. Component change: lookup in PackageSource graph
115160
rel=$(echo "$file" | sed -nE 's,^packages/(apps|system|extra)/([^/]+)/.*,\1/\2,p')
116161
if [ -n "$rel" ]; then
117162
src=$(echo "$OWNERS" | awk -v p="$rel" -F'\t' '$1==p {print $2}')
@@ -122,9 +167,15 @@ while IFS= read -r file; do
122167
# Inside packages/ but no graph entry — be conservative.
123168
trigger_full=1
124169
fi
170+
continue
125171
fi
126-
# Anything else (e.g. unrelated workflow files, top-level configs) is
127-
# silently ignored.
172+
173+
# 6. Unclassified. Escalate instead of ignoring: an empty selection makes both
174+
# e2e lanes skip Chainsaw and post "E2E Tests" green, so a path nobody has
175+
# classified must fail safe, not fail open (#3392). Add genuinely inert
176+
# paths to inert_config_pattern rather than relaxing this.
177+
echo "select-e2e: unclassified path '$file' — escalating to the full suite (classify it in hack/select-e2e.sh, see #3392)" >&2
178+
trigger_full=1
128179
done < "$CHANGED"
129180

130181
if [ "$trigger_full" = 1 ]; then

‎hack/select-e2e_test.bats‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,3 +170,107 @@
170170
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources") || true
171171
[ -z "$output" ]
172172
}
173+
174+
# --- #3392: every path is classified; unclassified escalates -----------------
175+
#
176+
# The bug these cover: an unrecognised path used to select nothing, both lanes
177+
# read an empty selection as "skip Chainsaw", and the required "E2E Tests"
178+
# status was then posted green with no suite run. Each test below pins one side
179+
# of the classification — escalate, or skip by an explicit rule.
180+
181+
@test "hack/*.mk triggers full suite (build flags of every image)" {
182+
tmp=$(mktemp -d)
183+
trap 'rm -rf "$tmp"' EXIT
184+
cp -r packages/core/platform/sources "$tmp/sources"
185+
echo "hack/common-envs.mk" > "$tmp/diff"
186+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources")
187+
[ "$(echo "$output" | wc -w)" -gt 5 ]
188+
}
189+
190+
@test "the fork e2e workflow triggers full suite" {
191+
tmp=$(mktemp -d)
192+
trap 'rm -rf "$tmp"' EXIT
193+
cp -r packages/core/platform/sources "$tmp/sources"
194+
echo ".github/workflows/e2e-fork.yaml" > "$tmp/diff"
195+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources")
196+
[ "$(echo "$output" | wc -w)" -gt 5 ]
197+
}
198+
199+
@test "pkg/ triggers full suite (shipped Go code)" {
200+
tmp=$(mktemp -d)
201+
trap 'rm -rf "$tmp"' EXIT
202+
cp -r packages/core/platform/sources "$tmp/sources"
203+
echo "pkg/cozystack/registry.go" > "$tmp/diff"
204+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources")
205+
[ "$(echo "$output" | wc -w)" -gt 5 ]
206+
}
207+
208+
@test "go.mod triggers full suite" {
209+
tmp=$(mktemp -d)
210+
trap 'rm -rf "$tmp"' EXIT
211+
cp -r packages/core/platform/sources "$tmp/sources"
212+
echo "go.mod" > "$tmp/diff"
213+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources")
214+
[ "$(echo "$output" | wc -w)" -gt 5 ]
215+
}
216+
217+
@test "hack/lib helper triggers full suite" {
218+
tmp=$(mktemp -d)
219+
trap 'rm -rf "$tmp"' EXIT
220+
cp -r packages/core/platform/sources "$tmp/sources"
221+
echo "hack/lib/image-refs.sh" > "$tmp/diff"
222+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources")
223+
[ "$(echo "$output" | wc -w)" -gt 5 ]
224+
}
225+
226+
@test "an unclassified path escalates instead of selecting nothing" {
227+
tmp=$(mktemp -d)
228+
trap 'rm -rf "$tmp"' EXIT
229+
cp -r packages/core/platform/sources "$tmp/sources"
230+
echo "brand-new-top-level/thing.conf" > "$tmp/diff"
231+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources" 2>/dev/null)
232+
[ "$(echo "$output" | wc -w)" -gt 5 ]
233+
}
234+
235+
@test "inert repo meta selects nothing" {
236+
tmp=$(mktemp -d)
237+
trap 'rm -rf "$tmp"' EXIT
238+
cp -r packages/core/platform/sources "$tmp/sources"
239+
printf '%s\n' .gitignore LICENSE .pre-commit-config.yaml > "$tmp/diff"
240+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources")
241+
[ -z "$output" ]
242+
}
243+
244+
@test "a non-e2e workflow selects nothing, an e2e one still escalates" {
245+
tmp=$(mktemp -d)
246+
trap 'rm -rf "$tmp"' EXIT
247+
cp -r packages/core/platform/sources "$tmp/sources"
248+
# .github/ is inert as a directory, but the escalation for the workflows
249+
# that run the suite is checked first and must win.
250+
echo ".github/workflows/tags.yaml" > "$tmp/diff"
251+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources")
252+
[ -z "$output" ]
253+
echo ".github/workflows/e2e-tag.yaml" > "$tmp/diff"
254+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources")
255+
[ "$(echo "$output" | wc -w)" -gt 5 ]
256+
}
257+
258+
@test "a README inside an escalating tree stays inert" {
259+
tmp=$(mktemp -d)
260+
trap 'rm -rf "$tmp"' EXIT
261+
cp -r packages/core/platform/sources "$tmp/sources"
262+
# packages/core/ escalates, but *.md is matched before that so a doc edit
263+
# under it does not burn a full run.
264+
echo "packages/core/installer/README.md" > "$tmp/diff"
265+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources")
266+
[ -z "$output" ]
267+
}
268+
269+
@test "an inert path alongside a real one does not mask the selection" {
270+
tmp=$(mktemp -d)
271+
trap 'rm -rf "$tmp"' EXIT
272+
cp -r packages/core/platform/sources "$tmp/sources"
273+
printf '%s\n' .gitignore packages/apps/postgres/values.yaml > "$tmp/diff"
274+
output=$(hack/select-e2e.sh "$tmp/diff" "$tmp/sources")
275+
[ "$output" = "postgres" ]
276+
}

0 commit comments

Comments
 (0)