test: blockaid update version and reenable specs #28121

seaona · 2024-10-28T11:36:58Z

Description

This PR does the following:

Changes the specs that were going to the test dapp using the ip address as hostname, to use localhost instead (see issue here)
Re-enables skipped specs
Updates the update-mock-cdn script, so it adds "Content-Type": "text/plain" into the response headers
Updates the ppom module to the last version (config, stale and stale diff) using the updated script

Related issues

Fixes:

Manual testing steps

Check ci

Screenshots/Recordings

Example of malicious signature working on e2e environment

malicious-e2e-working.mp4

Encoded mock data before/after content type

encoded-data-mcked.mp4

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-10-28T11:37:13Z

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-10-28T11:59:42Z

test/e2e/tests/ppom/ppom-blockaid-alert-simple-send.spec.js

-    })
+    .forGet(
+      'https://static.cx.metamask.io/api/v1/confirmations/ppom/ppom_version.json',
+    )


to cause a warning, it doesn't work anymore the debug_traceCall 500, but we can use this failed request instead

seaona · 2024-10-28T12:28:53Z

test/e2e/mock-cdn/cdn-config.txt

the updated txt files are obtained by running the update-mock-cdn script. The data is encoded in brotli, taken from making a real request to the prod endpoints, to get the latest data

Thanks @seaona for documenting this information.

seaona · 2024-10-28T12:29:36Z

test/e2e/mock-cdn/cdn-stale-diff-res-headers.json

@@ -1,3 +1,4 @@
 {
-  "Etag": "W/\"ece7f5f533b8978063633ea5b1f8a0fc\""
+  "Content-Type": "text/plain",


with this addition, we are now able to see the mocked response in the network tab (see attached video in the description)

seaona · 2024-10-28T12:30:00Z

test/e2e/tests/ppom/ppom-blockaid-alert-erc20-transfer.spec.js

@@ -161,14 +157,17 @@ async function mockInfura(mockServer) {
 }

 describe('PPOM Blockaid Alert - Malicious ERC20 Transfer @no-mmi', function () {
-  // eslint-disable-next-line mocha/no-skipped-tests
-  it.skip('should show banner alert', async function () {


not skipping the test anymore as it's now fixed

metamaskbot · 2024-10-28T12:30:23Z

Builds ready [94bad92]

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, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2113 ± 46 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1961	2407	2117	89	43
		domContentLoaded	1888	2294	2062	80	38
		load	1961	2449	2113	95	46
		domInteractive	19	79	47	18	9
		backgroundConnect	13	156	55	34	16
		firstReactRender	55	215	100	33	16
		getState	8	63	19	18	9
		initialActions	0	1	0	0	0
		loadScripts	1371	1706	1525	69	33
		setupStore	14	96	40	27	13
		uiStartup	2178	2677	2353	124	59

Bundle size diffs [🚨 Warning! Bundle size has increased!]

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

seaona · 2024-10-28T12:30:37Z

test/e2e/tests/ppom/ppom-blockaid-alert-erc20-transfer.spec.js

@@ -181,10 +180,10 @@ describe('PPOM Blockaid Alert - Malicious ERC20 Transfer @no-mmi', function () {
      async ({ driver }) => {
        const expectedTitle = 'This is a deceptive request';
        const expectedDescription =
-          'If you approve this request, you might lose your assets.';


the malicious descriptions have slightly changed so we need to modify them in the specs

seaona · 2024-10-28T12:31:23Z

test/e2e/tests/ppom/ppom-blockaid-alert-erc20-transfer.spec.js

-        assert(
-          bannerAlertText.includes(expectedDescription),
-          `Unexpected banner alert description. Expected: ${expectedDescription} \nExpected reason: transfer_farming\n`,
-        );


we already find the element by its text, so no need to assert after

seaona · 2024-10-28T12:31:39Z

test/e2e/tests/ppom/ppom-blockaid-alert-trade-order-farming.spec.js

@@ -90,14 +86,17 @@ async function mockInfura(mockServer) {
 }

 describe('PPOM Blockaid Alert - Set Trade farming order @no-mmi', function () {
-  // eslint-disable-next-line mocha/no-skipped-tests
-  it.skip('should show banner alert', async function () {


not skipping the test anymore as it's now fixed

seaona · 2024-10-28T12:32:29Z

test/e2e/tests/ppom/ppom-blockaid-alert-erc20-transfer.spec.js


        await unlockWallet(driver);
-        await openDapp(driver);
+        await driver.openNewPage('http://localhost:8080');


we need to go to localhost and not 127.0.0.1 otherwise the interactions won't be flagged by ppom (see issue)

hjetpoluru

LGTM

seaona · 2024-10-28T13:11:39Z

test/e2e/tests/ppom/ppom-blockaid-alert-simple-send.spec.js

      },
    );
  });

-  // eslint-disable-next-line mocha/no-skipped-tests
-  it.skip('should show "Request may not be safe" if the PPOM request fails to check transaction', async function () {


not skipping the test anymore as it's now fixed

chloeYue · 2024-10-28T13:45:33Z

LGTM !

blockaid update and reenable

2040d8b

github-actions bot added the team-extension-platform Extension Platform team label Oct 28, 2024

seaona changed the title ~~test: blockaid update and reenable~~ test: blockaid update version and reenable specs Oct 28, 2024

seaona self-assigned this Oct 28, 2024

fix lint

94bad92

seaona commented Oct 28, 2024

View reviewed changes

seaona marked this pull request as ready for review October 28, 2024 12:00

seaona requested a review from a team as a code owner October 28, 2024 12:00

seaona commented Oct 28, 2024

View reviewed changes

hjetpoluru approved these changes Oct 28, 2024

View reviewed changes

seaona commented Oct 28, 2024

View reviewed changes

chloeYue approved these changes Oct 28, 2024

View reviewed changes

seaona added this pull request to the merge queue Oct 28, 2024

Merged via the queue into develop with commit 3624df4 Oct 28, 2024
100 checks passed

seaona deleted the ppom-update branch October 28, 2024 16:21

github-actions bot locked and limited conversation to collaborators Oct 28, 2024

metamaskbot added the release-12.7.0 Issue or pull request that will be included in release 12.7.0 label Oct 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: blockaid update version and reenable specs #28121

test: blockaid update version and reenable specs #28121

seaona commented Oct 28, 2024 •

edited

Loading

github-actions bot commented Oct 28, 2024

seaona Oct 28, 2024

seaona Oct 28, 2024

hjetpoluru Oct 28, 2024

seaona Oct 28, 2024

seaona Oct 28, 2024

metamaskbot commented Oct 28, 2024

seaona Oct 28, 2024

seaona Oct 28, 2024

seaona Oct 28, 2024

seaona Oct 28, 2024

hjetpoluru left a comment

seaona Oct 28, 2024

chloeYue commented Oct 28, 2024

test: blockaid update version and reenable specs #28121

test: blockaid update version and reenable specs #28121

Conversation

seaona commented Oct 28, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Oct 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metamaskbot commented Oct 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hjetpoluru left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chloeYue commented Oct 28, 2024

seaona commented Oct 28, 2024 •

edited

Loading