fix(ci): close three blind spots in the e2e test selectors by lexfrei · Pull Request #4658 · cozystack/cozystack

Aleksei Sviridkin (lexfrei) · 2026-10-01T15:57:53Z

What this PR does

The e2e selectors miss Chainsaw suites that have no chainsaw-test.yaml or .yml directly in their directory. Chainsaw v0.2.15 also runs a directory that holds only numbered step files such as 01-install.yaml, and its discovery walks every subdirectory of the path the runner hands it. A suite in either layout ran only when nothing was selected. No narrowed selection named it, the full-suite list left it out, and select-install.sh --validate never asked for its mapping. Now hack/select-e2e.sh and hack/select-install.sh list a top-level directory as a suite when Chainsaw would load a test from it or from anywhere below it. No suite in the current tree uses either layout, so today's suite list stays the same.

A file placed directly in examples/backups/ (a shared common.sh, for example) selected nothing, because it fell through to the inert examples/ rule. It now escalates to the full suite, same as examples/backups/_lib/. README.md there stays inert through the *.md rule.

The test-chainsaw comment in packages/core/testing/Makefile said Chainsaw discovers every chainsaw-test.yaml, which was too narrow. I fixed it, and the matching sentences in docs/agents/e2e-testing.md too.

Every behaviour change has a bats test that fails against the scripts on main.

Known gaps I left for a follow-up, none of them a wrong answer today:

The full_suite_list helper in hack/select-e2e_test.bats repeats the selector's find/sed, so a bug in that shared pattern would pass every full-suite assertion. The step-file and nested tests compare against literal lists, which covers the new layouts.
The selectors are a little wider than Chainsaw: a broken chainsaw-test.y*ml symlink, or one with no Test in it, still lists its suite. That errs toward running more.
A few test comments describe what used to happen rather than why the rule exists.

Closes #4630

Screenshots

Downstream repositories

Release note

fix(ci): the e2e test selectors now see Chainsaw suites made of numbered step files or nested below a suite directory, and run the full suite for an edit to a file placed directly in examples/backups/.

Summary by CodeRabbit

Bug Fixes
- E2E suite discovery recognizes supported manifest and numbered-step files at any depth and maps them to the top-level suite.
- Changes to files directly under examples/backups/ trigger full-suite selection.
- Suite selection reports an error if discovery fails or finds no suites.
Documentation
- Clarified supported suite layouts, disabled suite-file handling, discovery and escalation rules, and per-path coverage guidance.
Tests
- Added coverage for nested suites, numbered steps, unmapped suite layouts, and backup-path selection.

coderabbitai · 2026-10-01T15:58:22Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: cozystack/cozystack/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 79135592-e20f-4304-a35a-d01b38a485cb

📥 Commits

Reviewing files that changed from the base of the PR and between 9e777b3 and 370f784.

📒 Files selected for processing (4)

docs/agents/e2e-testing.md
hack/select-e2e.sh
hack/select-e2e_test.bats
hack/select-install_test.bats

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Walkthrough

The E2E selectors now discover Chainsaw suites from manifests and numbered step files at any depth. The E2E selector also escalates loose files under examples/backups/ to the full suite. Documentation and tests describe and cover these behaviors.

Changes

E2E selector coverage

Layer / File(s)	Summary
Discover suites from manifests and step files `hack/select-e2e.sh`, `hack/select-e2e_test.bats`, `hack/select-install.sh`, `hack/select-install_test.bats`, `docs/agents/e2e-testing.md`, `packages/core/testing/Makefile`	Both selectors find Chainsaw manifests and numbered YAML step files recursively, then map tests to their top-level suite directory. Tests cover step-only suites and nested tests. The documentation describes suite layouts and discovery.
Escalate loose backup paths `hack/select-e2e.sh`, `hack/select-e2e_test.bats`	Files directly under `examples/backups/` trigger full-suite selection. Tests cover loose files in mixed diffs and confirm that a README remains unselected.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 370f7

The selectors broaden coverage for nested and step-only suites and escalate loose backup changes while leaving Markdown inert. No merge-blocking issue is established; merge after normal checks pass.

Architecture Summary

