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

ci: Prevent E2E timeouts on release changes (#26846) [cherry-pick] #26856

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Sep 3, 2024

This is a cherry-pick of #26846 for v12.2.0. Original description:

Description

Our CI was setup to compare each branch with develop, and run any tests that have changed since develop extra times to catch new flaky test regressions. Unfortunately this was happening even for PRs that do not target develop, resulting in massive lists of "changed" tests that ended up causing persistent test timeouts.

The quality gate should only impact PRs that target develop. PRs that target other branches no longer repeat "changed" tests. This should fix release PR e2e test timeouts caused by excessive e2e test runs.

You can see an example of this problem occurring here: https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/98357/workflows/b61a16b5-acfd-411f-b82e-7a31706ca658/jobs/3660843 Notice that
/home/circleci/project/test/e2e/tests/request-queuing/ui.spec.js appears 6 times in the full test list, as to every other "changed" test.

Open in GitHub Codespaces

Related issues

This quality gate was first introduced in #24556

Manual testing steps

The script can be run locally if you setup "fake" CircleCI environment variables to allow it to run properly. For example:

CIRCLE_PULL_REQUEST=https://github.com/MetaMask/metamask-extension/pull/26822 yarn tsx ./.circleci/scripts/git-diff-develop.ts

The CI run for this PR can be viewed as well:
https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/98369/workflows/e9de2170-a19e-433e-a83c-846eeb239842/jobs/3661034

Screenshots/Recordings

N/A

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • 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.

## **Description**

Our CI was setup to compare each branch with `develop`, and run any
tests that have changed since `develop` extra times to catch new flaky
test regressions. Unfortunately this was happening even for PRs that do
not target develop, resulting in massive lists of "changed" tests that
ended up causing persistent test timeouts.

The quality gate should only impact PRs that target `develop`. PRs that
target other branches no longer repeat "changed" tests. This should fix
release PR e2e test timeouts caused by excessive e2e test runs.

You can see an example of this problem occurring here:
https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/98357/workflows/b61a16b5-acfd-411f-b82e-7a31706ca658/jobs/3660843
Notice that
`/home/circleci/project/test/e2e/tests/request-queuing/ui.spec.js`
appears 6 times in the full test list, as to every other "changed" test.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26846?quickstart=1)

## **Related issues**

This quality gate was first introduced in #24556

## **Manual testing steps**

The script can be run locally if you setup "fake" CircleCI environment
variables to allow it to run properly. For example:

```
CIRCLE_PULL_REQUEST=#26822 yarn tsx ./.circleci/scripts/git-diff-develop.ts
```


The CI run for this PR can be viewed as well:
https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/98369/workflows/e9de2170-a19e-433e-a83c-846eeb239842/jobs/3661034

## **Screenshots/Recordings**

N/A

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.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-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] 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.
@Gudahtt Gudahtt marked this pull request as ready for review September 3, 2024 12:32
@Gudahtt Gudahtt requested review from kumavis and a team as code owners September 3, 2024 12:32
@Gudahtt Gudahtt merged commit 7a5acb3 into Version-v12.2.0 Sep 3, 2024
69 of 70 checks passed
@Gudahtt Gudahtt deleted the cherry-pick/prevent-e2e-test-timeouts branch September 3, 2024 12:52
@github-actions github-actions bot locked and limited conversation to collaborators Sep 3, 2024
@metamaskbot metamaskbot added the release-12.2.0 Issue or pull request that will be included in release 12.2.0 label Sep 3, 2024
@metamaskbot
Copy link
Collaborator

No release label on PR. Adding release label release-12.2.0 on PR, as PR was cherry-picked in branch 12.2.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.2.0 Issue or pull request that will be included in release 12.2.0 team-wallet-framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants