-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable E2E in merge queue #11747
Comments
The story ask is to run the e2e in the merge queue and I have been able to isolate some ci run flows for just merge queue process and run the e2e invocation points. However the way MQ works it actually makes an alternate read-only branch distinct and separate from the originating PR that was added to MQ with different commit shas/branch names which is unfortunate. the e2e bitrise process involves several mutating requests to both GH API/BitRise API I don't think it is feasible given some of the constraints around merge_group event being separate/distinct from pull_request event {"base_ref":"refs/heads/tester","base_sha":"a9e5e81f11be20533feecccdeaa6fd3c94cee6be","head_commit":{"author":{"email":"128608287+jake-perkins@users.noreply.github.com","name":"jake-perkins"},"committer":{"email":"noreply@github.com","name":"GitHub"},"id":"fb3feb4e819347d00eebc2ea058bf54889d6027d","message":"Merge pull request #5 from MetaMask/mq-checker\n\nchecker","timestamp":"2024-11-08T04:00:32Z","tree_id":"2f88d615e79f27ef6ca57c411915bac40e9fb925"}, Basically it spins up a secondary special ( gh-readonly-queue/ ) branch and all the shas are different which will break some of the linkage between Bitrise /GH. and all of the mutating API calls will fail to GH Option 1 : Option 2: |
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** This PR does a few things 1. Automatically set new PRs targeting main to "draft" state. 2. Enforce the E2E Bitrise Status Check on lifecycle on incoming PRs which will essentially enforce that the PR has 1 of the two labels 'Run Smoke E2E' or 'No E2E Smoke Needed' <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 4. What is the improvement/solution? --> With this change we're looking to improve the posture on our PR lifecycle with auto drafts and enforce a higher standard for incoming changes towards or E2E testing posture. ## **Related issues** [Fixes:] #11747 ## **Manual testing steps** UC1 : Nether E2E Label is set -> https://github.com/MetaMask/metamask-mobile/actions/runs/11826040868/job/32951145968?pr=12243 Expected : Not Mergeable UC2 : docs PR Should run: false, Reason: The pull request is documentation related. Skipping Bitrise status check. due to the following reason: The pull request is documentation related. Created 'Bitrise E2E Status' check with skipped status for commit dea08a9 https://github.com/MetaMask/metamask-mobile/actions/runs/11826187817/job/32951608300?pr=12243 Expected: Mergeable UC3 : No E2E Smoke Needed Label is set Anti label: true Should run: false, Reason: The pull request has the anti-label. Skipping Bitrise status check. due to the following reason: The pull request has the anti-label. Created 'Bitrise E2E Status' check with skipped status for commit dea08a9[22] https://github.com/MetaMask/metamask-mobile/actions/runs/11826234735/job/32951749305?pr=12243 Expected Mergeable UC4: Smoke E2E Label is set Should run: true, Reason: The smoke test label is present. Starting Bitrise build for commit dea08a9 ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> N/A CICD Changes only ### **After** <!-- [screenshots/recordings] --> N/A CICD Changes only ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Cal Leung <cal.leung@consensys.net>
Latest approach has showed very promising results using the checks CRUD API - PR here #12648 |
Hi @jake-perkins , what is the latest status of this issue? Is it on purpose that its status is set to "closed" on our Github Project board? |
Almost done, will be wrapped up tomorrow |
What is this about?
To improve the stability and reliability of our main branch, we are implementing a strategy to reduce the number of PRs that introduce failed E2E tests. Our goal is to enable E2E testing by default for any PR that enters the merge queue, ensuring that only thoroughly validated code makes it into the main branch. This will help catch potential issues early, prevent broken code from being merged, and increase overall confidence in our CI pipeline.
The Approach:
Keep the current logic where we do not require the e2e check on feature branches.
Enforce E2E Checks in the Merge Queue: We should set a branch protection rule that requires E2E tests to run on all PRs that are added to the merge queue. This ensures that every PR is subject to automated testing before it reaches main, reducing the likelihood of undetected issues.
Skip E2E for Documentation PRs: To avoid wasting resources, the E2E tests will be automatically skipped for PRs whose titles contain "docs." This helps streamline the process for non-code changes, ensuring that documentation updates can be merged quickly without unnecessary testing overhead. The below job was created to verify whether a PR follows the conventional naming format by including "docs" in its title.
Please feel free to use this if you see fit.
Here is PR that introduced the e2e check in GitHub Actions: #8495
Scenario
No response
Design
No response
Technical Details
No response
Acceptance Criteria
No response
Stakeholder review needed before the work gets merged
References
No response
The text was updated successfully, but these errors were encountered: