test: [POM] Migrate autodetect and import nft e2e tests to use Page Object Model #28383

chloeYue · 2024-11-08T16:52:24Z

Description

Migrate e2e tests test/e2e/tests/tokens/nft/auto-detect-nft.spec.js to TS and Page Object Model, to reduce flakiness.
Migrate e2e tests test/e2e/tests/tokens/nft/import-nft.spec.js to TS and Page Object Model, to reduce flakiness.
Create page classe functions for nft
Deprecate/remove old functions in helper.js

Related issues

Fixes: #28388

Manual testing steps

Check code readability, make sure tests 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-11-08T16:52:37Z

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.

metamaskbot · 2024-11-08T21:39:57Z

Builds ready [60d54eb]

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, 7
- 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, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1885 ± 136 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1632	2815	1901	288	138
		domContentLoaded	1611	2803	1863	281	135
		load	1629	2815	1885	283	136
		domInteractive	14	214	61	45	21
		backgroundConnect	8	133	26	28	14
		firstReactRender	52	279	116	56	27
		getState	4	120	22	29	14
		initialActions	0	1	0	0	0
		loadScripts	1163	2280	1393	251	120
		setupStore	6	47	10	9	4
		uiStartup	1803	3061	2120	308	148

Bundle size diffs

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

metamaskbot · 2024-11-12T10:54:47Z

Builds ready [b5ba5fb]

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, 7
- 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, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (2109 ± 141 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1682	2790	2125	297	143
		domContentLoaded	1667	2766	2085	284	136
		load	1671	2794	2109	293	141
		domInteractive	28	94	58	20	9
		backgroundConnect	7	72	25	19	9
		firstReactRender	49	300	127	65	31
		getState	5	64	15	16	8
		initialActions	0	1	0	0	0
		loadScripts	1202	2011	1534	217	104
		setupStore	5	86	22	24	11
		uiStartup	1877	3109	2385	363	174

Bundle size diffs

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

seaona

LGTM!! nice job 🙌

hjetpoluru · 2024-11-12T18:58:15Z

test/e2e/tests/tokens/nft/auto-detect-nft.spec.ts

+      async ({ driver }) => {
+        await loginWithBalanceValidation(driver);
+
+        // navigate to security & privacy settings and toggle on NFT autodetection


@chloeYue, this is a suggestion: could we include comments in the test console.log?

I feel that when analyzing any flaky test, it would be helpful especially when actions and pages are repeated like visiting the settings twice or if the test has lot of switching windows.

Including the test console.log would be beneficial. You don't have to modify this PR, but if you think it would help please do include them.

Hi @hjetpoluru , yes, I totally agree that we currently have very poor logging and need to add more console.log statements for page interactions. However, I try to put these logs in the page class functions instead of the test bodies. This way, when people call these functions, the logs are automatically added. Please see the current log on this branch for autodetect NFT:

hjetpoluru · 2024-11-12T18:59:48Z

@chloeYue, general question: how do you determine which flaky tests you want to migrate?

hjetpoluru

Nice and neat.

chloeYue · 2024-11-12T22:06:12Z

@chloeYue, general question: how do you determine which flaky tests you want to migrate?

Hi @hjetpoluru, thanks for your comment, that's a good question.

Generally, I choose flaky tests that cover important functionalities and have a lower likelihood of being fixed by other teams. For example, confirmation team has people actively working on e2e tests and creating new tests following Page Object Models, so I leave confirmation related tests to them.

For assets-related, accounts-related, and wallet UX related tests, I try to prioritize them because these teams don't have a dedicated QA engineer and need more support. Additionally, the functionalities for these teams are foundational for many other scenarios. For instance, many other scenarios require switching accounts and sending transactions. By fixing flaky tests for accounts and assets, I create Page Object Model classes with stable functions that are ready to use, which benefits other feature teams and other functionalities as well.

Another priority example is onboarding. Onboarding tests were flaky and belong to extension platform team, so I also prioritize onboarding tests.

metamaskbot · 2024-11-13T07:29:42Z

Builds ready [252a3e5]

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, 7
- 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, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1754 ± 69 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	303	2180	1628	451	217
		domContentLoaded	1569	2096	1724	129	62
		load	1586	2183	1754	143	69
		domInteractive	24	82	44	17	8
		backgroundConnect	9	87	30	24	12
		firstReactRender	78	154	97	15	7
		getState	5	54	16	15	7
		initialActions	0	1	0	0	0
		loadScripts	1118	1578	1263	110	53
		setupStore	6	42	9	8	4
		uiStartup	1772	2478	1939	168	81

Bundle size diffs

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

chloeYue added 4 commits November 6, 2024 18:32

migrate import nft tests

f1c7747

migrate reveal srp tests

52d3c80

Merge branch 'migrate-srp-tests' into migrate-import-nft-tests

afb6552

migrate autodetect nft tests

200b48c

chloeYue added the team-extension-platform label Nov 8, 2024

chloeYue self-assigned this Nov 8, 2024

chloeYue requested a review from a team as a code owner November 8, 2024 16:52

chloeYue requested a review from mikesposito November 8, 2024 16:52

chloeYue marked this pull request as draft November 8, 2024 16:52

chloeYue removed the request for review from mikesposito November 8, 2024 16:52

chloeYue added 2 commits November 8, 2024 18:20

Merge branch 'develop' into migrate-import-nft-tests

3fa97d5

fix lint

60d54eb

chloeYue changed the title ~~test: Migrate import nft tests~~ test: [POM] Migrate autodetect and import nft e2e tests to use Page Object Model Nov 8, 2024

chloeYue added 6 commits November 12, 2024 10:24

Merge branch 'develop' into migrate-import-nft-tests

1f9df48

fix lint

38cbfd0

improve function name

0c7b8c6

remobe unused function

2b8912e

fix function name

9ca7e4f

fix lint

b5ba5fb

chloeYue marked this pull request as ready for review November 12, 2024 10:55

seaona approved these changes Nov 12, 2024

View reviewed changes

hjetpoluru reviewed Nov 12, 2024

View reviewed changes

hjetpoluru approved these changes Nov 12, 2024

View reviewed changes

chloeYue added this pull request to the merge queue Nov 12, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 12, 2024

chloeYue added this pull request to the merge queue Nov 12, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 12, 2024

chloeYue added this pull request to the merge queue Nov 12, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 12, 2024

Merge branch 'develop' into migrate-import-nft-tests

252a3e5

chloeYue added this pull request to the merge queue Nov 13, 2024

Merged via the queue into develop with commit 54a9743 Nov 13, 2024
77 checks passed

chloeYue deleted the migrate-import-nft-tests branch November 13, 2024 07:41

github-actions bot locked and limited conversation to collaborators Nov 13, 2024

metamaskbot added the release-12.8.0 Issue or pull request that will be included in release 12.8.0 label Nov 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: [POM] Migrate autodetect and import nft e2e tests to use Page Object Model #28383

test: [POM] Migrate autodetect and import nft e2e tests to use Page Object Model #28383

chloeYue commented Nov 8, 2024 •

edited

Loading

github-actions bot commented Nov 8, 2024

metamaskbot commented Nov 8, 2024

metamaskbot commented Nov 12, 2024

seaona left a comment

hjetpoluru Nov 12, 2024

chloeYue Nov 12, 2024 •

edited

Loading

hjetpoluru commented Nov 12, 2024

hjetpoluru left a comment

chloeYue commented Nov 12, 2024

metamaskbot commented Nov 13, 2024

test: [POM] Migrate autodetect and import nft e2e tests to use Page Object Model #28383

test: [POM] Migrate autodetect and import nft e2e tests to use Page Object Model #28383

Conversation

chloeYue commented Nov 8, 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 Nov 8, 2024

metamaskbot commented Nov 8, 2024

metamaskbot commented Nov 12, 2024

seaona left a comment

Choose a reason for hiding this comment

hjetpoluru Nov 12, 2024

Choose a reason for hiding this comment

chloeYue Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

hjetpoluru commented Nov 12, 2024

hjetpoluru left a comment

Choose a reason for hiding this comment

chloeYue commented Nov 12, 2024

metamaskbot commented Nov 13, 2024

chloeYue commented Nov 8, 2024 •

edited

Loading

chloeYue Nov 12, 2024 •

edited

Loading