Skip to content

Commit

Permalink
fix: flaky test `ERC721 NFTs testdapp interaction should prompt users…
Browse files Browse the repository at this point in the history
… to add their NFTs to their wallet (all at once)` (#27889)

<!--
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 the Deposit transaction by its
text in the activity tab, but this element updates its state, from
`pending`to `confirm`, meaning that it can become stale when we do the
assertion after (see video).

```
await driver.waitForSelector({
          css: '[data-testid="activity-list-item-action"]',
          text: 'Deposit',
});
assert.equal(await transactionItem.isDisplayed(), true);
```

To mitigate this problem, we wait until the transaction is confirmed,
and then look for the Deposit tx.
This not only fixes the race condition, but it also ensures that the tx
is successful.

<!--
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?
2. What is the improvement/solution?
-->

![Screenshot from 2024-10-16
08-35-56](https://github.com/user-attachments/assets/db176ec1-b71c-4019-bc67-7a4714c2c17b)


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

## **Related issues**

Fixes: #26759

## **Manual testing steps**

1. Check ci
## **Screenshots/Recordings**



https://github.com/user-attachments/assets/0e463ddf-f744-4428-8472-0ec2a585171e



### **Before**

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

### **After**

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

## **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.
  • Loading branch information
seaona authored Oct 16, 2024
1 parent 0edfb48 commit 95045ed
Showing 1 changed file with 14 additions and 8 deletions.
22 changes: 14 additions & 8 deletions test/e2e/tests/tokens/nft/erc721-interaction.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,25 +225,30 @@ describe('ERC721 NFTs testdapp interaction', function () {
await driver.clickElement({ text: 'Mint', tag: 'button' });

// Notification
await driver.waitUntilXWindowHandles(3);
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
await driver.waitForSelector({
css: '.confirm-page-container-summary__action__name',
text: 'Deposit',
});
await driver.clickElement({ text: 'Confirm', tag: 'button' });
await driver.waitUntilXWindowHandles(2);
await driver.clickElementAndWaitForWindowToClose({
text: 'Confirm',
tag: 'button',
});
await driver.switchToWindowWithTitle(
WINDOW_TITLES.ExtensionInFullScreenView,
);
await driver.clickElement(
'[data-testid="account-overview__activity-tab"]',
);
const transactionItem = await driver.waitForSelector({

// We need to wait until the transaction is confirmed before looking for the tx
// otherwise the element becomes stale, as it updates from 'pending' to 'confirmed'
await driver.waitForSelector('.transaction-status-label--confirmed');

await driver.waitForSelector({
css: '[data-testid="activity-list-item-action"]',
text: 'Deposit',
});
assert.equal(await transactionItem.isDisplayed(), true);
// verify the mint transaction has finished
await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp);
const nftsMintStatus = await driver.findElement({
Expand All @@ -255,7 +260,6 @@ describe('ERC721 NFTs testdapp interaction', function () {
// watch all nfts
await driver.clickElement({ text: 'Watch all NFTs', tag: 'button' });

await driver.waitUntilXWindowHandles(3);
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);

// confirm watchNFT
Expand All @@ -277,8 +281,10 @@ describe('ERC721 NFTs testdapp interaction', function () {
);
await removeButtons[0].click();

await driver.clickElement({ text: 'Add NFTs', tag: 'button' });
await driver.waitUntilXWindowHandles(2);
await driver.clickElementAndWaitForWindowToClose({
text: 'Add NFTs',
tag: 'button',
});

await driver.switchToWindowWithTitle(
WINDOW_TITLES.ExtensionInFullScreenView,
Expand Down

0 comments on commit 95045ed

Please sign in to comment.