Skip to content

E2E Gates reports success when E2E shards are skipped by an upstream failure #2043

Description

@steilerDev

Problem

E2E Gates — the main-only required check whose entire purpose is to block a betamain promotion when E2E fails — reports success when no E2E test has run at all.

Observed on the promotion PR #2041 (run 31104570445):

Check Result
E2E Cache Warmup ❌ failure
E2E Tests (Shard 1..16/16) ⏭️ skipped
E2E Smoke Tests ⏭️ skipped
Merge E2E Reports ⏭️ skipped
E2E Gates success

Mechanism

  1. e2e declares needs: [detect-changes, docker, e2e-warmup] (.github/workflows/ci.yml:523).

  2. When e2e-warmup fails, GitHub skips every dependent job — so all 16 shards become skipped, not failure.

  3. e2e-gates evaluates results with (.github/workflows/ci.yml:219):

    if [ "$result" != "success" ] && [ "$result" != "skipped" ]; then

    skipped is explicitly accepted, so the gate passes.

The skipped allowance is itself legitimate — it exists for the path-filter case, where e2e is skipped via if: needs.detect-changes.outputs.app == 'true' || needs.detect-changes.outputs.e2e == 'true' because nothing testable changed. The defect is that the check conflates two very different meanings of skipped: "there was nothing to test" and "an upstream job failed, so we never tested."

Impact

This is the last gate before code reaches main, and it can pass vacuously. It compounds an existing property of the setup: E2E Gates is required on main only, so commits merged to beta are never full-E2E-gated either. A promotion could therefore reach main with zero E2E tests executed against the range, with a green gate.

Concretely, on #2041 the warmup failed for an unrelated infrastructure reason (a competing apt-get held /var/cache/apt/archives/lock, so playwright install-deps exited 100 on all three retry attempts). The gate went green anyway. It was caught only by manually inspecting shard-level conclusions; re-running the failed jobs then produced a genuine 16/16 pass.

Suggested fix

Make the gate distinguish the two skip reasons:

  • Add e2e-warmup (and docker) to e2e-gates's needs and fail the gate when either did not succeed.
  • Only accept skipped for e2e / e2e-merge-reports when detect-changes reports nothing testable changed; otherwise require success.

Acceptance criteria

  1. When e2e-warmup fails, E2E Gates fails.
  2. When any E2E shard is skipped due to a failed upstream dependency, E2E Gates fails.
  3. When detect-changes reports no app/e2e changes and the shards are legitimately skipped, E2E Gates still passes (no regression to the path-filter path).
  4. A green E2E Gates on a main-targeted PR implies every shard actually reported success.

Notes

Found during /release for promotion PR #2041. Not fixed there deliberately — a CI-gate change does not belong bundled into a promotion.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions