feat: added create solana account test #28866

javiergarciavera · 2024-12-03T11:11:08Z

Description

E2E tests covering different scenarios for creating/removing Solana accounts.
Refactor around some snap logic for BTC/Solana

Related issues

Fixes:

Manual testing steps

Running flask tests should pass

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-12-03T11:11:22Z

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.

socket-security · 2024-12-05T09:51:22Z

No dependency changes detected. Learn more about Socket for GitHub ↗︎

👍 No dependency changes detected in pull request

chloeYue · 2024-12-19T09:18:28Z

test/e2e/page-objects/pages/account-list-page.ts

+        return isValid;
+      },
+      20000,
+      true,


could you check the change here? i don't think the change is necessary and correct here for function check_numberOfAvailableAccounts.

this was a change proposed by Charlie but it was missing the return statement and that's why some unrelated tests were failing. I added this part and now they perfectly work

metamaskbot · 2024-12-19T09:21:53Z

Builds ready [b82bfe3]

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

Page Load Metrics (1474 ± 42 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1349	1673	1476	88	42
		domContentLoaded	1341	1653	1455	82	39
		load	1349	1673	1474	87	42
		domInteractive	22	68	39	18	9
		backgroundConnect	8	54	22	15	7
		firstReactRender	15	103	36	31	15
		getState	4	83	19	25	12
		initialActions	0	1	0	0	0
		loadScripts	966	1230	1076	68	33
		setupStore	6	51	12	13	6
		uiStartup	1515	2405	1727	228	109

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

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

chloeYue

please recheck the change for check_numberOfAvailableAccounts, it's not correct.

javiergarciavera · 2024-12-19T09:43:39Z

please recheck the change for check_numberOfAvailableAccounts, it's not correct.
Hey @chloeYue could you explain what part is not correct? The code was basically a suggestion from @ccharly, but it was missing the return statement, that's why some general e2e tests failed (unrelated to the ones added in this PR), so I added the missing return statement and now everything passes. If you look at the change, it's doing exactly the same but a bit more verbose

chloeYue · 2024-12-19T09:44:58Z

Hi guys, i blocked this PR to be merged because the change for functionality check_numberOfAvailableAccounts is wrong, it's very dangerous to merge this PR, because with the wrong change, the functionality is broken and will be success in anycase. See this example that i try to call this function with a wrong parameter: the test still success after the timeout

Screen.Recording.2024-12-19.at.10.39.32.mov

chloeYue · 2024-12-19T09:47:48Z

Suggested change is to revert the change, we don't need the console log for each polling, it will make the log message too long, and we don't need the change for the added parameter true, it will block the code to throw error, which causes the problem.

chloeYue · 2024-12-19T09:52:57Z

please recheck the change for check_numberOfAvailableAccounts, it's not correct.
Hey @chloeYue could you explain what part is not correct? The code was basically a suggestion from @ccharly, but it was missing the return statement, that's why some general e2e tests failed (unrelated to the ones added in this PR), so I added the missing return statement and now everything passes. If you look at the change, it's doing exactly the same but a bit more verbose

it's not doing exactly the same as before, see explanation above.

chloeYue · 2024-12-19T10:00:50Z

please recheck the change for check_numberOfAvailableAccounts, it's not correct.
Hey @chloeYue could you explain what part is not correct? The code was basically a suggestion from @ccharly, but it was missing the return statement, that's why some general e2e tests failed (unrelated to the ones added in this PR), so I added the missing return statement and now everything passes. If you look at the change, it's doing exactly the same but a bit more verbose

Could you explain what do you mean by the origin function was missing the return statement ? it has the return statement return internalAccounts.length === expectedNumberOfAccounts;

javiergarciavera · 2024-12-19T10:26:32Z

please recheck the change for check_numberOfAvailableAccounts, it's not correct.

hey @chloeYue I made a change aligned to what you mentioned. Could you please review and confirm it's alright and approve?
Thanks

metamaskbot · 2024-12-19T10:39:05Z

Builds ready [72029fc]

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

Page Load Metrics (1805 ± 139 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	245	2551	1630	525	252
		domContentLoaded	1447	2457	1771	288	138
		load	1480	2528	1805	290	139
		domInteractive	25	108	51	23	11
		backgroundConnect	9	78	30	21	10
		firstReactRender	16	98	44	30	14
		getState	6	85	23	21	10
		initialActions	0	1	0	0	0
		loadScripts	1077	1882	1340	235	113
		setupStore	6	58	18	17	8
		uiStartup	1708	2881	2156	367	176

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

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

chloeYue

LGTM

chloeYue · 2024-12-19T10:42:47Z

please recheck the change for check_numberOfAvailableAccounts, it's not correct.

hey @chloeYue I made a change aligned to what you mentioned. Could you please review and confirm it's alright and approve? Thanks

Thanks for the change, looks good now.

ulissesferreira and others added 3 commits December 3, 2024 09:25

chore: solana changes to work locally

2c8211f

feat: added mocks for solana snap on flask tests

17c2be1

feat: added create solana account test

0d894c6

javiergarciavera requested a review from a team as a code owner December 3, 2024 11:11

javiergarciavera added 5 commits December 3, 2024 13:56

feat: updated snapId to not run locally

1338d6e

feat: create-account scenario working

236603b

feat: added create/remove solana account

cccee7d

feat: fixed tests

2144433

feat: tests done

7fd295c

javiergarciavera changed the base branch from solana to main December 5, 2024 10:05

javiergarciavera requested a review from a team as a code owner December 5, 2024 10:05

javiergarciavera and others added 11 commits December 5, 2024 11:20

feat: rebase main

54a195a

feat: rebase

a0e0313

feat: revert builds.yml

7960ad6

Merge branch 'main' into create-solana-account-e2e

9f81c4e

feat: upgraded solana snap version

7834f81

feat: rebase

4df5496

feat: upgrade snap version

6ad09a4

feat: added solana to the flask build

b5b3f4f

feat: fix lint

3ba9c0d

feat: fixed flakiness

a2c10f5

feat: updated policy

3ff0057

javiergarciavera requested a review from a team as a code owner December 10, 2024 15:47

javiergarciavera added 5 commits December 10, 2024 16:51

feat: fix lint

faa7e9d

feat: fixes conflicts

ea3683f

feat: fix conflict

625f83f

feat: fixed conflicts on yarn.lock

cae684d

feat: fix flakiness waiting for a component

6dac98b

javiergarciavera dismissed xavier-brochard’s stale review via b82bfe3 December 19, 2024 08:48

chloeYue reviewed Dec 19, 2024

View reviewed changes

xavier-brochard previously approved these changes Dec 19, 2024

View reviewed changes

C-Ivan self-requested a review December 19, 2024 09:30

C-Ivan previously approved these changes Dec 19, 2024

View reviewed changes

javiergarciavera added this pull request to the merge queue Dec 19, 2024

chloeYue requested changes Dec 19, 2024

View reviewed changes

chloeYue removed this pull request from the merge queue due to a manual request Dec 19, 2024

racitores previously approved these changes Dec 19, 2024

View reviewed changes

feat: changed call to wait

72029fc

javiergarciavera dismissed stale reviews from racitores, C-Ivan, and xavier-brochard via 72029fc December 19, 2024 09:55

chloeYue approved these changes Dec 19, 2024

View reviewed changes

racitores approved these changes Dec 19, 2024

View reviewed changes

javiergarciavera added this pull request to the merge queue Dec 19, 2024

Merged via the queue into main with commit dd65944 Dec 19, 2024
77 checks passed

javiergarciavera deleted the create-solana-account-e2e branch December 19, 2024 11:20

github-actions bot locked and limited conversation to collaborators Dec 19, 2024

metamaskbot added the release-12.11.0 Issue or pull request that will be included in release 12.11.0 label Dec 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: added create solana account test #28866

feat: added create solana account test #28866

javiergarciavera commented Dec 3, 2024 •

edited

Loading

github-actions bot commented Dec 3, 2024 •

edited

Loading

socket-security bot commented Dec 5, 2024 •

edited

Loading

chloeYue Dec 19, 2024

javiergarciavera Dec 19, 2024

metamaskbot commented Dec 19, 2024

chloeYue left a comment

javiergarciavera commented Dec 19, 2024 •

edited

Loading

chloeYue commented Dec 19, 2024 •

edited

Loading

chloeYue commented Dec 19, 2024

chloeYue commented Dec 19, 2024

chloeYue commented Dec 19, 2024

javiergarciavera commented Dec 19, 2024

metamaskbot commented Dec 19, 2024

chloeYue left a comment

chloeYue commented Dec 19, 2024

feat: added create solana account test #28866

feat: added create solana account test #28866

Conversation

javiergarciavera commented Dec 3, 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 Dec 3, 2024 • edited Loading

socket-security bot commented Dec 5, 2024 • edited Loading

chloeYue Dec 19, 2024

Choose a reason for hiding this comment

javiergarciavera Dec 19, 2024

Choose a reason for hiding this comment

metamaskbot commented Dec 19, 2024

chloeYue left a comment

Choose a reason for hiding this comment

javiergarciavera commented Dec 19, 2024 • edited Loading

chloeYue commented Dec 19, 2024 • edited Loading

chloeYue commented Dec 19, 2024

chloeYue commented Dec 19, 2024

chloeYue commented Dec 19, 2024

javiergarciavera commented Dec 19, 2024

metamaskbot commented Dec 19, 2024

chloeYue left a comment

Choose a reason for hiding this comment

chloeYue commented Dec 19, 2024

javiergarciavera commented Dec 3, 2024 •

edited

Loading

github-actions bot commented Dec 3, 2024 •

edited

Loading

socket-security bot commented Dec 5, 2024 •

edited

Loading

javiergarciavera commented Dec 19, 2024 •

edited

Loading

chloeYue commented Dec 19, 2024 •

edited

Loading