Skip to content

Run phase7-e2e before merge#14

Merged
Galzi1 merged 1 commit into
mainfrom
fix-phase7-e2e-required
Apr 8, 2026
Merged

Run phase7-e2e before merge#14
Galzi1 merged 1 commit into
mainfrom
fix-phase7-e2e-required

Conversation

@Galzi1

@Galzi1 Galzi1 commented Apr 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • run the phase7-e2e job on normal pre-merge CI events instead of only merge queue events
  • keep the check name stable as phase7-e2e for branch protection
  • update the e2e marker description to match required pre-merge CI

Why

The job was being skipped for regular PR and push workflows because it was gated to merge_group only, which prevented it from acting as a required blocking check before merge.

Ensure the phase7-e2e workflow job runs on pull_request and push events instead of only merge_group, so it can be required by branch protection before merging. Also update the pytest marker description to reflect required pre-merge CI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Enable phase7-e2e as required pre-merge CI check

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Remove merge_group gate to run phase7-e2e on pull_request and push events
• Enable phase7-e2e as required pre-merge blocking check
• Update pytest marker description to reflect pre-merge CI requirement
• Add explicit job name for branch protection stability
Diagram
flowchart LR
  A["phase7-e2e job"] -->|Remove merge_group gate| B["Run on PR and push events"]
  B -->|Enable as required check| C["Branch protection blocking"]
  D["pytest marker"] -->|Update description| E["Pre-merge CI instead of merge-queue"]
Loading

Grey Divider

File Changes

1. .github/workflows/ci.yml ⚙️ Configuration changes +1/-1

Enable phase7-e2e on PR and push events

• Removed if: github.event_name == 'merge_group' condition from phase7-e2e job
• Added explicit name: phase7-e2e for stable check naming in branch protection
• Job now runs on pull_request and push events by default

.github/workflows/ci.yml


2. pyproject.toml 📝 Documentation +1/-1

Update e2e marker description for pre-merge CI

• Updated e2e pytest marker description from "merge-queue CI" to "required pre-merge CI"
• Clarifies the marker's purpose for pre-merge validation

pyproject.toml


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Apr 8, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📎 Requirement gaps (0)   🎨 UX Issues (0)
🐞\ ➹ Performance (1)

Grey Divider


Remediation recommended

1. E2E runs on all pushes 🐞
Description
Removing the job-level if: github.event_name == 'merge_group' makes phase7-e2e execute for every
workflow trigger, including the unfiltered push event. This can run the e2e suite on
feature-branch/tag pushes and can duplicate CI work for same-repo PRs (one run from push, one from
pull_request).
Code

.github/workflows/ci.yml[R45-47]

  phase7-e2e:
-    if: github.event_name == 'merge_group'
+    name: phase7-e2e
    runs-on: ubuntu-latest
Evidence
ci.yml is configured to run on push without any branch/tag filters, and the phase7-e2e job no
longer has an if: guard; therefore it will run on every push in addition to PR/merge-queue events.

.github/workflows/ci.yml[3-10]
.github/workflows/ci.yml[45-47]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`phase7-e2e` is now unconditional, and since the workflow triggers on unfiltered `push`, it will run on every branch/tag push and can also duplicate work alongside `pull_request` runs for the same commit.

### Issue Context
The PR removed the `merge_group`-only guard but the workflow still has `on: push:` with no filters.

### Fix Focus Areas
- .github/workflows/ci.yml[3-10]
- .github/workflows/ci.yml[45-47]

### Suggested fix
Choose one of these (ordered by typical “required pre-merge CI” intent):
1) Restrict workflow pushes to `main` only (or remove `push` entirely if PR CI is sufficient):
  - `on.push.branches: [main]`
  - optionally ignore tags (e.g., `tags-ignore: ['**']`)
2) Keep `push` but gate only `phase7-e2e` to intended events, e.g.:
  - `if: github.event_name != 'push' || github.ref == 'refs/heads/main'`
3) If you intentionally want both `push` and `pull_request`, add a `concurrency` group to cancel superseded runs to avoid duplicate e2e execution per branch/PR.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@Galzi1
Galzi1 merged commit bc97480 into main Apr 8, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant