Skip to content
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

Open
9 tasks
cortisiko opened this issue Oct 10, 2024 · 2 comments
Open
9 tasks

Enable E2E in merge queue #11747

cortisiko opened this issue Oct 10, 2024 · 2 comments
Assignees

Comments

@cortisiko
Copy link
Member

cortisiko commented Oct 10, 2024

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.

 is-documentation-pull-request:
    name: Check if pull request is for documentation
    runs-on: ubuntu-latest
    outputs:
      IS_DOCS_PR: ${{ steps.is-docs.outputs.IS_DOCS_PR }}
    steps:
      - uses: actions/checkout@v3
      - name: Check if docs PR
        id: is-docs
        run: |
          if [[ "${{ startsWith(github.event.pull_request.title, 'docs:') }}" == "true" ]]; then
            echo "Is a documentation PR, will skip running e2e"
          else
            echo "Is not a documentation PR will proceed to running tests"
            fi

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

  • Engineering (needed in most cases)
  • Design
  • Product
  • QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
  • Security
  • Legal
  • Marketing
  • Management (please specify)
  • Other (please specify)

References

No response

@jake-perkins
Copy link
Contributor

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
for example here an example webhook payload for MQ ( merge_group )

{"base_ref":"refs/heads/tester","base_sha":"a9e5e81f11be20533feecccdeaa6fd3c94cee6be","head_commit":{"author":{"email":"[email protected]","name":"jake-perkins"},"committer":{"email":"[email protected]","name":"GitHub"},"id":"fb3feb4e819347d00eebc2ea058bf54889d6027d","message":"Merge pull request #5 from MetaMask/mq-checker\n\nchecker","timestamp":"2024-11-08T04:00:32Z","tree_id":"2f88d615e79f27ef6ca57c411915bac40e9fb925"},
"head_ref":"refs/heads/gh-readonly-queue/tester/pr-5-a9e5e81f11be20533feecccdeaa6fd3c94cee6be","head_sha":"fb3feb4e819347d00eebc2ea058bf54889d6027d"}

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 :
One recommendation/alternate suggested was to simply make the status check a required check on the branch protection rule for main
- One potential unknown here is if the check being assigned to a random check bucket will impact
the gate, we can quickly test on this one though
( CLA Signature Bot / Bitrise E2E Status (pull_request_target) ) is how it currently shows

Option 2:
Auto Draft PRs on Open + Run E2E checks on Ready to Review PRs only

@jake-perkins
Copy link
Contributor

#12243

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

No branches or pull requests

3 participants