From aac9e25dc6f2edd46b11edbbf754d37cd916c3e3 Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Thu, 27 Aug 2026 16:40:59 +0300 Subject: [PATCH 1/2] ci(e2e): gate merges on E2E through the merge queue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the `merge_group` trigger to the PR pipeline and to the E2E orchestrator, plus an `E2E tests passed` aggregate job to require on `main` alongside `All required checks pass`. A queue entry skips the suites when the PR head commit already carries a passing `E2E tests passed` check — from the `e2e-tests` label or a dependabot push — or when the PR touches nothing E2E covers. Every unexpected case runs the suites instead of waving the merge through. In merge queue runs `dorny/paths-filter` has to keep its `base` input empty so its own base_sha..head_sha handling applies; `base: main` collapses the diff there and every suite would skip. Enabling the queue and marking the two checks required are branch protection changes still to be made. Refs #RI-8373 Co-Authored-By: Claude Fable 5 --- .github/workflows/tests-e2e-playwright-v2.yml | 185 +++++++++++++++++- .github/workflows/tests.yml | 30 ++- 2 files changed, 203 insertions(+), 12 deletions(-) diff --git a/.github/workflows/tests-e2e-playwright-v2.yml b/.github/workflows/tests-e2e-playwright-v2.yml index c5693e8f9e..d027ad1809 100644 --- a/.github/workflows/tests-e2e-playwright-v2.yml +++ b/.github/workflows/tests-e2e-playwright-v2.yml @@ -10,13 +10,19 @@ name: E2E Playwright Tests (v2) # 3. Someone adds `e2e-tests` or `run-all-tests` to a PR. # 4. The nightly schedule, at 00:00 UTC. # 5. Someone starts it by hand from the Actions tab. +# 6. A PR enters the merge queue for `main`, and its head commit has no +# successful E2E run yet. This is the merge gate: `E2E tests passed` is a +# required check, so nothing reaches `main` without E2E behind it. # # `skip-e2e` on a PR overrides cases 1, 2 and 3. It stops a run already going and -# keeps later ones off. +# keeps later ones off. It does not apply to case 6 — a label must not be able to +# waive the merge gate. # # Opening a PR or pushing to it does not start E2E unless the branch is # `dependabot/**`. Case 3 is how you ask for it, and removing then re-adding the -# label is how you rerun it. +# label is how you rerun it. Cases 1-3 and 5 all publish the `E2E tests passed` +# check on the PR's head commit, which is what case 6 looks for to decide it has +# nothing left to do. on: # Case 5. @@ -47,9 +53,14 @@ on: schedule: - cron: '0 0 * * *' + # Case 6. Fires for every merge queue entry; `check-trigger` decides whether + # the entry needs a run or already has one. + merge_group: + concurrency: # One run per branch at a time. A new push replaces the one already going, - # which is what case 2 needs. + # which is what case 2 needs. Merge queue entries (case 6) each get a ref of + # their own, so they never cancel one another or a PR's own run. # # The key is the branch rather than the PR so that a push (cases 1 and 2) and a # label (case 3) on the same PR land in the same group. That is what lets @@ -74,9 +85,93 @@ jobs: permissions: contents: read pull-requests: read + # Reading the E2E check on a queued PR's head commit (case 6). + checks: read outputs: should_run: ${{ steps.check.outputs.should_run }} + skip_reason: ${{ steps.check.outputs.skip_reason }} steps: + # Case 6. A merge queue entry has no PR in its payload, but its ref is + # `gh-readonly-queue//pr--`, so the number comes from + # there. Anything unexpected leaves both outputs empty, and the run goes + # ahead: failing open here would merge untested code. + - name: Look for an E2E run on the queued PR's head commit + id: queued-pr + if: github.event_name == 'merge_group' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + REF_NAME: ${{ github.ref_name }} + # Must match the `name:` of the `e2e-passed` job below. That job + # publishes the check this reads; renaming one without the other + # means every queue entry reruns E2E from scratch. + GATE_CHECK_NAME: E2E tests passed + run: | + pr_number=$(sed -nE 's|.*/pr-([0-9]+)-[0-9a-f]+$|\1|p' <<< "$REF_NAME") + if [[ -z "$pr_number" ]]; then + echo "Could not read a PR number from '$REF_NAME' — running E2E." + exit 0 + fi + echo "Merge queue entry for PR #$pr_number." + + head_sha=$(gh api "repos/$GH_REPO/pulls/$pr_number" --jq '.head.sha' || true) + if [[ -z "$head_sha" ]]; then + echo "Could not read the head commit of PR #$pr_number — running E2E." + exit 0 + fi + + # A skipped or failed check must not count, so match the conclusion + # too. `--paginate` because a commit can carry many checks, and it + # prints one count per page, hence the sum. + # + # `|| true` keeps a flaky API call from failing the step: no answer + # means no known passing run, which runs E2E rather than blocking the + # merge on an unrelated hiccup. The step runs under `pipefail`, so the + # guard has to sit inside the substitution. + passed=$( { gh api "repos/$GH_REPO/commits/$head_sha/check-runs" --paginate \ + --jq '[.check_runs[] | select(.name == env.GATE_CHECK_NAME and .conclusion == "success")] | length' \ + || true; } | awk '{ total += $1 } END { print total + 0 }') + + if [[ "${passed:-0}" -gt 0 ]]; then + echo "already_passed=true" >> "$GITHUB_OUTPUT" + echo "head_sha=$head_sha" >> "$GITHUB_OUTPUT" + echo "PR #$pr_number already has '$GATE_CHECK_NAME' on $head_sha." + else + echo "No '$GATE_CHECK_NAME' on $head_sha — running E2E." + fi + + # Nothing E2E covers can change without one of these paths changing, so a + # docs-only or workflow-comment-only PR does not need three suites and two + # builds to prove it. Checked per PR, not per merge group: every entry in a + # batch passes through here with its own number. + - name: Check the queued PR for changes E2E can see + id: queued-paths + if: github.event_name == 'merge_group' && steps.queued-pr.outputs.already_passed != 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + REF_NAME: ${{ github.ref_name }} + run: | + pr_number=$(sed -nE 's|.*/pr-([0-9]+)-[0-9a-f]+$|\1|p' <<< "$REF_NAME") + [[ -z "$pr_number" ]] && exit 0 + + files=$(gh api "repos/$GH_REPO/pulls/$pr_number/files" --paginate \ + --jq '.[].filename' || true) + if [[ -z "$files" ]]; then + echo "Could not list the files of PR #$pr_number — running E2E." + exit 0 + fi + + # Application code, the E2E suite itself, the test environment, the + # build inputs, and the workflows that run all of it. + if grep -qE '^(redisinsight/|tests/e2e|package(-lock)?\.json|\.nvmrc|Dockerfile|\.dockerignore|configs/|scripts/|\.github/(workflows|actions|build)/)' <<< "$files"; then + echo "PR #$pr_number touches paths E2E covers." + else + echo "no_relevant_changes=true" >> "$GITHUB_OUTPUT" + echo "PR #$pr_number touches nothing E2E covers:" + sed 's/^/ /' <<< "$files" + fi + # A push event carries no PR, so the labels have to be fetched to honour # `skip-e2e` in cases 1 and 2. - name: Read labels for the pushed branch @@ -99,17 +194,32 @@ jobs: LABEL_ADDED: ${{ github.event.label.name }} PUSHED_PR_LABELS: ${{ steps.pushed-pr.outputs.labels }} HAS_SKIP_LABEL: ${{ contains(github.event.pull_request.labels.*.name, 'skip-e2e') }} + MQ_ALREADY_PASSED: ${{ steps.queued-pr.outputs.already_passed }} + MQ_NO_RELEVANT_CHANGES: ${{ steps.queued-paths.outputs.no_relevant_changes }} + MQ_HEAD_SHA: ${{ steps.queued-pr.outputs.head_sha }} run: | # Case 3. The same list appears in the concurrency group above, next to # `skip-e2e`. Changing one without the other breaks quietly. OPT_IN_LABELS='e2e-tests run-all-tests' should_run=false + skip_reason='' case "$EVENT_NAME" in workflow_dispatch | schedule) # Cases 4 and 5, always. should_run=true ;; + merge_group) + # Case 6. Run unless the two steps above found a reason not to. + # `skip-e2e` is deliberately not consulted here. + if [[ "$MQ_ALREADY_PASSED" == "true" ]]; then + skip_reason="E2E already passed on the PR head commit ($MQ_HEAD_SHA)." + elif [[ "$MQ_NO_RELEVANT_CHANGES" == "true" ]]; then + skip_reason='The PR changes nothing E2E covers.' + else + should_run=true + fi + ;; push) # Cases 1 and 2. The branch filter has already narrowed this to # dependency branches, so `skip-e2e` is the only thing left to check. @@ -130,7 +240,15 @@ jobs: fi ;; esac - echo "should_run=$should_run" >> "$GITHUB_OUTPUT" + + { + echo "should_run=$should_run" + echo "skip_reason=$skip_reason" + } >> "$GITHUB_OUTPUT" + + if [[ -n "$skip_reason" ]]; then + echo "Not running E2E: $skip_reason" + fi # Lint and type-check E2E code lint: @@ -191,6 +309,65 @@ jobs: environment: ${{ inputs.environment || 'staging' }} debug: ${{ inputs.debug || false }} + # The one E2E check to read, and the one to require on `main`. On a merge queue + # entry it is green either because the suites just passed on the queue's branch + # or because `check-trigger` found a passing run on the PR head commit. + # + # On cases 1-5 it publishes that same passing run, which is what a later queue + # entry looks for. So it must not go green when nothing ran: a skipped job + # reports `skipped`, not `success`, and the lookup ignores it. + # + # A job left out of `needs` cannot block a merge, so add new suites here. + e2e-passed: + # Renaming this also means changing GATE_CHECK_NAME in `check-trigger`. + name: E2E tests passed + # Queue entries always report. Everything else reports only when it ran. + if: ${{ always() && (github.event_name == 'merge_group' || needs.check-trigger.outputs.should_run == 'true') }} + needs: + - check-trigger + - lint + - e2e-dev-chromium + - build-docker + - e2e-docker + - build-linux + - e2e-electron + runs-on: ubuntu-latest + steps: + - name: Check the jobs this waits on + env: + EVENT_NAME: ${{ github.event_name }} + CHECK_TRIGGER_RESULT: ${{ needs.check-trigger.result }} + SHOULD_RUN: ${{ needs.check-trigger.outputs.should_run }} + SKIP_REASON: ${{ needs.check-trigger.outputs.skip_reason }} + NEEDS_JSON: ${{ toJSON(needs) }} + run: | + # The skip path needs `check-trigger` to have actually reached a + # decision. Without this, a crashed `check-trigger` leaves SHOULD_RUN + # empty and the gate would wave the merge through. + if [[ "$EVENT_NAME" == "merge_group" \ + && "$CHECK_TRIGGER_RESULT" == "success" \ + && "$SHOULD_RUN" != "true" ]]; then + echo "E2E did not need to run for this merge queue entry." + echo "${SKIP_REASON:-No reason recorded.}" + exit 0 + fi + + # Unlike the unit/integration gate, a skip is a failure here: every + # job below was meant to run, so a skip means the graph broke. + failing=$(jq -r ' + to_entries[] + | select(.value.result != "success") + | " \(.key): \(.value.result)" + ' <<< "$NEEDS_JSON") + + if [ -n "$failing" ]; then + echo "These jobs did not pass:" + echo "$failing" + exit 1 + fi + + echo "Every E2E job passed." + # Alert on any non-success nightly result (incl. 'cancelled' from a timeout). notify-slack: name: Notify Slack on nightly failure diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 921700846e..bb6031d168 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -1,7 +1,8 @@ name: ✅ Tests -# Runs on every pull request. The changed files pick the suites; the -# `run-*-tests` labels can widen that. +# Runs on every pull request and on every merge queue entry. The changed files +# pick the suites; the `run-*-tests` labels can widen that (labels exist only on +# pull request events). # # Do not add `labeled` here. `tests-on-demand.yml` owns that trigger. A label # reaching this workflow reports a second `All required checks pass` on the same @@ -11,6 +12,11 @@ on: pull_request: types: [opened, synchronize, reopened] + # Merge queue entries for `main`. The queue's temporary branch gets the same + # suites as a PR run; `All required checks pass` is the required check that + # gates the merge either way. + merge_group: + workflow_dispatch: inputs: redis_client: @@ -71,7 +77,13 @@ jobs: # Compare against main so each push's gating reflects the # cumulative branch diff, not just the latest commit. Keeps # the merge-gate aligned with what would actually land. - base: main + # + # On merge_group events the input must stay empty: a manual `base` + # overrides the action's built-in merge-queue handling, which diffs + # the entry's own base_sha..head_sha — exactly what the queue would + # land on main. `base: main` there collapses the diff to nothing and + # every suite would skip. + base: ${{ github.event_name != 'merge_group' && 'main' || '' }} filters: | frontend: - 'redisinsight/ui/**' @@ -200,7 +212,8 @@ jobs: frontend-tests-coverage: needs: frontend-tests - if: ${{ github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.fork != true }} + # Coverage attaches to a PR, so merge-queue runs have nowhere to report it. + if: ${{ github.event_name != 'merge_group' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.fork != true }} uses: ./.github/workflows/code-coverage.yml secrets: inherit with: @@ -215,7 +228,7 @@ jobs: backend-tests-coverage: needs: backend-tests - if: ${{ github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.fork != true }} + if: ${{ github.event_name != 'merge_group' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.fork != true }} uses: ./.github/workflows/code-coverage.yml secrets: inherit with: @@ -238,7 +251,7 @@ jobs: integration-tests-coverage: needs: integration-tests - if: ${{ github.actor != 'dependabot[bot]' }} + if: ${{ github.event_name != 'merge_group' && github.actor != 'dependabot[bot]' }} uses: ./.github/workflows/code-coverage.yml secrets: inherit with: @@ -248,8 +261,9 @@ jobs: clean: uses: ./.github/workflows/clean-deployments.yml # `always()` runs this even when every test job skips, so it needs its own - # guard against deleting deployments for a run that did no work. - if: ${{ always() && needs.changes.result != 'skipped' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.fork != true }} + # guard against deleting deployments for a run that did no work. Merge-queue + # runs have no PR deployments to clean. + if: ${{ always() && github.event_name != 'merge_group' && needs.changes.result != 'skipped' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.fork != true }} permissions: actions: write contents: read From ec82dbe621b5d7f3a21e1a88aaa381a705f750e2 Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Thu, 27 Aug 2026 16:49:44 +0300 Subject: [PATCH 2/2] docs(ci): note that the E2E merge gate needs queue group size 1 The prior-pass lookup reads the single PR named in the merge queue ref. GitHub batches up to 5 PRs per group by default, and a batched group's ref names only its newest PR, so a batch could skip E2E on the strength of a different PR's passing run. Co-Authored-By: Claude Fable 5 --- .github/workflows/tests-e2e-playwright-v2.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/tests-e2e-playwright-v2.yml b/.github/workflows/tests-e2e-playwright-v2.yml index d027ad1809..0fd59b5801 100644 --- a/.github/workflows/tests-e2e-playwright-v2.yml +++ b/.github/workflows/tests-e2e-playwright-v2.yml @@ -95,6 +95,11 @@ jobs: # `gh-readonly-queue//pr--`, so the number comes from # there. Anything unexpected leaves both outputs empty, and the run goes # ahead: failing open here would merge untested code. + # + # This reads one PR, so the queue's maximum group size has to stay at 1. + # A batched group names only its newest PR in the ref, so a batch could + # skip E2E on the strength of a different PR's passing run. Raising the + # group size means reworking this step to cover every PR in the group. - name: Look for an E2E run on the queued PR's head commit id: queued-pr if: github.event_name == 'merge_group'