Architecture risk: 🔵 Low · up to 370f7

The change affects 3 systems.

Changed systems: hack, docs, packages/core

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

observed — hack (service) was modified; 4 changed files map to changed impact.
observed — docs (service) was modified; 1 changed file maps to changed impact.
observed — packages/core (library) was modified; 1 changed file maps to changed impact.

Before / after behavior

observed — Modified behavior in hack/select-install.sh: discover_suites now searches recursively beneath the suites directory and extracts the first path component for files matching either Chainsaw test-manifest names or numbered step-file names. Previously it searched only exactly two levels deep and recognized only chainsaw-test.yaml and chainsaw-test.yml.
observed — Modified behavior in packages/core/testing/Makefile: The comment changes the described Chainsaw discovery unit from every chainsaw-test.yaml under the directory to each Test in or below a suite directory; executable behavior is unchanged.
observed — Modified behavior in docs/agents/e2e-testing.md: The app-suite description removes chainsaw-test.yml as an alternative layout from this sentence; the later TIA description covers both extensions and additional layouts.
observed — Modified behavior in docs/agents/e2e-testing.md: The Proxmox-suite explanation now specifies that neither Chainsaw nor the selector counts .disabled files as tests, replacing the selector-only discovery explanation.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR satisfies [`#4630`]. `hack/select-e2e.sh` and `hack/select-install.sh` discover `chainsaw-test.yaml`, `chainsaw-test.yml`, and numbered step-file suites at nested paths. A direct file under `exam…
Out of Scope Changes check	✅ Passed	The selector changes, BATS tests, Makefile comment, and E2E documentation directly implement or explain [`#4630`]. The reviewed scope contains no demonstrated unrelated change.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (1 skipped: 1 …
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies a CI fix for blind spots in the e2e test selectors. It accurately summarizes the main change and is concise.

✨ Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

With no chainsaw-test.* in a directory, Chainsaw v0.2.15 builds a Test from files named like 01-install.yaml. Such a directory runs whenever the whole tree is handed to chainsaw, but neither selector listed it, so no escalation carried it and no edit to it could select it. Both scripts now derive the suite list from the file names Chainsaw itself accepts. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>

A file beside the per-app backup directories rather than inside one belongs to no single walkthrough, but it matched neither backup rule and fell to examples/ in the inert list, so an edit to it selected nothing and both lanes skipped Chainsaw. It is now treated as shared by all of them and escalates the same way examples/backups/_lib/ does. README.md there stays inert through the *.md rule. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>

Chainsaw walks every subdirectory of the path it is handed, and the runner hands it each top-level directory under hack/e2e-chainsaw/, so a test in <dir>/<sub>/ runs as part of <dir>. Both selectors read only one level down, so a directory whose tests all sit deeper was never listed: no selection named it, the full-suite list left it out too, and only a run with no selection at all executed it. Both scripts now take the top-level directory of a test file found in that directory or anywhere below it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>

The test-chainsaw comment said Chainsaw discovers every chainsaw-test.yaml. It also loads chainsaw-test.yml, builds a test from numbered step files, and walks subdirectories. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>

Chainsaw builds step files only from directory entries that are not directories, so a directory named like 01-x.yaml makes nothing a suite. Both selectors listed it as one, and select-install demanded a mapping for it. A symlink still counts, as it does for Chainsaw. A directory named chainsaw-test.yaml keeps its suite listed: Chainsaw reads that name whatever its type and fails loudly on a directory, and an unlisted suite would hide that failure. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>

IvanHunters

LGTM. Careful fix, and I checked the central claim against the real tree instead of taking it on trust.

I ran the suite enumeration under hack/e2e-chainsaw/ on both the old and the new pattern. Both produce the same 23 suites, so "today's suite list stays the same" holds. The two parked suites (backup/ and kubernetes-proxmox/, both chainsaw-test.yaml.disabled) stay out as they should: the $-anchored \.ya?ml in the sed drops .disabled, and kubernetes-proxmox/tenant.yaml doesn't bring the suite back because it has no leading-number step shape. _lib/ has no numbered yaml, so it never shows up as a spurious suite.

Three things I confirmed, since these are the usual ways a selector regresses without anyone noticing:

