Skip to content

Commit

Permalink
fix: flaky test `Swap-Send ETH to non-contract address with data that…
Browse files Browse the repository at this point in the history
… matches swap data signature submits a transaction successfully` (#25545)

<!--
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**

The problem is that we are looking for a pending transaction and then a
confirmed transaction. If the transaction is confirmed very fast, the
pending transaction is never found, thus making the test fail.
This is a bad pattern that introduces flakiness, as we are trying to
find an element by its transient state, which is variable. What we
really want to assert is that the transaction is in the end confirmed,
so we shouldn't try to look for the pending transaction (something that
disappears fast) in the first place.


Error:

```
TimeoutError: Waiting for element to be located By(css selector, .transaction-status-label--pending)
Wait timed out after 10081ms
```
[ci
error](https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/89418/workflows/924ea8b8-1698-4dca-ae6e-b724264fe4a2/jobs/3304211/parallel-runs/17?filterBy=ALL)

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

## **Related issues**

Fixes: #25546

## **Manual testing steps**

1. Check ci

## **Screenshots/Recordings**
See the screenshot in ci, how the expected transaction is indeed there
(confirmed). However, on the error we couldn't locate the element when
it was pending (possibly due to being really fast at occasions)


![image](https://github.com/MetaMask/metamask-extension/assets/54408225/6dd42cb2-d71a-4587-b663-49a424493e72)

![Screenshot from 2024-06-27
09-51-55](https://github.com/MetaMask/metamask-extension/assets/54408225/2eab4f8e-bba1-4c23-ab7a-cb141a5a30e9)


<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] 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).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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.
  • Loading branch information
seaona authored Jun 27, 2024
1 parent e01083d commit 1d33d6f
Showing 1 changed file with 0 additions and 6 deletions.
6 changes: 0 additions & 6 deletions test/e2e/tests/swap-send/swap-send-eth.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,6 @@ describe('Swap-Send ETH', function () {
// TODO assert swap api request payload

await swapSendPage.submitSwap();
await swapSendPage.verifyHistoryEntry(
'Send ETH as TST',
'Pending',
'-1 ETH',
'',
);
await swapSendPage.verifyHistoryEntry(
'Send ETH as TST',
'Confirmed',
Expand Down

0 comments on commit 1d33d6f

Please sign in to comment.