fix: flaky test 25912 #25913

hjetpoluru · 2024-07-17T20:47:15Z

Description

This PR is to fix the flaky test associated with the 'User Operations from swap' test case. While I couldn't replicate the issue locally, the screenshots suggest a delay in transitioning from the swap action to clicking the close button.
Upon further investigation with @seaona, we determined that processing the swap transaction was taking time, as shown in the screenshot below. Consequently, a mock for the swap has been introduced.

Related issues

Fixes: #25912

Manual testing steps

Run the test locally or in codespaces using below commands:
yarn
yarn build:test:flask
yarn test:e2e:single test/e2e/flask/user-operations.spec.ts --browser=chrome

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
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 format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). 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.

github-actions · 2024-07-17T20:47:29Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

seaona · 2024-07-18T07:01:08Z

test/e2e/flask/user-operations.spec.ts

@@ -102,7 +102,16 @@ async function createSwap(driver: Driver) {
    swapFrom: 'TESTETH',
    swapTo: 'USDC',
  });
+
+  await driver.waitForSelector({ text: 'Swap', tag: 'button' });


this won''t have any effect as the clickElement function already waits for the element to be visible and clickable

seaona · 2024-07-18T07:06:17Z

test/e2e/flask/user-operations.spec.ts

+    {
+      text: 'View in activity',
+      tag: 'button',
+    },


it seems that this is still no enough. From the flow we can know that:

the View in activity remains present until the tx is confirmed

once the tx is confirmed, the View in activity disappears and the Close button appears

In the failed tests screenshots we can see how we are still in the View activity phase, meaning that the transaction is not yet confirmed. I think we could investigate why the transaction takes so much to be confirmed. Maybe some mock is missing? 🤔

Yes I agree @seaona. Test has failed 4 times in this PR and as you suggested I will investigate why the transaction is taking longer than expected from the mocks perspective.

we can revert this change, as now we've seen that the root cause was that a mock was missing, causing a non-deterministic delay in the API response for the trades

test/e2e/flask/user-operations.spec.ts

seaona · 2024-07-22T15:23:48Z

test/data/mock-data.js

@@ -429,6 +429,291 @@ const TRADES_API_MOCK_RESULT = [
  },
 ];

+const SWAP_TRANSACTION_TRADES_QUOTE_MOCK = [


I think it can be good if we add a more explicit name, so it can be re-used in another test if needed. Something like:

Suggested change

const SWAP_TRANSACTION_TRADES_QUOTE_MOCK = [

const SWAP_ETH_USDC_TRADES_MOCK = [

Co-authored-by: seaona <[email protected]>

…xtension into fix-flaky-test-25912

… fix-flaky-test-25912

greptile-apps

PR Summary

This pull request addresses the flaky test issue in the 'User Operations from swap' test case by introducing additional wait times and implementing checks to ensure the 'View in activity' button is absent.

test/data/mock-data.js: Added SWAP_TEST_ETH_USDC_TRADES_MOCK to support various swap scenarios.
test/e2e/flask/user-operations.spec.ts: Introduced mock data and server utility, added a function to mock swap transaction quotes, and updated test setup to use this mock function for consistent test conditions.

_{2 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

seaona

LGTM! Nice work @hjetpoluru 🙌

chloeYue

LGTM ! Thanks for the fix !

… fix-flaky-test-25912

hjetpoluru · 2024-07-23T12:57:44Z

Thanks @seaona for helping me in analyzing and guiding the process.

greptile-apps

PR Summary

(updates since last review)

This PR addresses the flaky test issue in the 'User Operations from swap' test case by introducing additional wait times and implementing checks to ensure the 'View in activity' button is absent.

test/e2e/flask/user-operations.spec.ts: Introduced mock data and server utility, added a function to mock swap transaction quotes, and updated test setup to use this mock function for consistent test conditions.
app/scripts/controllers/user-storage/user-storage-controller.ts: Updated isProfileSyncingEnabled property to allow a null value, requiring careful testing to ensure all code paths handle null correctly.
app/scripts/migrations/120.1.ts: Added migration to set isProfileSyncingEnabled to null in UserStorageController state, ensuring consistency in user data.
app/scripts/migrations/122.ts: Added migration to set redesignedConfirmationsEnabled to true in PreferencesController, enabling redesigned confirmations by default.
test/e2e/helpers.js: Introduced tempToggleSettingRedesignedConfirmations function to ensure redesigned confirmation settings are applied during tests.

_{47 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

sonarqubecloud · 2024-07-23T16:05:43Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

greptile-apps

PR Summary

(updates since last review)

This PR addresses the flaky test issue in the 'User Operations from swap' test case by introducing mocks and additional wait times to stabilize the test.

.circleci/config.yml: Removed test-e2e-swap-playwright job to streamline CI pipeline.
app/scripts/background.js: Introduced mock implementations for Trezor and Ledger bridges in test environments.
app/scripts/lib/hardware-keyring-builder-factory.ts: Added FakeKeyringBridge type for improved testing.
test/e2e/api-specs/ConfirmationRejectionRule.ts: Simplified logic to always click 'Cancel' instead of 'Reject'.
test/stub/keyring-bridge.js: Added mock implementations for keyring bridges to improve test reliability.

_{8 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

codecov · 2024-07-23T16:27:41Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.68%. Comparing base (82382cd) to head (afefcf2).
Report is 1 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #25913   +/-   ##
========================================
  Coverage    69.68%   69.68%           
========================================
  Files         1405     1405           
  Lines        49701    49701           
  Branches     13738    13738           
========================================
  Hits         34630    34630           
  Misses       15071    15071

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

metamaskbot · 2024-07-23T16:40:15Z

Builds ready [afefcf2]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (189 ± 192 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	74	341	134	53	26
		domContentLoaded	10	136	38	27	13
		load	47	1919	189	399	192
		domInteractive	10	136	38	27	13

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

hjetpoluru added 4 commits July 17, 2024 16:26

Fix for the flaky test swap

3813a95

Fixed lint

f814f16

conditional wait added

6480645

Fixed lint

e67d49c

hjetpoluru requested review from HowardBraham, seaona and chloeYue July 17, 2024 20:47

hjetpoluru requested a review from a team as a code owner July 17, 2024 20:47

hjetpoluru self-assigned this Jul 17, 2024

hjetpoluru added team-extension-platform flaky tests labels Jul 17, 2024

metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Jul 17, 2024

hjetpoluru marked this pull request as draft July 17, 2024 22:53

seaona reviewed Jul 18, 2024

View reviewed changes

Added swap transcation quote mock

1759cfe

seaona reviewed Jul 22, 2024

View reviewed changes

test/e2e/flask/user-operations.spec.ts Outdated Show resolved Hide resolved

seaona reviewed Jul 22, 2024

View reviewed changes

test/e2e/flask/user-operations.spec.ts Outdated Show resolved Hide resolved

seaona reviewed Jul 22, 2024

View reviewed changes

hjetpoluru and others added 7 commits July 22, 2024 12:35

Update test/e2e/flask/user-operations.spec.ts

2b32af7

Co-authored-by: seaona <[email protected]>

Update test/e2e/flask/user-operations.spec.ts

cc83bef

Co-authored-by: seaona <[email protected]>

Addressed review comments by Mariona and fixed lint

af65ef7

Merge branch 'fix-flaky-test-25912' of github.com:MetaMask/metamask-e…

6ecce9d

…xtension into fix-flaky-test-25912

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

6fbe8a1

… fix-flaky-test-25912

Fixing lint

0a8fe9e

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

076c592

… fix-flaky-test-25912

hjetpoluru marked this pull request as ready for review July 22, 2024 18:42

hjetpoluru requested a review from seaona July 22, 2024 18:43

greptile-apps bot reviewed Jul 22, 2024

View reviewed changes

metamaskbot removed the INVALID-PR-TEMPLATE PR's body doesn't match template label Jul 22, 2024

hjetpoluru requested a review from vthomas13 July 22, 2024 20:17

seaona approved these changes Jul 23, 2024

View reviewed changes

pedronfigueiredo approved these changes Jul 23, 2024

View reviewed changes

chloeYue approved these changes Jul 23, 2024

View reviewed changes

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

5dac9ff

… fix-flaky-test-25912

greptile-apps bot reviewed Jul 23, 2024

View reviewed changes

Merge branch 'develop' into fix-flaky-test-25912

afefcf2

greptile-apps bot reviewed Jul 23, 2024

View reviewed changes

HowardBraham approved these changes Jul 23, 2024

View reviewed changes

HowardBraham merged commit cebb0db into develop Jul 23, 2024
75 of 76 checks passed

HowardBraham deleted the fix-flaky-test-25912 branch July 23, 2024 16:28

github-actions bot locked and limited conversation to collaborators Jul 23, 2024

metamaskbot added the release-12.3.0 Issue or pull request that will be included in release 12.3.0 label Jul 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: flaky test 25912 #25913

fix: flaky test 25912 #25913

hjetpoluru commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024

seaona Jul 18, 2024

seaona Jul 18, 2024

hjetpoluru Jul 18, 2024

seaona Jul 22, 2024

seaona Jul 22, 2024

greptile-apps bot left a comment

seaona left a comment

chloeYue left a comment

hjetpoluru commented Jul 23, 2024

greptile-apps bot left a comment

sonarqubecloud bot commented Jul 23, 2024

greptile-apps bot left a comment

codecov bot commented Jul 23, 2024

metamaskbot commented Jul 23, 2024

	const SWAP_TRANSACTION_TRADES_QUOTE_MOCK = [
	const SWAP_ETH_USDC_TRADES_MOCK = [

fix: flaky test 25912 #25913

fix: flaky test 25912 #25913

Conversation

hjetpoluru commented Jul 17, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Jul 17, 2024

seaona Jul 18, 2024

Choose a reason for hiding this comment

seaona Jul 18, 2024

Choose a reason for hiding this comment

hjetpoluru Jul 18, 2024

Choose a reason for hiding this comment

seaona Jul 22, 2024

Choose a reason for hiding this comment

seaona Jul 22, 2024

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

seaona left a comment

Choose a reason for hiding this comment

chloeYue left a comment

Choose a reason for hiding this comment

hjetpoluru commented Jul 23, 2024

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

sonarqubecloud bot commented Jul 23, 2024

Quality Gate passed

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

codecov bot commented Jul 23, 2024

Codecov Report

metamaskbot commented Jul 23, 2024

hjetpoluru commented Jul 17, 2024 •

edited

Loading