Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

New automation tests #1080

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from
Draft

New automation tests #1080

wants to merge 5 commits into from

Conversation

xbeghers
Copy link
Collaborator

Description

Tests added

  • Hide account
  • Remove imported account
  • Switch account
  • Rename account
  • Review flow
  • Backup secret phrase
  • Change password

Small improvements and fixes across the tests.
Fixed issue with not always loading CSPR value for tests which use transaction

Linked tickets

No active tickets were created for test automation

ost-ptk
ost-ptk requested changes Oct 30, 2024
View reviewed changes
eslint.config.mjs Outdated
@@ -0,0 +1,13 @@
import globals from "globals";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please tell me why we need this file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file shouldn't be here, will update in next PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove it in this PR

package.json Outdated
"eslint-plugin-react-hooks": "4.6.0",
"file-loader": "^6.2.0",
"fs-extra": "11.2.0",
"globals": "^15.11.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this package do?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably was added when installing and trying to fix issue with eslint. Will try to clean it up

package.json Outdated
@@ -167,6 +169,7 @@
"ts-node": "10.9.1",
"tsconfig-paths-webpack-plugin": "^4.1.0",
"typescript": "5.4.5",
"typescript-eslint": "^8.9.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this?

package.json Outdated
@@ -110,6 +110,7 @@
"@babel/preset-env": "7.23.2",
"@babel/preset-react": "7.18.6",
"@babel/preset-typescript": "^7.23.3",
"@eslint/js": "^9.12.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this?

README.md Outdated
@@ -160,7 +160,7 @@ You should install Redux DevTools browser extension and connect it to Redux DevT

Write tests into `e2e-tests` folder.

To run e2e tests, you must use npm script `npm run e2e:chrome:ui:popup` or `e2e:chrome:ui:onboarding`.
To run e2e tests, you must use npm script `` or `e2e:chrome:ui:onboarding`.npm run e2e:chrome:ui:popup
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess there is an error

e2e-tests/popup/review-flow/review-flow.spec.ts Outdated
);

popup(
'negative review flow - go to home page',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same

e2e-tests/popup/review-flow/review-flow.spec.ts Outdated
Comment on lines 228 to 305
await unlockVault();

await popupPage.route(URLS.rpc, route =>
route.fulfill(RPC_RESPONSE.success)
);

await new Promise(r => setTimeout(r, 5000));

await popupPage.getByText('Send').click();

await popupExpect(
popupPage.getByRole('heading', { name: 'Select token and account' })
).toBeVisible();

await popupPage.getByRole('button', { name: 'Next' }).click();

await popupExpect(
popupPage.getByRole('heading', { name: 'Specify recipient' })
).toBeVisible();

await popupExpect(
popupPage.getByRole('button', { name: 'Next' })
).toBeDisabled();

await popupPage
.getByPlaceholder('Public key or name', { exact: true })
.fill(DEFAULT_SECOND_ACCOUNT.publicKey);

await popupExpect(
popupPage.getByText(DEFAULT_SECOND_ACCOUNT.mediumTruncatedPublicKey, {
exact: true
})
).toBeVisible();

await popupPage
.getByText(DEFAULT_SECOND_ACCOUNT.mediumTruncatedPublicKey, {
exact: true
})
.click();

await popupPage.getByRole('button', { name: 'Next' }).click();

await popupExpect(
popupPage.getByRole('heading', { name: 'Enter amount' })
).toBeVisible();

await popupPage.getByRole('button', { name: 'Next' }).click();

await popupExpect(
popupPage.getByRole('heading', { name: 'Confirm sending' })
).toBeVisible();

await popupExpect(
popupPage.getByText(DEFAULT_SECOND_ACCOUNT.publicKey)
).toBeVisible();

await popupExpect(
popupPage.getByRole('button', { name: 'Confirm send' })
).toBeDisabled();

// Scroll to the bottom
await popupPage.evaluate(() => {
const container = document.querySelector('#ms-container');

container?.scrollTo(0, 1000);
});

await popupExpect(
popupPage.getByRole('button', { name: 'Confirm send' })
).not.toBeDisabled();

await popupPage.getByRole('button', { name: 'Confirm send' }).click();

await popupExpect(
popupPage.getByRole('heading', { name: 'You submitted a transaction' })
).toBeVisible();

await popupPage.getByRole('button', { name: 'Done' }).click();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code is used 3 times in this file. Maybe try to reuse it somehow and not duplicate

e2e-tests/popup/settings/backup-secret-phrase.spec.ts Outdated
}
);
popup(
'should display safety tips',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that this test is useful. Here you test UI text and no functionality

e2e-tests/popup/settings/backup-secret-phrase.spec.ts
);

popup(
'should timeout after providing wrong password 5 times',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we already have a test for locking the wallet for 5 minutes when the user types the wrong password 5 times
common/wallet.spec.ts

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but I wanted to include it in other fields which require password

e2e-tests/popup/settings/change-password.spec.ts
Comment on lines +36 to +38
await popupExpect(
popupPage.getByTestId('network-switcher').getByRole('img')
).toBeVisible();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you check this? it does not apply to this test

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it only to make sure that user is logged in

@ost-ptk
Copy link
Member

ost-ptk commented Oct 30, 2024

Please follow our style of naming PR and description

image

@ost-ptk
Copy link
Member

ost-ptk commented Oct 30, 2024

Also, please check why tests failed on github

@Comp0te Comp0te marked this pull request as draft November 15, 2024 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ost-ptk ost-ptk ost-ptk requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@xbeghers xbeghers

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@xbeghers @ost-ptk