The $ ! -type d -o -name chainsaw-test.yaml -o -name chainsaw-test.yml $ predicate is a broad OR. It emits almost everything and leaves the real filtering to the sed, so it is not an inverted filter that silently matches nothing. The bias is toward over-selecting, which is the safe direction for a test selector.
Ordering holds. The *.md/docs/ drop runs before the case block, so examples/backups/README.md stays inert, and examples/backups/*/* takes nested per-app paths before the new loose examples/backups/* arm, so the escalation doesn't swallow per-app selection.
hack/select-install.sh mirrors the same discovery. The cd "$1" is scoped to the command-substitution subshell, and || return 1 keeps a failed find distinct from an empty result.

Tests pass locally (the new step-file, nested, and loose-backup cases in both bats files), and CI is green on the head.

Non-blocking, and you flagged all three in the PR body yourself:

full_suite_list() in hack/select-e2e_test.bats:32 repeats the selector's own find/sed, so a bug in that shared pattern would slip through every full-suite assertion. Contained, since the step-file and nested cases assert against literal lists. [NIT]
The selector runs a touch wider than Chainsaw: a broken chainsaw-test.y*ml symlink or a Test-less suite still lists its directory. That errs toward running more. [NIT]
A few test comments still describe the old behaviour rather than the rule. [NIT]

Aleksei Sviridkin (lexfrei) requested review from IvanHunters, Kingdon Barrett (kingdonb), Andrei Kvapil (kvaps), Timofei Larkin (lllamnyp), myasnikovdaniil, Kirill Ilin (sircthulhu) and Timur Tukaev (tym83) as code owners October 1, 2026 15:57

github-actions Bot added size/L This PR changes 100-499 lines, ignoring generated files area/ci Issues or PRs related to CI workflows, GitHub Actions, automation kind/bug Categorizes issue or PR as related to a bug labels Oct 1, 2026

Aleksei Sviridkin (lexfrei) force-pushed the fix/select-e2e-blind-spots branch 3 times, most recently from 0853a14 to 9e777b3 Compare October 1, 2026 16:53

Aleksei Sviridkin (lexfrei) added 2 commits October 1, 2026 20:02

Aleksei Sviridkin (lexfrei) force-pushed the fix/select-e2e-blind-spots branch from 9e777b3 to 370f784 Compare October 1, 2026 17:27

Aleksei Sviridkin (lexfrei) added 2 commits October 1, 2026 20:35

Aleksei Sviridkin (lexfrei) force-pushed the fix/select-e2e-blind-spots branch from 370f784 to 0bd4c2b Compare October 1, 2026 17:59

Aleksei Sviridkin (lexfrei) force-pushed the fix/select-e2e-blind-spots branch from 0bd4c2b to 5fde704 Compare October 1, 2026 18:40

Aleksei Sviridkin (lexfrei) mentioned this pull request Oct 1, 2026

e2e: the leak-guard allowlist and the suite docs assume every suite is a chainsaw-test.yaml #4659

Open

IvanHunters approved these changes Oct 5, 2026

View reviewed changes

IvanHunters merged commit 02fbe76 into main Oct 5, 2026
19 checks passed

IvanHunters deleted the fix/select-e2e-blind-spots branch October 5, 2026 11:06

This was referenced Oct 5, 2026

test(e2e): spell the find negation as -not in select-e2e_test #4748

Closed

feat(platform, tests): select E2E packages and declare controller dependencies #4022

Merged

Navigation Menu

Repository navigation

fix(ci): close three blind spots in the e2e test selectors - #4658

fix(ci): close three blind spots in the e2e test selectors#4658
IvanHunters merged 5 commits into
mainfrom
fix/select-e2e-blind-spots

Aleksei Sviridkin (lexfrei) commented Oct 1, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 1, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

Uh oh!

IvanHunters left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Aleksei Sviridkin (lexfrei) commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Screenshots

Downstream repositories

Release note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Architecture Summary

Uh oh!

IvanHunters left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Aleksei Sviridkin (lexfrei) commented Oct 1, 2026 •

edited

Loading

coderabbitai Bot commented Oct 1, 2026 •

edited

Loading