Skip to content

select-e2e: a per-suite path naming no runnable suite has its escalation swallowed by any other selection #3816

Description

@myasnikovdaniil

hack/select-e2e.sh decides escalation per changed path everywhere except one arm. The per-suite Chainsaw rule at hack/select-e2e.sh:265-269 derives a suite name from the directory and appends it without checking that the name is a suite that exists:

hack/e2e-chainsaw/*/*)
  app=$(echo "$file" | sed -nE 's,^hack/e2e-chainsaw/([^/]+)/.*,\1,p')
  selected_apps="$selected_apps $app"
  trigger_any=1
  continue ;;

A name that no suite carries is dropped later by intersect_suites, so whether the path escalates depends on what else is in the diff. Alone it reaches the backstop at :413 and runs everything. Together with any path that contributes a real suite, the final selection is non-empty, the backstop never fires, and the unresolvable path is gone from the answer with nothing in the output to record it.

hack/e2e-chainsaw/backup/ is the one directory in the tree that holds no chainsaw-test.yaml, since its suite is parked as chainsaw-test.yaml.disabled. Using a hypothetical file in it against origin/main:

changed files selection stderr
hack/e2e-chainsaw/backup/values.yaml all 21 suites empty
the same file plus packages/apps/redis/values.yaml redis empty

This is the shape #3330 fixed for graph-resolved paths, where reading escalation off the merged selection let one contributing path swallow another's. That fix made coverage a property of the changed path. This arm still decides after the merge.

It is latent rather than live. backup/ contains only chainsaw-test.yaml.disabled, matched by the *.disabled rule before this one, and README.md, dropped by the documentation rule first, so no file in the tree today reaches it. It becomes reachable on the first non-.md file added under backup/, on any suite nested deeper than the depth-2 scan all_apps performs, or on a new directory added beside a suite before its chainsaw-test.yaml exists.

The fix is the one the sibling hack/e2e-apps/*.bats arm now uses: membership-test the derived name against all_apps before appending it, and escalate with a named reason when it is not a suite, so the decision stays a property of the path. A test belongs with it, because a rule that only misbehaves in a multi-path diff is invisible to a single-path test, which is how this survived.

Two things worth knowing for whoever picks it up. The backstop is the only thing that catches this class in a single-path diff, so its reason line is load-bearing rather than decorative. And an empty selection is not a safe default here: both e2e lanes read it as "skip Chainsaw" and then post the required E2E Tests status green, which is the failure #3392 exists to prevent.

Activity

  1. added
    kind/bugCategorizes issue or PR as related to a bug
    priority/backlogGeneral backlog priority. Lower than priority/important-longterm
    area/ciIssues or PRs related to CI workflows, GitHub Actions, automation
    area/testingIssues or PRs related to testing (e2e, bats, unit tests)
    on Aug 14, 2026
  2. added
    triage/acceptedIndicates an issue is ready to be actively worked on
    and removed
    triage/needs-triageIndicates an issue needs triage by a maintainer
    on Sep 23, 2026
  3. added a commit that references this issue on Oct 1, 2026
    cdc4b88
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

area/ciIssues or PRs related to CI workflows, GitHub Actions, automationarea/testingIssues or PRs related to testing (e2e, bats, unit tests)kind/bugCategorizes issue or PR as related to a bugpriority/backlogGeneral backlog priority. Lower than priority/important-longtermtriage/acceptedIndicates an issue is ready to be actively worked on

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions