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.

Sign up for GitHub

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

Jump to bottom

Reduce e2e Test Flakiness #17047

Closed
hilvmason opened this issue Dec 22, 2022 · 2 comments
Closed

Reduce e2e Test Flakiness #17047

hilvmason opened this issue Dec 22, 2022 · 2 comments
Assignees
@PeterYinusa
Labels
epic team-extension-platform

Comments

@hilvmason
Copy link
Contributor

Context:
In order to achieve this result, we need to Test Insights into the e2e test runs. This allows us to capture the following test metrics in Circle-CI: Flaky Tests, Most Failed Tests and Slowest Tests.
@hilvmason hilvmason added the epic label Dec 22, 2022
@hilvmason hilvmason changed the title Reduce e2e Test Flakiness KR-1: Reduce e2e Test Flakiness Jan 13, 2023
@hilvmason hilvmason changed the title KR-1: Reduce e2e Test Flakiness Reduce e2e Test Flakiness Feb 6, 2023
@github-actions github-actions bot added the stale issues and PRs marked as stale label Jul 20, 2023
@pedronfigueiredo pedronfigueiredo removed the stale issues and PRs marked as stale label Jul 20, 2023
@MetaMask MetaMask deleted a comment from github-actions bot Jul 20, 2023
@digiwand digiwand mentioned this issue Oct 16, 2023
Merged
14 tasks
@digiwand digiwand added the team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead label Oct 16, 2023
digiwand added a commit that referenced this issue Oct 17, 2023
@digiwand
test: Fix flaky e2e tests | Add retry logic to switchToWindowWithTitl…
957c8b9 
…e | Update retry helper fn (#21394)

## **Description**
We've recently had a [spike in flaky e2e
tests](https://app.circleci.com/insights/github/MetaMask/metamask-extension/workflows/test_and_release/tests).
We observed these tests to have a common denominator:
`switchToWindowWithTitle`. This PR adds retry logic to
`switchToWindowWithTitle`.

After the first try, the logic will retry a max number of 8 times with
2500ms delay in-between. These numbers are arbitrary and generous just
in case.

This PR also updates the retry logic in `development/lib/retry.js` to
return the function value on success.

Links:
- [CircleCI Insights - Flaky
Tests](https://app.circleci.com/insights/github/MetaMask/metamask-extension/workflows/test_and_release/tests)
- [Internal
discussion](https://consensys.slack.com/archives/GTQAGKY5V/p1697150920731179)

## **Manual testing steps**

1. Run CI/CD to observe tests

## **Screenshots/Recordings**

### **Before**

Screenshot 2023-10-16 21:00:
<img
src="https://github.com/MetaMask/metamask-extension/assets/20778143/f6cde1bb-b85e-4ceb-81ad-f7d8ba9f7ceb"
width="420" />

Screenshot 2023-10-16 21:50:
<img
src="https://github.com/MetaMask/metamask-extension/assets/20778143/71583e57-6e80-4129-a51f-560b65b2bbfa"
width="420" />

### **After**

## **Related issues**

Fixes #17047

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've clearly explained:
  - [ ] What problem this PR is solving.
  - [ ] How this problem was solved.
  - [ ] How reviewers can test my changes.
- [ ] I’ve indicated what issue this PR is linked to: Fixes #???
- [ ] I’ve included tests if applicable.
- [ ] I’ve documented any added code.
- [ ] 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)).
- [ ] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
@metamaskbot metamaskbot added the release-11.5.0 Issue or pull request that will be included in release 11.5.0 label Oct 17, 2023
@digiwand digiwand reopened this Oct 17, 2023
@digiwand digiwand removed the team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead label Oct 17, 2023
@digiwand
Copy link
Contributor

whoops, thought this was a regular issue until I opened Zenhub. It seems like a good place to relate following flakiness issues. will add new issues here

k-g-j pushed a commit that referenced this issue Nov 1, 2023
@digiwand @k-g-j
test: Fix flaky e2e tests | Add retry logic to switchToWindowWithTitl…
e0d30bf 
…e | Update retry helper fn (#21394)

## **Description**
We've recently had a [spike in flaky e2e
tests](https://app.circleci.com/insights/github/MetaMask/metamask-extension/workflows/test_and_release/tests).
We observed these tests to have a common denominator:
`switchToWindowWithTitle`. This PR adds retry logic to
`switchToWindowWithTitle`.

After the first try, the logic will retry a max number of 8 times with
2500ms delay in-between. These numbers are arbitrary and generous just
in case.

This PR also updates the retry logic in `development/lib/retry.js` to
return the function value on success.

Links:
- [CircleCI Insights - Flaky
Tests](https://app.circleci.com/insights/github/MetaMask/metamask-extension/workflows/test_and_release/tests)
- [Internal
discussion](https://consensys.slack.com/archives/GTQAGKY5V/p1697150920731179)

## **Manual testing steps**

1. Run CI/CD to observe tests

## **Screenshots/Recordings**

### **Before**

Screenshot 2023-10-16 21:00:
<img
src="https://github.com/MetaMask/metamask-extension/assets/20778143/f6cde1bb-b85e-4ceb-81ad-f7d8ba9f7ceb"
width="420" />

Screenshot 2023-10-16 21:50:
<img
src="https://github.com/MetaMask/metamask-extension/assets/20778143/71583e57-6e80-4129-a51f-560b65b2bbfa"
width="420" />

### **After**

## **Related issues**

Fixes #17047

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've clearly explained:
  - [ ] What problem this PR is solving.
  - [ ] How this problem was solved.
  - [ ] How reviewers can test my changes.
- [ ] I’ve indicated what issue this PR is linked to: Fixes #???
- [ ] I’ve included tests if applicable.
- [ ] I’ve documented any added code.
- [ ] 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)).
- [ ] I’ve properly set the pull request status:
  - [ ] In case it's not yet "ready for review", I've set it to "draft".
- [ ] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
@danjm danjm removed the release-11.5.0 Issue or pull request that will be included in release 11.5.0 label Nov 7, 2023
@gauthierpetetin
Copy link
Contributor

Closing this epic in favour of this one: #17755

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@PeterYinusa PeterYinusa

Labels
epic team-extension-platform
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@gauthierpetetin @danjm @pedronfigueiredo @digiwand @metamaskbot @PeterYinusa @hilvmason