Skip to content

Commit

Permalink
fix: flaky test `Request Queuing for Multiple Dapps and Txs on differ…
Browse files Browse the repository at this point in the history
…ent networks should batch confirmation txs for different dapps on different networks.` (#27095)

<!--
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**
In this test, there is an anti-pattern, where we first find an element
and then assert its inner value. This is a source of flakiness, since at
the moment we assert the value, it could be that it doesn't have the
expected value yet, making the assertion to fail.

```
let navigationElement = await driver.findElement(
  '.confirm-page-container-navigation',
);
let navigationText = await navigationElement.getText();
assert.equal(navigationText.includes('1 of 2'), true);
```

![Screenshot from 2024-09-12
10-55-08](https://github.com/user-attachments/assets/aba08bc4-563a-4b6e-95ba-f72d05aeae88)

Instead of this, we should try to directly find the element by its
value. This ensures that we wait until the element has the right value.

Note: in this case, since the value is broken down, we find the value by
the xpath.

![Screenshot from 2024-09-12
10-57-15](https://github.com/user-attachments/assets/4802725c-fc34-47d9-92e5-3da3e3ab1ca8)

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

## **Related issues**

Fixes: #26933

## **Manual testing steps**

1. Check ci

## **Screenshots/Recordings**
Above

## **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 Sep 12, 2024
1 parent 5053c3e commit 5701a0b
Showing 1 changed file with 5 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { strict: assert } = require('assert');
const { By } = require('selenium-webdriver');
const FixtureBuilder = require('../../fixture-builder');
const {
withFixtures,
Expand Down Expand Up @@ -124,14 +124,10 @@ describe('Request Queuing for Multiple Dapps and Txs on different networks', fun

await switchToNotificationWindow(driver, 4);

let navigationElement = await driver.findElement(
'.confirm-page-container-navigation',
await driver.findElement(
By.xpath("//div[normalize-space(.)='1 of 2']"),
);

let navigationText = await navigationElement.getText();

assert.equal(navigationText.includes('1 of 2'), true);

// Check correct network on confirm tx.
await driver.findElement({
css: '[data-testid="network-display"]',
Expand All @@ -149,14 +145,10 @@ describe('Request Queuing for Multiple Dapps and Txs on different networks', fun
// Wait for new confirmations queued from second dapp to open
await switchToNotificationWindow(driver, 4);

navigationElement = await driver.findElement(
'.confirm-page-container-navigation',
await driver.findElement(
By.xpath("//div[normalize-space(.)='1 of 2']"),
);

navigationText = await navigationElement.getText();

assert.equal(navigationText.includes('1 of 2'), true);

// Check correct network on confirm tx.
await driver.findElement({
css: '[data-testid="network-display"]',
Expand Down

0 comments on commit 5701a0b

Please sign in to comment.