Skip to content

Commit

Permalink
fix: flaky test `Speed Up and Cancel Transaction Tests Cancel transac…
Browse files Browse the repository at this point in the history
…tion Successfully cancels a pending transaction` (#30435)

<!--
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 our ganache instance, auto-mining is set with `blockTime: 2` , so a
new block is mined every 2 seconds.
In the specs cancel/speed up, we are sending a transaction with low gas,
so then we can speed it up/cancel. The problem is if this whole confirm
and speed up/cancel, happens in the same block (within the 2 seconds).

Then the transaction stays forever in a Pending state, and the spec
fails as it cannot find the correct tx status.
To fix this, we can make sure that we are working with a deterministic
block behaviour. So after we confirm the tx we mine a block, and after
we speed up/cancel, we mine another block.


![image](https://github.com/user-attachments/assets/d100db76-31f4-4df2-b966-b11e42451608)



Ci failure:
https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/125141/workflows/92ff1531-7350-490d-bfd1-811098db7535/jobs/4556227/tests

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

## **Related issues**

Fixes:

## **Manual testing steps**

1. Check ci webpack job
https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/126169/workflows/f76d3a08-d90e-4cae-8033-897e85cf29d6/jobs/4563925

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **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/main/.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/main/.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 Feb 19, 2025
1 parent e04e2f7 commit 468ab8f
Showing 1 changed file with 6 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { createDappTransaction } from '../../../page-objects/flows/transaction';
import Confirmation from '../../../page-objects/pages/confirmations/redesign/confirmation';
import ActivityListPage from '../../../page-objects/pages/home/activity-list';
import HomePage from '../../../page-objects/pages/home/homepage';
import { SMART_CONTRACTS } from '../../../seeder/smart-contracts';
import { TestSuiteArguments } from './shared';

const { WINDOW_TITLES, withFixtures } = require('../../../helpers');
Expand All @@ -28,10 +27,9 @@ describe('Speed Up and Cancel Transaction Tests', function () {
.withPermissionControllerConnectedToTestDapp()
.build(),
localNodeOptions: defaultGanacheOptionsForType2Transactions,
smartContract: SMART_CONTRACTS.PIGGYBANK,
title: this.test?.fullTitle(),
},
async ({ driver }: TestSuiteArguments) => {
async ({ driver, ganacheServer }: TestSuiteArguments) => {
await unlockWallet(driver);

// Create initial stuck transaction
Expand All @@ -43,7 +41,6 @@ describe('Speed Up and Cancel Transaction Tests', function () {
});

// Wait for confirmation dialog and confirm initial transaction
await driver.waitUntilXWindowHandles(3);
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);

const confirmationPage = new Confirmation(driver);
Expand All @@ -53,6 +50,7 @@ describe('Speed Up and Cancel Transaction Tests', function () {
await driver.switchToWindowWithTitle(
WINDOW_TITLES.ExtensionInFullScreenView,
);
await ganacheServer?.mineBlock();

const homePage = new HomePage(driver);
await homePage.goToActivityList();
Expand All @@ -63,6 +61,7 @@ describe('Speed Up and Cancel Transaction Tests', function () {
await activityListPage.click_transactionListItem();
await activityListPage.click_speedUpTransaction();
await activityListPage.click_confirmTransactionReplacement();
await ganacheServer?.mineBlock();

await activityListPage.check_waitForTransactionStatus('confirmed');
},
Expand All @@ -79,10 +78,9 @@ describe('Speed Up and Cancel Transaction Tests', function () {
.withPermissionControllerConnectedToTestDapp()
.build(),
localNodeOptions: defaultGanacheOptionsForType2Transactions,
smartContract: SMART_CONTRACTS.PIGGYBANK,
title: this.test?.fullTitle(),
},
async ({ driver }: TestSuiteArguments) => {
async ({ driver, ganacheServer }: TestSuiteArguments) => {
await unlockWallet(driver);

// Create initial stuck transaction
Expand All @@ -93,11 +91,11 @@ describe('Speed Up and Cancel Transaction Tests', function () {
to: DEFAULT_FIXTURE_ACCOUNT,
});

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

const confirmationPage = new Confirmation(driver);
await confirmationPage.clickFooterConfirmButton();
await ganacheServer?.mineBlock();

await driver.switchToWindowWithTitle(
WINDOW_TITLES.ExtensionInFullScreenView,
Expand All @@ -111,6 +109,7 @@ describe('Speed Up and Cancel Transaction Tests', function () {

await activityListPage.click_cancelTransaction();
await activityListPage.click_confirmTransactionReplacement();
await ganacheServer?.mineBlock();

await activityListPage.check_waitForTransactionStatus('cancelled');
},
Expand Down

0 comments on commit 468ab8f

Please sign in to comment.